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

Task-local logger is set with nworkers == 0, but not with nworkers > 0 #148

Open
Drvi opened this issue Feb 27, 2024 · 0 comments
Open

Task-local logger is set with nworkers == 0, but not with nworkers > 0 #148

Drvi opened this issue Feb 27, 2024 · 0 comments

Comments

@Drvi
Copy link
Collaborator

Drvi commented Feb 27, 2024

This has led to some confusion in the past, when people set a global logger and sometimes the change took effect (nworkers > 0) and sometimes it didn't (nworkers==0, we have a task-local logger which takes precedence).

We’ve been using this pattern, when the coordinator process uses with_logger(current_logger()) do ... to get around the world age issues we’ve been seeing (e.g. here). This isn't strictly a noop as it does set the task-local logger, and we only do this for the nworkers==0 case.

Ideally, there wouldn't be a difference between the two cases -- we could remove the task local logger when running tests locally. We should be careful not to violate the following while doing so:

  1. worker_init_expr should (IMO) be able to set the global / task local logger, so our change would need to happen before worker_init_expr (but we seem to silently ignore worker_init_expr with nworkers==0 which doesn't feel right)
  2. The user should be able to control the internal logging of ReTestItems which happens after worker_init_expr

So to fully address this issue, I'd propose to
a) Respect worker_init_expr when nworkers==0 (this is somewhat orthogonal, but the current state doesn't seem right)
b) Capture the current logger before worker_init_expr and remove the current logger while test items are evaluated
c) Wrap all internal logs in ReTestItems with the original logger from b)

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

No branches or pull requests

1 participant