search

databio / pypiper

Python toolkit for building restartable pipelines
http://pypiper.databio.org
BSD 2-Clause "Simplified" License
45 stars 9 forks source link

Check that logfile for manager's logger doesn't collide with manager's own logfile #215

Closed vreuter closed 6 months ago

vreuter commented 6 months ago

Close #212 Close #211

vreuter commented 6 months ago

IDK why lint action is failing; the 2 files it alleges in the logs that would be reformatted haven't changed since the last run, for version 0.14 release. Maybe something w/ black's Github Action changed?

When I just run black . locally, all's good.

donaldcampbelljr commented 6 months ago

Try upgrading your local black. I had this issue happen to me a couple of weeks ago and I seem to remember that upgrading caused the file to be formatted differently (and finally matched the github action).

vreuter commented 6 months ago

Try upgrading your local black. I had this issue happen to me a couple of weeks ago and I seem to remember that upgrading caused the file to be formatted differently (and finally matched the github action).

TY @donaldcampbelljr , indeed that works!

vreuter commented 6 months ago

@nsheff if you review this also, note that the only real changes are to tests and to manager.py; the other files in the diff are formatter-related.

vreuter commented 6 months ago

@donaldcampbelljr OK to merge to master, or you want me to switch the target to dev?

donaldcampbelljr commented 6 months ago

Oh lets do dev for now, please.

nsheff commented 6 months ago

Just a broader question: should we just disable the logmuse handle to change the log files in the first place?

I mean, there are no logs coming out of pypiper that logmuse should be touching since it has its own logging system, right?

Or did I miss something?

vreuter commented 6 months ago

there are no logs coming out of pypiper that logmuse should be touching since it has its own logging system, right?

@nsheff sorry I don't understand this, what do you mean here exactly? What do you mean by "coming out of pyiper" and "should be touching"?

vreuter commented 6 months ago

Oh lets do dev for now, please.

👌 , done :)

vreuter commented 6 months ago

Just a broader question: should we just disable the logmuse handle to change the log files in the first place?

I mean, there are no logs coming out of pypiper that logmuse should be touching since it has its own logging system, right?

Or did I miss something?

A couple reasons that come to mind to have a separate one... a) If I write a manager subclass, I can use the logger for things about the manager while reserving the pipeline log, conceptually, for things about the pipeline b) I can get a logfile that's much less verbose, e.g. setting the level for the logger's logfile to warning, while the pipeline log will tend to get many more messages.

nsheff commented 6 months ago

To me, I think there's really only one set of logs: the logs from the pipeline. Everything that should be output by a PipelineManager is the pipeline's logs.

So, I don't see these as separate, and so I've never used logmuse to configure logs on something that uses pypiper, and that's why this has never been a problem.

Do you see an actual use case where you want some logs to come from the Manager and some other logs to come from the Pipeline? I can't imagine such a situation, but if you think you would use it that way, then this is fine with me! I think I view the Manager object as somehow synonymous with the pipeline itself, so I don't understand why someone would want that level of control.

vreuter commented 6 months ago

Do you see an actual use case where you want some logs to come from the Manager and some other logs to come from the Pipeline? I can't imagine such a situation, but if you think you would use it that way, then this is fine with me! I think I view the Manager object as somehow synonymous with the pipeline itself, so I don't understand why someone would want that level of control.

I sort of see both sides. Personally, I can definitely see using the logfile for the manager's logger to output more technical details and/or using it for only more severe messages, so having the flexibility both in terms of verbosity (e.g., I may want to go through a logfile that has nothing from any of the 3rd-party tools my pipeline may be using, and maybe I don't want to look at certain types of noisy numpy warnings) and in terms of nature/subject of message (e.g., mechanics of pipeline to logger, actual bioinformatics work to to pypiper log).

But I understand the appeal of the simplicity of the single log destination, too. If starting from scratch, I'd probably be inclined to favor the single logfile in fact. But since the logger's in place, I'd like to keep it for now. We could revisit as a future refactor the consolidation/merging of logging destinations.

@nsheff would you like me to open a ticket with that as a refactor idea?