Re: [PATCH v4 5/9] dts: add ssh connection extension

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Sep 13, 2022 at 01:32:33PM -0400, Owen Hilyard wrote:
>      On Fri, Jul 29, 2022 at 10:55:46AM +0000, Juraj Linkeš wrote:
>      > The class adds logging and history records to existing pexpect
>      methods.
>      >
>      > Signed-off-by: Owen Hilyard <[1]ohilyard@iol.unh.edu>
>      > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>      > ---
>      >  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.
> 
I'm a little confused by that explanation - can you perhaps clarify? This
close function in part of an class, and the logger member is assigned its value
in the constructor for that class, so how is it possible to call
obj.close() before obj has been constructed?