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
DM-27118: Add support for recreating logging configuration #439
Conversation
New pipetask option for multiprocessing spawn method needs support for transporting global state from parent to child processes. Logging configuration is a part of this global state, this commit adds support for recording and replaying logging configuration changes.
# execute each one in order | ||
for call in configState: | ||
method, *args = call | ||
method(*args) |
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.
why not use functools.partial
? (binding arg vals to funcs is what it's made for)
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.
I thought about that but decided that I want to avoid extra imports and complexity. There are certainly cases when partial
is useful, but in this case it's easy to do things without it.
python/lsst/daf/butler/cli/cliLog.py
Outdated
""" | ||
if cls._initialized or cls.configState: | ||
# Already initialized, do not touch anything. | ||
log = logging.getLogger(__name__.partition(".")[2]) |
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.
In all other butler code we use __name__
and don't strip the lsst.
(I see it's actually two places because something else crept in as well)
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.
I'm trying to follow recommendations that say lsst
should be dropped: https://developer.lsst.io/stack/logging.html#logger-names. I can certainly change that to just __name__
if this is preferred.
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.
butler is special and has been for a long time -- if you drop it people will have to use lsst.daf.butler and daf.butler to turn on debug logging .
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.
OK, I'll replace that and other cases if I find any.
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.
The only other one was in core/utils.py I think
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.
Replaced 3 instances in a new commit.
New pipetask option for multiprocessing spawn method needs support for
transporting global state from parent to child processes. Logging
configuration is a part of this global state, this commit adds support
for recording and replaying logging configuration changes.