Add FromLifespan to extract dependencies from the lifespan state #9299
Conversation
| @@ -0,0 +1,42 @@ | |||
| from inspect import Parameter | |||
There was a problem hiding this comment.
I wasn't sure where to put this, module, I leave that up to you
fastapi/dependencies/_aliases.py
Outdated
| class _FromLifespan: | ||
| def __init__(self, param: Parameter) -> None: | ||
| self._type = get_type(param) | ||
| self._key: Optional[str] = None |
There was a problem hiding this comment.
One interesting thing about this API is that there might be some type information available from the lifepsan itself. Should FastAPI introspect that? Should it force users to return something typed? Would that help prevent mistakes at startup, e.g. where the user requests a type that is not available or is maybe ambiguous?
| def get_type(param: Parameter) -> Type[Any]: | ||
| annotation = param.annotation | ||
| while get_origin(annotation) is Annotated: | ||
| annotation = get_args(annotation)[0] | ||
| return annotation # type: ignore[no-any-return] |
There was a problem hiding this comment.
There probably is (or maybe could be) a utility function somewhere else that does this and other manipulation of Annotated.
|
📝 Docs preview for commit 7f2bf1e at: https://641be2efb8c906675eb00d9c--fastapi.netlify.app |
|
📝 Docs preview for commit 84072a1 at: https://641beb0212105f67c02db483--fastapi.netlify.app |
| class DependsContext: | ||
| def __init__(self, param: Parameter) -> None: | ||
| self.param = param |
There was a problem hiding this comment.
I'm wrapping the single item in a custom class so that in the future we can add more stuff here in a backwards compatible manner. E.g. I can imagine a world where a dependency wants to know what path it is registered under, what dependencies depend on it, etc.
|
📝 Docs preview for commit 4fc3b45 at: https://641c539c8b423f0977b3ab47--fastapi.netlify.app |
|
I'm failing to see how all those issues are related to this. 🤔 Don't we want a dependency that can be accessed on the lifespan itself instead? Like the "app" scope that you have on xpresso. If I do the following, isn't it the same as what this PR proposes? from fastapi import Depends, FastAPI, Request
from contextlib import asynccontexmanager
from typing import Annotation
@asynccontextmanager
async def lifespan(app: FastAPI) -> dict:
async with connect() as db:
yield {"db": db}
app = FastAPI(lifespan=lifespan)
async def get_connection(request: Request):
yield request.state.db
@app.get("/")
async def homepage(db: Annotation[Session, Depends(get_connection)]):
... |
|
Yes, it's the same thing. This is just making it more seamless. It’s the same with the rest of the dependency injection system: you could just not use it and store stuff in Minor nit: it would be `request[“state”][“db”] I think. |
| @asynccontextmanager | ||
| async def lifespan(app: FastAPI) -> AsyncIterator[Dict[str, Any]]: | ||
| async with connect(42) as db: | ||
| yield {"db": db} |
There was a problem hiding this comment.
I'm not a big fan of this design as a solution to lifespan dependency, it's really just syntactic sugar over the lifespan state like this.
The value of dependencies is the ability to inject dependencies in a more distributed way. For example I might have a lifespan dependency that's only applicable to one group of API routes so I want to define it as a lifespan dependency in that context rather than having to define it on a lifespan passed directly into the main FastAPI application.
There was a problem hiding this comment.
I think there’s actually quite a bit of value in having users manually wire the dependencies in the lifespan. It mirrors the “composition root” concept in most standalone dependency injection systems. It’s a singular well identified place where you tell the dependency injection system how to build your dependencies.
This keeps the execution model clear and simple. I’ve implemented the more “distributed” version in Xpresso and it can get messy.
But thank you for the feedback. This is just my opinion and I could be swayed.
|
@adriangb: I'll echo what Kludex and Chippiewill are saying here, I'm not sure that's the... FastAPI-tonic?? ...way of handling lifetime-scoped DI. I get the benefits you're bringing up with regards to a "composition root" as opposed to dependencies being spread out everywhere, but consider how all of FastAPI is designed around being spread out like that, on account of using routing annotations instead of a single routing table. Single roots make sense for Starlette, but I'm not so sure it's the path I'd want FastAPI to go down if we want the experience to be consistant. My suggestion in #617 was more or less exactly what you ended up implementing in xpresso, with the intended goal that I could have an Oracle connection pool, a Postgres connection pool, an HTTPX connection pool and a... anything that isn't a connection pool, all declared in either the same or different files as needed, without the need for a single lifespan function to tie them together when initializing my FastAPI app, making their usage mostly seamless when compared to current dependencies. |
|
Thanks @sm-Fifteen the feedback is helpful. I’ll try to cook up a version of this that uses that approach. |
This adds a
FromLifespanannotation that extracts dependencies from lifespans, which I believe satisfies the requests in #617, #9215, part of #3641 and probably several other issues.It also adds what is essentially an extractor system that allows dependencies to get a view into their parameter; this could effectively allow replacing a lot of
fastapi/dependencies/utils.pywith implementations for each type of parameter, making things like #2077 a lot easier to implement and opening up the door for 3rd party extensions to FastAPI's dependency / extractor system. This does the rest of #3641.I purposefully added as single docs-style test to demonstrate how this is used and in-lieu of writing docs; I'll do that once / if there's some initial feedback and buy in.