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

Handle multiprocessing for named loggers. #34

Merged
merged 5 commits into from
May 2, 2024

Conversation

JoeZiminski
Copy link
Member

@JoeZiminski JoeZiminski commented May 1, 2024

I realised in #30 multiprocess logging was not considered. It seems that multiprocess-logging is only setup to work on the root logger. Therefore now if a named logger is requested with multiprocess logging an error is raised.

If multiprocessing is not raised, the startup logs are logged to the previously created logging (rather than root).

This adds a test to check the error is raised. I don't think any documentation is required.

@adamltyson I am not very familiar with multiprocess logging. Would it be hard to write a test for this?

@JoeZiminski JoeZiminski requested a review from adamltyson May 1, 2024 11:57
@JoeZiminski JoeZiminski changed the title Use created logger for multiprocessing setup. Handle multiprocessing for named loggers. May 1, 2024
@adamltyson
Copy link
Member

@adamltyson I am not very familiar with multiprocess logging. Would it be hard to write a test for this?

No idea TBH.

@adamltyson
Copy link
Member

@JoeZiminski multiprocessing logging is only used in a few cases (and isn't supported on Windows). I'm happy for this to be merged as is.

@adamltyson adamltyson merged commit 6372af5 into master May 2, 2024
19 checks passed
@adamltyson adamltyson deleted the fix_multiprocessing_setup branch May 2, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants