You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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)
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)
The text was updated successfully, but these errors were encountered:
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 thenworkers==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:
worker_init_expr
should (IMO) be able to set the global / task local logger, so our change would need to happen beforeworker_init_expr
(but we seem to silently ignoreworker_init_expr
withnworkers==0
which doesn't feel right)ReTestItems
which happens afterworker_init_expr
So to fully address this issue, I'd propose to
a) Respect
worker_init_expr
whennworkers==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 evaluatedc) Wrap all internal logs in ReTestItems with the original logger from b)
The text was updated successfully, but these errors were encountered: