-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
🐛 Fix Depends(func, scope='function') for top level (parameterless) dependencies
#14301
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
Conversation
|
Hi! 👋 I think the test logic looks a bit complicated. Can we try to simplify it? My suggestion: from typing import Any
from fastapi import APIRouter, Depends, FastAPI, HTTPException
from fastapi.testclient import TestClient
app = FastAPI()
def raise_after_yield() -> Any:
yield
raise HTTPException(status_code=503, detail="Exception after yield")
router = APIRouter()
@router.get("/")
def get_index():
return {"status": "ok"}
app.include_router(
prefix="/router-scope-function",
router=router,
dependencies=[Depends(raise_after_yield, scope="function")],
)
app.include_router(
prefix="/router-scope-request",
router=router,
dependencies=[Depends(raise_after_yield, scope="request")],
)
client = TestClient(app)
def test_router_level_dep_scope_function() -> None:
response = client.get("/router-scope-function/")
assert response.status_code == 503
assert response.json() == {"detail": "Exception after yield"}
def test_router_level_dep_scope_request() -> None:
client = TestClient(app, raise_server_exceptions=False)
response = client.get("/router-scope-request/")
assert response.status_code == 200
assert response.json() == {"status": "ok"}
def test_app_level_dep_scope_function() -> None:
app = FastAPI(dependencies=[Depends(raise_after_yield, scope="function")])
@app.get("/app-scope-function")
def get_app_scope_function():
return {"status": "ok"}
client = TestClient(app)
response = client.get("/app-scope-function")
assert response.status_code == 503
assert response.json() == {"detail": "Exception after yield"}
def test_app_level_dep_scope_request() -> None:
app = FastAPI(dependencies=[Depends(raise_after_yield, scope="request")])
@app.get("/app-scope-request")
def get_app_scope_request():
return {"status": "ok"}
client = TestClient(app, raise_server_exceptions=False)
response = client.get("/app-scope-request")
assert response.status_code == 200
assert response.json() == {"status": "ok"}Also, I think we should put these tests into existing What do you think? |
|
How about changing the title to ? |
|
I totally agree with all suggestions. I was unsure to use dependencies on include_router but seems reasonable. I’ll update the PR later this evening 😁 |
Depends(func, scope='function') for top level (parameterless) dependencies
…to test_dependency_yield_scope
YuriiMotov
left a comment
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.
LGTM!
Thanks!
tiangolo
left a comment
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.
Awesome! Great job, thank you @luzzodev! 🚀
And thanks a lot @YuriiMotov! 🙌
This will be available in the next hours in FastAPI version 0.121.1 🎉
Summary
This PR enables proper support for
scope="function"onAPIRoutedependencies by passing the dependency’s scope through toget_dependant. Previously, parameter-less dependencies created viaget_parameterless_sub_dependantignoreddepends.scope, which led to unexpected behavior for function-scoped dependencies.Motivation
Users expect
scope="function"to work as defined, even when registered at the route level. Becausedepends.scopewasn’t forwarded, function scope behaved like the default(request). This change aligns behavior with user expectations and documentation around dependency scopes.(Related: #14296)