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.

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

logger_kwargs isn't used in one branch of the pipeline manager setup #211

Closed
vreuter opened this issue Feb 17, 2024 · 4 comments · Fixed by #215
Closed

logger_kwargs isn't used in one branch of the pipeline manager setup #211

vreuter opened this issue Feb 17, 2024 · 4 comments · Fixed by #215

Comments

@vreuter
Copy link
Member

vreuter commented Feb 17, 2024

In the pipeline manager constructor, it looks like logger_kwargs isn't used when args is non-empty or non-null. Is this intended? There's also no docstring for logger_kwargs, where it could be noted if this is indeed intended behavior, or at least describing what these values are to be used for.

pypiper/pypiper/manager.py

Lines 216 to 232 in 3a8465a

if not args:
# strict is only for logger_via_cli.
kwds = {k: v for k, v in logger_kwargs.items() if k != "strict"}
try:
name = kwds.pop("name")
except KeyError:
name = default_logname
self._logger = logmuse.init_logger(name, **kwds)
self.debug("Logger set with logmuse.init_logger")
else:
logger_kwargs.setdefault("name", default_logname)
try:
self._logger = logmuse.logger_via_cli(args)
self.debug("Logger set with logmuse.logger_via_cli")
except logmuse.est.AbsentOptionException:
self._logger = logmuse.init_logger("pypiper", level="DEBUG")
self.debug("logger_via_cli failed; Logger set with logmuse.init_logger")

Possibly related to #210

@vreuter
Copy link
Member Author

vreuter commented Feb 17, 2024

Perhaps **logger_kwargs should be passed to logmuse.init_logger?

@vreuter
Copy link
Member Author

vreuter commented Feb 17, 2024

^^ controlling, perhaps, for possibility of collision w/ level (i.e., either remove level from the keyword args mapping if it's there--noting difference w/ DEBUG if that's the case--or not passing level='DEBUG' if level's already in the kwargs mapping.

@nsheff
Copy link
Member

nsheff commented Feb 17, 2024

Hmm, I never understood what logmuse was doing with init_logger and I still don't :).

you taught me everything I know about the logging package

@vreuter
Copy link
Member Author

vreuter commented May 8, 2024

Closed by #215

@vreuter vreuter closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants