-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Add Starlette lifespan handler implementation #683
base: master
Are you sure you want to change the base?
Conversation
1275379
to
c1c65b1
Compare
Looks like project is still alive. Rebased with latest |
|
||
if result is not None: | ||
await result | ||
|
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.
Hi!
Thanks for the nice change. Perhaps it would be appropriate to add an explicit return None
at the end of __aenter__
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.
You generally want explicit return None
at the end if you return
ed something earlier in the function. There are no returns at all here, so this is extra.
def __call__(self, app: Any) -> Self: | ||
return self | ||
|
||
async def __aenter__(self) -> None: |
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.
Here you need to change the return type probably to Optional[Awaitable]
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 method will be inferred as
def __aenter__(self) -> Awaitable[None]:
-> Optional[Awaitable]
means function returns eitherNone
or something that you canawait
(e.g. signature of theinit_resources
). Given the pt.1, annotating it with your suggestion will result inAwaitable[Optional[Awaitable]]
, which does not conforms to StatelessLifespan protocol.
Great! Integrating DI Resources with FastAPI lifespan is something me and colleagues will investigate soon, good to see the potential of universal solution. IMO it would be benefitial to make this feature more flexible, allowing user to chose what resources to initialize in lifespan as you may want to init some resources at other moment From the top of my head Lifespan can take individual resources to be initialized/shutdown on lifespan events, so the usage could be: class Container(DeclarativeContainer):
init = Resource(init)
other_resource = Resource(init_other_resource) # to be intialized not on startup event
lifespan = Singleton(Lifespan, init, ... )
app = Factory(FastAPI, lifespan=lifespan)
... also preserving the option to pass This leads to question how to not initialize these resources when calling maybe I'm complicating, let me know wdyt |
@VladyslavHl I guess your problem is much more general and your proposal will only solve it partially if at all. From my experience using DI, it lacks notion of scopes for resources. Like "app", "request", "task", etc... I've solved the problem by doing scopes explicitly, i.e. injecting context manager that will initialize resources (e.g. db sessions) and storing them into contextvars. Not exactly great solution, but works when you have mix of different "apps" in your project (fastapi, typer, celery, alembic, etc...) but still need common setup code to run (configuring logging, apms, service discovery, etc...). Introducing scopes would probably require significant re-engeneering of how DI itself works, so for the time being my advice for you would be to implement your own Also, take a look at ContextLocalSingleton. This is not a 100% replacement for resources, but might help in certain situations. |
Starlette recently added cool new feature called lifespan in v0.26.0 and FastAPI followed the suit in v0.93.0. This makes it possible to get rid of good chunk of initialization spaghetti in starlette-based apps (making container a single entry point in the application), + reduces amount of the pain in the ass scheduling background tasks in the event loop of the ASGI app.
Example
myapp/container.py
myapp/asgi.py
:run.sh
: