The sut_tools could use a more involved logging system to replace "print" commands.

what the title of the attachment says.

Comment on attachment 665587 [details] [diff] [review] update sut_tools to use log.* functions instead of print (1.0) Review of attachment 665587 [details] [diff] [review]: ----------------------------------------------------------------- ::: sut_tools/sut_lib.py @@ +23,5 @@ > import json > > +def getSUTLogger(filename=None, loggername=None): > + if not filename: > + filename = "sut_tools.log" If nothing else, I need/want a way to not have the commands run log to a file, for when buildbot calls stuff. I know you're not great at logging stuff, and I'm not a fan of this sut_lib code, I'll see if I can make a pass at this building upon your log.* changes elsewhere, unless you think you can cleanup at the least the filename stuff in a way we can try to land this and get the improvement earlier.

update to not default to a file.

Comment on attachment 666584 [details] [diff] [review] update sut_tools to use log.* functions instead of print (1.1) Review of attachment 666584 [details] [diff] [review]: ----------------------------------------------------------------- ::: sut_tools/reboot.py @@ +10,5 @@ > > +def reboot(dm): > + cwd = os.getcwd() > + proxyFile = os.path.join(cwd, '..', 'proxy.flg') > + errorFile = os.path.join(cwd, '..', 'error.flg') warning this still has a pretty strict requirement on what the cwd is for the flags to be useful outside of this file -- but it relies on this in both the before and after of your patch, so just an observation @@ +28,5 @@ > + waitForDevice(dm, waitTime=600) > + except SystemExit: > + clearFlag(proxyFile) > + setFlag(errorFile, "Remote Device Error: call for device reboot failed") > + sys.exit(1) not required, but we might as well move this sys.exit() to be a return 1 and handled like other sut_* scripts are these days. ::: sut_tools/sut_lib.py @@ +39,5 @@ > + datefmt='%m/%d/%Y %H:%M:%S') > + else: > + logging.basicConfig(level=logging.DEBUG, > + filename=filename, > + filemode="a", nit: lets use **kwargs magic here. So something like: extra_kwagrs = {} if filename: extra_kwargs['filename'] = filename extra_kwargs['filemode'] = "a" logging.basicConfig(... **extra_kwargs) And we do want stream output in both the file and the default logging.

updated to include review nits.

Hrm we broke some here: + for i in '${tegras}' + '[' -d tegra-013 ']' + cd tegra-013 + '[' '!' -e clientproxy.pid ']' + python clientproxy.py -b --tegra=tegra-013 --debug No handlers could be found for logger "multiprocessing" + sleep 60 Though it does seem to still be logging, so we might just be missing some early logging related to that.