kaxil commented on code in PR #65179:
URL: https://github.com/apache/airflow/pull/65179#discussion_r3076815679
##########
airflow-core/tests/unit/api_fastapi/core_api/test_app.py:
##########
@@ -93,9 +93,13 @@ def
test_no_streaming_endpoint_uses_function_scoped_depends(self):
assert not violations, (
"Streaming endpoints must not use function-scoped dependencies
like "
- "SessionDep. Use Annotated[Session, Depends(_get_session)]
(default "
- "request scope) instead — function-scoped cleanup runs before the "
- "response body is streamed, leaking database connections.\n"
+ "SessionDep — function-scoped cleanup runs before the response
body "
Review Comment:
The error message now says "Do NOT use `Annotated[Session,
Depends(_get_session)]` or other session dependencies either", but the test
only checks `metadata.scope == "function"`. A `Depends(_get_session)` without
an explicit scope has `scope=None`, so it would pass the test silently. Since
the guidance is now "no injected session at all on streaming endpoints", the
check could also flag any `Session`-typed `Depends` regardless of scope.
Not a blocker since the implementation itself is correct, but the test
doesn't fully enforce what the error message promises.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
