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

Add Starlette lifespan handler implementation #683

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ZipFile
Copy link
Contributor

@ZipFile ZipFile commented Mar 21, 2023

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

from dependency_injector.containers import DeclarativeContainer
from dependency_injector.ext.starlette import Lifespan
from dependency_injector.providers import Factory, Resource, Self, Singleton
from fastapi import FastAPI


def init():
  print("Inittializing resources")
  yield
  print("Cleaning up resources")


class Container(DeclarativeContainer):
    __self__ = Self()
    lifespan = Singleton(Lifespan, __self__)
    app = Factory(FastAPI, lifespan=lifespan)
    init = Resource(init)

myapp/asgi.py:

from .container import Container

container = Container()
app = container.app()

run.sh:

uvicorn myapp.asgi:app

@ZipFile ZipFile force-pushed the lifespan branch 2 times, most recently from 1275379 to c1c65b1 Compare August 12, 2024 12:43
@ZipFile
Copy link
Contributor Author

ZipFile commented Aug 12, 2024

Looks like project is still alive. Rebased with latest master.


if result is not None:
await result

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__

Copy link
Contributor Author

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 returned 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:

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]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This method will be inferred as def __aenter__(self) -> Awaitable[None]:
  2. -> Optional[Awaitable] means function returns either None or something that you can await (e.g. signature of the init_resources). Given the pt.1, annotating it with your suggestion will result in Awaitable[Optional[Awaitable]], which does not conforms to StatelessLifespan protocol.

@VladyslavHl
Copy link

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 __self__ to init all resources of container as it is implemented now.

This leads to question how to not initialize these resources when calling container.init_resources(), maybe the its sufficient to just init them explicily by calling container.other_resource.init() when needed.

maybe I'm complicating, let me know wdyt

@ZipFile
Copy link
Contributor Author

ZipFile commented Dec 2, 2024

@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 Lifespan the way you proposed. My implementation is intentionally dumb.

Also, take a look at ContextLocalSingleton. This is not a 100% replacement for resources, but might help in certain situations.

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

Successfully merging this pull request may close these issues.

3 participants