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 EA233A0032; Tue, 13 Sep 2022 19:33:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AB554021D; Tue, 13 Sep 2022 19:33:11 +0200 (CEST) Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by mails.dpdk.org (Postfix) with ESMTP id 530A640151 for ; Tue, 13 Sep 2022 19:33:10 +0200 (CEST) Received: by mail-pf1-f181.google.com with SMTP id b23so12417749pfp.9 for ; Tue, 13 Sep 2022 10:33:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=3Y7pTV12s5mi1aH+zY+mgkplJ7xLAfssywPdQ9OgNuk=; b=PZCCtWA2wirMD4xvtKmeMjWC7dzB6+jeJgaupMxG+LPypGw41SbJunYqsRy8rvMfMb iCjC1nnjzgyMFJy5fQIu29OGd+qZEtd1ERcqCNghPrZVlrNJVXCOMAQuwqK2I/HSUh/e lI5rgTPveakewXKIpUBIScQj7wR0bQ95Vk2Ec= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=3Y7pTV12s5mi1aH+zY+mgkplJ7xLAfssywPdQ9OgNuk=; b=eu3BjnJAnvj0UFwMfYKSOqWs9XjhNZWev1rTGsT4M1LZ4dXumXDJs7sGkE0tpgdPe0 xiQD+4FpRiJEckkZIgdvHJhenOGrNHr1ysYMJyGJil/evCq8cVubi/R8ovYpyMm1fryQ W27H6au2yVruOB5s9tAGNYFqmCQh2Muu+ZmK0AXBgzHjKoJKa/ugunEB3J43QFe9J2BS iUOQFaNS0JFzCwQuAKoR45CPlg2TvnTgmW8GYgQAUbxQzVrjeEkJ+w3s+PqXzE91ORtc ErErmu3q013WtPhjspyiDWj2sa4yYEXymoY0cg7/pHMr+nH04uYn4JUJgJojCgLIFZUN x+Yw== X-Gm-Message-State: ACgBeo1k3tCZzpGTUzQCprNkhC5U5poJGTrycs+Lk3ku7pCvw2dPc/9y y2kksADlCTg66YCPrILfwLdpNuWkOEnfTGqAK1lNaw== X-Google-Smtp-Source: AA6agR4P2HhlEXVsayOT7u8pixH2iNxMKjmsGspDxvMxHOtdmSSmch7YH4BKzuPmD8zJJIb9bIcoSmKqL3tuDwxs/PU= X-Received: by 2002:aa7:888d:0:b0:538:328b:2ffb with SMTP id z13-20020aa7888d000000b00538328b2ffbmr33757623pfe.82.1663090389407; Tue, 13 Sep 2022 10:33:09 -0700 (PDT) MIME-Version: 1.0 References: <20220728100044.1318484-1-juraj.linkes@pantheon.tech> <20220729105550.1382664-1-juraj.linkes@pantheon.tech> <20220729105550.1382664-6-juraj.linkes@pantheon.tech> In-Reply-To: From: Owen Hilyard Date: Tue, 13 Sep 2022 13:32:33 -0400 Message-ID: Subject: Re: [PATCH v4 5/9] dts: add ssh connection extension To: Bruce Richardson Cc: =?UTF-8?Q?Juraj_Linke=C5=A1?= , Thomas Monjalon , David Marchand , ronan.randles@intel.com, Honnappa Nagarahalli , "Tu, Lijuan" , dev Content-Type: multipart/alternative; boundary="000000000000e3f9bb05e8926777" 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 --000000000000e3f9bb05e8926777 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > > On Fri, Jul 29, 2022 at 10:55:46AM +0000, Juraj Linke=C5=A1 wrote: > > The class adds logging and history records to existing pexpect methods. > > > > Signed-off-by: Owen Hilyard > > Signed-off-by: Juraj Linke=C5=A1 > > --- > > dts/framework/ssh_connection.py | 70 +++++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) > > create mode 100644 dts/framework/ssh_connection.py > > > One comment inline below. > /Bruce Two questions on this function: * Is the getattr() check not equivalent to "if self.logger:"? It is. I missed it when looking over this code. I know that this close function can run in a context where it loses the ability to make system calls (an exit hook), but that doesn't matter for this as far as I know. > * Why the check for a non-none logger in this function, when other functions above always seem to call the logger directly without checking? "close" can be called before "init_log" if the program crashes early enough, so this is avoiding calling a function on a null object. No other function can have that issue because by the time control is returned to the user the logger is properly initalized. This is especially important because an early failure in the community lab will only be able to use logs to figure out what happened. > > > + > > + self.session.close(force) > > -- > 2.30.2 > > --000000000000e3f9bb05e8926777 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, Jul 29, 2022 at 10:55:46AM +0000, Juraj = Linke=C5=A1 wrote:
> The cl= ass adds logging and history records to existing pexpect methods.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linke=C5=A1 <jur= aj.linkes@pantheon.tech>
&g= t; ---
>=C2=A0 dts/framewor= k/ssh_connection.py | 70 +++++++++++++++++++++++++++++++++
>=C2=A0 1 file changed, 70 insertions(+)>=C2=A0 create mode 100644 dt= s/framework/ssh_connection.py
= >

=
One comment inline below.=

/Bruce
<= div>
<= blockquote>
Two questions on= this function:
* Is the getattr() check not equivalent to "if self.logger:"?<= /blockquote>

It is. I missed it when looking over this c= ode. I know that this close function can run in a context where it loses th= e ability to make system calls (an exit hook), but that doesn't matter = for this as far as I know.
=C2=A0
* Why the check for a non-none logger in this funct= ion, when other
=C2=A0 functions above always seem to call the logger directly without c= hecking?

"close" can be called before "= init_log" if the program crashes early enough, so this is avoiding cal= ling a function on a null object. No other function can have that issue bec= ause by the time control is returned to the user the logger is properly ini= talized. This is especially important because an early failure in the commu= nity lab will only be able to use logs to figure out what happened.=C2=A0=C2=A0

<= /blockquote>
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.session.close(force)=
> --
> 2.30.2
>
--000000000000e3f9bb05e8926777--