-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
orchestra/console: log output from pexpect commands #1869
Conversation
@zmc, please have a look and let's discuss the approach. I'm not fully comfortable with it but it might be the best we can really do |
example output (new log lines have 'output:' in them):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmick I think using the self.logfile
instance variable like this could introduce races in concurrent execution. What I'd change about this approach would be to drop self.logfile
entirely from PhysicalConsole
, and logfile
from its __init__
method. Since you're already setting the logfile_read
on the pexpect.spawn
object, the method calls that produce one can just access that same attr from the returned object. Calling seek(0)
will still be necessary, but we'd be guaranteed that each pexpect
session has its own buffer to use.
probably a good idea. I don't think we currently have overlapping spawns, but I did ponder that as a potential problem. I couldn't figure out any good way to "tie" the pexpect logfile option directly to a logger; I looked into subclassing files to add in a call to log.debug but I just couldn't puzzle it out. |
Ah, look at |
Repushed with suggested changes. |
jenkins retest please |
#1870 aims to fix the CI |
in case anything weird is being noticed and communicated by ipmitool, try to display anything it says Signed-off-by: Dan Mick <[email protected]>
rebased on main after merging #1870, tests pass now |
in case anything weird is being noticed and communicated by ipmitool, try to display anything it says