Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Log the arguments when running a task #24

Merged
merged 3 commits into from
Oct 3, 2016
Merged

Log the arguments when running a task #24

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
502bbfa
Log command info when running command line tasks
r-owen Oct 1, 2016
1dfcc2d
Fix flake8 warnings
r-owen Oct 3, 2016
54f122d
Use deferred formatting for all log messages
r-owen Oct 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions python/lsst/pipe/base/cmdLineTask.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from builtins import str
from builtins import object

import lsst.utils
from lsst.base import disableImplicitThreading
import lsst.afw.table as afwTable
from .task import Task, TaskError
Expand Down Expand Up @@ -464,10 +465,19 @@ def parseAndRun(cls, args=None, config=None, log=None, doReturnResults=False):
This will typically be a list of `None` unless doReturnResults is `True`;
see cls.RunnerClass (TaskRunner by default) for more information.
"""
if args is None:
commandAsStr = " ".join(sys.argv)
args = sys.argv[1:]
else:
commandAsStr = "{}{}".format(lsst.utils.get_caller_name(skip=1), tuple(args))

argumentParser = cls._makeArgumentParser()
if config is None:
config = cls.ConfigClass()
parsedCmd = argumentParser.parse_args(config=config, args=args, log=log, override=cls.applyOverrides)
# print this message after parsing the command so the log is fully configured
parsedCmd.log.info("Running: {}".format(commandAsStr))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay but maybe better parsedCmd.log.info("Running: %s", commandAsStr) as log does the % formatting (and only the % formatting at this moment...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that delayed formatting is useful primarily when a log message may not be displayed. I would expect info and fatal to always be displayed. Still...it ought to be a harmless change and I guess it is good log style.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does delayed %s formatting work for objects such as exceptions? Or must one specify a string representation of the object?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, practically it only makes some differences in debugging messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same %s formatting, just done after checking the log level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick test shows that %s does work on arbitrary objects, as does %r. I just pushed a new commit that changes logging to use deferred formatting in all instances I found in pipe_base (which were all in cmdLineTask.py and argumentParser.py)


taskRunner = cls.RunnerClass(TaskClass=cls, parsedCmd=parsedCmd, doReturnResults=doReturnResults)
resultList = taskRunner.run(parsedCmd)
return Struct(
Expand Down