-
-
Notifications
You must be signed in to change notification settings - Fork 951
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
Try to propagate contexvars from lifespan to endpoints #1257
Conversation
@graingert this is in reference to adriangb/di#1 (comment)
Working now |
starlette/routing.py
Outdated
if self.user_ctx is not None: | ||
current_ctx = contextvars.copy_context() | ||
for cvar in self.user_ctx: | ||
if cvar in current_ctx: | ||
# not set by the user, skip to avoid modifying | ||
# vars used by event loops, servers, etc. | ||
continue | ||
try: | ||
if cvar.get() != self.user_ctx.get(cvar): | ||
cvar.set(self.user_ctx.get(cvar)) | ||
except LookupError: | ||
cvar.set(self.user_ctx.get(cvar)) |
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.
I'm pretty sure this sort of copying back the context can be used to allow things run by run_in_threadpool
to behave like if they were being called directly (which would solve issues FastAPI has with sync dependencies)
try: | ||
from contextvars import Context, ContextVar, copy_context | ||
except ImportError: # pragma: no cover | ||
Context = object # type: ignore | ||
ContextVar = copy_context = None # type: ignore |
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.
This module can be cleaned up better. I was trying to encapsulate all of the try: import contexvars
checks into one module
The goal is that, from a users perspective, it endpoints should be run as if they were a task being executed from within the `yield of a lifespan.