From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E6DCE43DD0; Mon, 8 Apr 2024 18:20:52 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 78E43402C9; Mon, 8 Apr 2024 18:20:52 +0200 (CEST) Received: from mail-pg1-f173.google.com (mail-pg1-f173.google.com [209.85.215.173]) by mails.dpdk.org (Postfix) with ESMTP id 9C1DE402C0 for ; Mon, 8 Apr 2024 18:20:50 +0200 (CEST) Received: by mail-pg1-f173.google.com with SMTP id 41be03b00d2f7-5d8b887bb0cso3938558a12.2 for ; Mon, 08 Apr 2024 09:20:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1712593249; x=1713198049; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=84711+Dl7LWWsC7e5qaSKd6SuodqadrsDYDE4rYXGuA=; b=VDR06iPhqRoT+RmAwyfTIyzLjf3V489CF24ylUAPGXgE2hqiG3m/ZCMD8Rbg+VeiJR ZCrF9VSaS++DVp5a6n1bVSN0eNPBeu8EQz1DR7gNJlU1ecnX21cmggJWWB/ib9Nc7Czj 4rX6kbpgjMcYNRUZV7/tZYPbI61e84JmoNSeo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712593249; x=1713198049; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=84711+Dl7LWWsC7e5qaSKd6SuodqadrsDYDE4rYXGuA=; b=cJYnVOTvibHRC5j4hwijTBez/E5qex3QAHr4ZMWQzvUbSx4261YCbwGGKuS0tiRCWF nM44ENote7j/iUVbiwPchX1UuQEtI8nMSyNA1Env0oMMz8lzyazEM/jf31Fx888lkHsg C8nTUtP9NPP8IPly8p7PBfe8PcVk/JpNP5SmI/6pP+17BTQqckaG858Q6sPf9MSpeHfk AH0OWJjysMz7Bi6OpeRV+vV0fLUz49iz6S2r0HduaObrLhfzlxtQnHzXxzTpbJsR4Wwr Srl1PXItT2+Us9SjiinKbAolMX13/FAUvaBryvZxiJnAjOE/LuJTXZIUxylCwH3X7v6d uz2w== X-Forwarded-Encrypted: i=1; AJvYcCVe9uq7RGQElxfedDuOLBjy/+yeNW/L/0gdDWhatZYjh5uYFfT6Y6v+V5/q7Trv8HUKLyPP7lRRV9j6eqs= X-Gm-Message-State: AOJu0YyoCjEknC/7Mn7LKWeVKn9YHGeZMo15q7nIC/yvJJheq90dmgJs KIXYP/YdhulN860ViglTDbzG0uK5c70WqBtYcnrxqKz1HCUGe7CtzTZQ6DT/0hNXMLFwtp78nju gyHsN+JAa/lP8OT1IUYb2CX/Ip3oHK3VxV9/Kfg== X-Google-Smtp-Source: AGHT+IHtlxUsoXVUUBUYJrGuBt6EaYEEXrNYSYFm3w5wWnMvHtJKkhTQSgUnZ9RzUcmsSut8vOI976kIBrHzxxHJigY= X-Received: by 2002:a17:902:f552:b0:1e2:44c7:b2af with SMTP id h18-20020a170902f55200b001e244c7b2afmr13212972plf.61.1712593249585; Mon, 08 Apr 2024 09:20:49 -0700 (PDT) MIME-Version: 1.0 References: <20240312172558.11844-1-jspewock@iol.unh.edu> <20240312172558.11844-2-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Mon, 8 Apr 2024 12:20:38 -0400 Message-ID: Subject: Re: [PATCH v1 1/2] dts: Improve output gathering in interactive shells To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, Luca.Vizzarro@arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Apr 3, 2024 at 5:00=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > On Tue, Mar 12, 2024 at 6:26=E2=80=AFPM wrote: > > > > From: Jeremy Spewock > > > > The current implementation of consuming output from interactive shells > > relies on being able to find an expected prompt somewhere within the > > output buffer after sending the command. This is useful in situations > > where the prompt does not appear in the output itself, but in some > > practical cases (such as the starting of an XML-RPC server for scapy) > > the prompt exists in one of the commands sent to the shell and this can > > cause the command to exit early and creates a race condition between th= e > > server starting and the first command being sent to the server. > > > > This patch addresses this problem by searching for a line that strictly > > ends with the provided prompt, rather than one that simply contains it, > > so that the detection that a command is finished is more consistent. It > > also adds a catch to detect when a command times out before finding the > > prompt so that the exception can be wrapped into a more explicit one an= d > > display the output that it did manage to gather before timing out. > > > > This could still cause problems if the prompt appears at the end of a > line in the output. Maybe we don't need to worry about that until we > actually hit that problem. In any case, I'd like to test this, so > please rebase the patch. :-) I will rebase and send out a v2, but that is a good point. it would be just as easy to instead check to see that the prompt is the only thing on the line, so we could do that instead, which do you think is better? I'm sort of indifferent, I can see how verifying that the line only contains the prompt would be useful in cases like it appears in a docstring or something similar (and it's nice to be more explicit in general), and I think that leaving it as the end of the line could potentially save some verbosity if the line you are looking for is long so you can just detect the end of it. Another problem that I could think of with long lines potentially is if, somehow, you were looking for a prompt that the pseudo-terminal split across a few lines and it split your prompt, but I'm not sure this is really relevant to us at all since we only really expect prompts that are usually short. > > > + try: > > + for line in self._stdout: > > + out +=3D line > > + if line.rstrip().endswith(prompt): > > + break > > + except TimeoutError: > > I like this addition, but maybe we could do better. In the regular SSH > session, we're expected to raise: > * SSHSessionDeadError if the session is not alive, > * SSHTimeoutError if the command execution times out. > > Can we do that here as well? This is a good point, I don't see why we couldn't and thinking about it I like the idea of making this more consistent, good catch. > > > + raise InteractiveCommandExecutionError( > > We could just reuse the SSHTimeoutError exception. Is there a reason > to distinguish between interactive and non-interactive timeouts? I wanted to add a distinction between interactive and non-interactive just because in general the way we log messages when sending commands to interactive shells looks pretty close to what it looks like when you do the same for non-interactive, so if there was an error I thought it might be more helpful in the logs to know which session you were sending the command to when an error was encountered. Maybe, however, a better approach to this would just be keep the exception types the same but change the messages. > > > 2.43.2 > >