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-31801: Use lsst prefix for loggers #143
Conversation
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 a bit unclear about the scope of this work, but have no serious objections to the changes.
python/lsst/ap/verify/ingestion.py
Outdated
@@ -48,6 +48,8 @@ | |||
from lsst.pipe.tasks.ingestCalibs import IngestCalibsTask | |||
from lsst.pipe.tasks.ingestCuratedCalibs import IngestCuratedCalibsTask | |||
|
|||
_LOG = lsst.log.Log.getLogger(__name__) |
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.
Do we need this boilerplate in every module? I'm a bit suspicious of global variables...
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.
Where else would you put it? We use this form all the time in middleware.
Now forward lsst.log to Python logging.
@kfindeisen I switched it to python logging. I'm not sure what default log configuration you'd like but I use INFO and log messages to stdout. |
The scope of this work is to change the logger names but there is also a general requirement to switch all python code over to python logging where practical. Since I was already in the files I thought I may as well do that on this ticket. |
I think in order to switch to python logging we'll have to add a logging.basicConfig and lsst.log forwarding in here somewhere.