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 282D443E33; Wed, 10 Apr 2024 12:20:32 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CEB8E402C7; Wed, 10 Apr 2024 12:20:30 +0200 (CEST) Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by mails.dpdk.org (Postfix) with ESMTP id BA1AB402C5 for ; Wed, 10 Apr 2024 12:20:28 +0200 (CEST) Received: by mail-ej1-f50.google.com with SMTP id a640c23a62f3a-a520410b7c0so115577866b.0 for ; Wed, 10 Apr 2024 03:20:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1712744428; x=1713349228; 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=ou0WfQLBcJYndFlo0QOQSxm/NNmnh/KPKvBQ927+wT0=; b=oSt5A1gqFAXTSt7LDKqTDWtYHtEHZwcQ6JdNan5nahbr+Ovk6TlLefEtJV0/ycRsfk CMVKMPXEVsLpilwBL7Rq0ONhoAQZ2HMosnHU4mMUCRI0frtyBVXaFZR0My/8gsMbTAn/ hG8NWtQQna3q8CAsjaCHWGo1uynfUeVENyxxL5cipsTGoW0+ydY0QxCRt18E9kMEFvr/ +Ky5ZlxD+9tC2hVJ45YzkVAIHEh9nyfsARWVic5eMo46kAWD1D6uoZQ+tcyjII2JCj7s LNKEH5XpMSo4ZKgZtd4to4CbUkhi6jAWthNyo0dZaHcu0tbd8wy690wCSErKp4i6SlWr Cz9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712744428; x=1713349228; 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=ou0WfQLBcJYndFlo0QOQSxm/NNmnh/KPKvBQ927+wT0=; b=G0hjOt0UsmYLKag+4hqTtDBMr8Jd7j6HND6WWfXYwtjtTvAOS4pChZ297HcEG8x1hv JKSKwMRz7qV4J3skzVirWjmLOC471XYBfHvb1NUWi40v6IBYmowA0DxHtB+jf6js8otY 4LY9YKiRb1F5nNkAeFhvaZCLYR9kmkeum2z4DElOqxjtw9mQLOuRVZKoQ0Yds3zpufib 7j6SlKsEJcSDv8Vj4XilHMmfLLhTf0ht4XlSlviB+516pHzX13x/ipexgAI7OUaEHrbi xZphmHvTRFX+b9eFMUccIkZQ6faWF+7BtA6nOFw5ny8PNFKQuhX0jslSe9qvTVgntJfu 6C5A== X-Forwarded-Encrypted: i=1; AJvYcCUmr5iVAOMwWjh1ExmRlAcs7uhliRWvRS3F0occaW1pQ0NKCbSh2M2a+oD+s7/QAFQBsgQpqJptNj4bx/4= X-Gm-Message-State: AOJu0YzOJlfu0Tk1lRIXQIjnLdAIjpxqELRvUJrhUcRW41j5YZDIKjU9 3kSVjPqnEkzCNxOLa19cuOpBUxL88+bUNUa7uuijRqeRxm5UMVcFgC6VU1C80IxB/1XRkbahFdU y0DEXaxd/tfSfqBChuJO6Kipz5hitho0oNg4UwQ== X-Google-Smtp-Source: AGHT+IHFtAfupFw5nDMZPbjOXaOM4g4+qL2w/GtAWilS2cuW+dKjAAqcckmttegeIRoCooy9ApG85dBiVhv0aOGzgec= X-Received: by 2002:a17:906:b1d7:b0:a4e:5c23:f776 with SMTP id bv23-20020a170906b1d700b00a4e5c23f776mr1505521ejb.37.1712744428391; Wed, 10 Apr 2024 03:20:28 -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: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Wed, 10 Apr 2024 12:20:17 +0200 Message-ID: Subject: Re: [PATCH v1 1/2] dts: Improve output gathering in interactive shells To: Jeremy Spewock 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 Mon, Apr 8, 2024 at 6:20=E2=80=AFPM Jeremy Spewock wrote: > > 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 shell= s > > > 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 c= an > > > cause the command to exit early and creates a race condition between = the > > > server starting and the first command being sent to the server. > > > > > > This patch addresses this problem by searching for a line that strict= ly > > > ends with the provided prompt, rather than one that simply contains i= t, > > > 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 t= he > > > prompt so that the exception can be wrapped into a more explicit one = and > > > 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? Would this work? I'm thinking we may need to send another extra newline character to put the solitary prompt into the buffer if the command output doesn't contain a newline. > 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. > If it works with checking just the end of the line let's leave it like this (because of the possibility above). I think there shouldn't be any prompts without something after them in docstrings. > > > > > > + 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. > There is probably some value to distinguish between them. We could just mirror the non-interactive exceptions and keep the messages the same. > > > > > > 2.43.2 > > >