-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Support Stimulus ID's in Scheduler with ContextVars #6046
Conversation
Unit Test Results 18 files ±0 18 suites ±0 9h 27m 19s ⏱️ + 32m 12s For more details on these failures, see this check. Results for commit d0aa434. ± Comparison against base commit bddc3f2. ♻️ This comment has been updated with latest results. |
.take |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things appeared to be a bit more complex than I anticipated and I became a bit nervous of whether contextvars are the right approach. I think there should only be one global thing (I might be wrong) and I went on implementing something on top of this PR, see #6068
We should sync up and align what the next steps are
I'm a bit swarmed right now and can't keep up with the review process - I'm leaving it to @fjetter |
I've changed the ContextVar to a single global variable, rather than a per Scheduler instance ContextVar. I don't think this changes the underlying functionality much, as the ContextVar is only used internally in the Scheduler at present. I think it would be great to to do a follow up PR that extends this to Client and Workers. |
Introduces too much indentation into the diff. Closing in favour of #6083 |
pre-commit run --all-files