Open adriangb opened 2 years ago
Some context on what Pytest does here: it always uses the fixture with the alphabetically largest string name. So basically undefined behavior. It certainly does not automatically use the "session"
scoped fixture or anything like that.
In the end the decision was to not allow customization here and instead special case this as a prohibited usage
I think this warrants re-opening in light of https://github.com/adriangb/xpresso/pull/57
Some use cases for this:
In this case we want the dependencies to be treated as two separate unrelated entities.
One of them "exits" when the "app" scope exits and the other when the "connection" scope exists.
It would be extremely confusing if they were interchanged, even if we "picked" the outermost scope or something.
On the other hand the way it is now (prohibiting the exact same dependency) forces the user to write a bunch of boilerplate just to tell di
that they are different dependencies.
Using share=False
does not help because may want the dependencies to be shared (e.g. if the endpoint and another sub-dependency both want to add background tasks), we just don't want the the "app" scoped and "connection" scoped instances to be merged/mixed up.
In this case we have a dependency (AppState
) that we want to use both from an "app" scoped dependency and an "endpoint" scoped dependency.
What is (currently) happening there is that AppState
exists in two different DAGs (the one for lifespans and the one for the endpoint), but when the endpoint is called the cached version from the "app" scope is picked up.
This is pretty handy, it avoids the boilerplate:
class AppState(BaseModel):
started: bool = False
@asynccontextmanager
async def lifespan(state: AppState) -> AsyncIterator[None]: # implicit "app" scope
state.started = True
yield
async def healthcheck(state: Annotated[AppState, Dependant(scope="app")): # needs explicit "app" scope
return AppHealth(running=state.started)
Within a DAG, I think we can either prohibit dependencies having the same scope (current behavior) or treat them as separate dependencies (previous behavior).
The more problematic aspect is caching: should we look for the same dependency in another scope for caching? It seems like in the first case the answer is clearly "no" but in the second it is "yes".
On the other hand, identifying a dependency only by its callable is a pretty sensible and understandable thing to do. Identifying it by other things, like it's scope, muddies the waters in my opinion.
What happens when the same dependency is declared with different scopes for a given DAG? Currently they are treated as different dependencies. But customizing
DependantBase.cache_key
they can be made to be treated as the same dependency. But this cannot be changed independently of caching. We could have 2"cache keys"
, but even then what if we want to have one replace the other, with some specific business logic?