> > On Wed, Sep 14, 2022 at 3:46 AM Bruce Richardson < > bruce.richard...@intel.com> 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]ohily...@iol.unh.edu> > > > Signed-off-by: Juraj Linkeš <juraj.lin...@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?
Due to how we are forced to add the hooks to flush logger buffers to disk before shutdown, even in the case of failures or SIGTERM, close can run WHILE the constructor is in the middle of executing. All of the resources except for the logger can be freed by python, but the logger requires special handling, so we check if it is null and then flush the buffers to disk if it is not. The actual logger object is only assigned to self.logger after it is completely initalized, so if it's not null, then it is in a good state and can be safely flushed. Here's a sequence of events that would lead to that check being needed: 1. Start constructing logging handle 2. Execute first line of the constructor 3. SIGTERM self.logger == None, since the entire world stops and moves to the cleanup functions registered with the interpreter. If we don't do this check, then the cleanup context crashes in this case.
