jecos opened a new pull request, #76403:
URL: https://github.com/apache/airflow/pull/76403
The sync FAB FastAPI routes leave the thread-local `settings.Session` open
on the threadpool thread that ran them. A read such as `find_user` or
`find_role` opens a transaction that nothing ends, so on PostgreSQL the backend
stays `idle in transaction` and the connection is never returned to the pool.
`cleanup_session_middleware` calls `Session.remove()`, but it runs on the
event-loop thread and never reaches the worker thread's session. A client that
polls `GET /auth/fab/v1/users/{username}` or `/roles/{name}` exhausts the API
server's pool (`QueuePool limit of size 5 overflow 10 reached`): health checks
time out, the liveness probe restarts the pod, and tasks lose the Execution API.
#72578 and #72950 fixed the collection routes by moving them to
`create_session(scoped=False)`. The single-object and write routes still go
through the security manager (`find_user`, `find_role`, `add_user`,
`update_user`, `bulk_sync_roles`, ...), which is bound to the scoped session.
Converting each of those paths would mean rewriting the security manager calls
one by one. This PR instead releases the session at the request boundary, on
the handler thread, for every route.
**Changes**
- `routes/login.py`: new `_release_scoped_session()` context manager. On
exit, it calls `settings.Session.remove()` on the current thread, and logs
instead of raising if the remove fails, so it never masks the response.
- `routes/users.py`, `routes/roles.py`, `routes/login.py`: every sync route
enters it next to `_get_flask_app().app_context()`, so the session is removed
after the response model has been built.
- `tests/.../routes/test_session_release.py`:
- tests the helper: success, exception in the body, failing `remove()`, no
`Session` configured;
- tests that `GET` / `DELETE` on `/users/{username}` and `/roles/{name}`
remove the session on the handler thread, not only on the middleware's thread.
These four route tests fail without the change.
I did not register a `teardown_appcontext` on the FAB Flask app, which would
be the Flask-native place for this. It would also run on the nested
`app_context()` blocks in `www/app.py` and `models/db.py` and could detach
objects still in use.
**Testing**
- `providers/fab`: `uv run pytest tests/unit/fab/auth_manager/api_fastapi`:
165 passed. One error in `test_parameters.py` comes from my local environment
(`ModuleNotFoundError: airflow_shared`) and is unrelated.
- The new route tests fail on `main` without the route change (4 failed, 4
passed) and pass with it.
- `ruff format` and `ruff check` on the changed files.
closes: #76402
related: #72362
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Opus 5.5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
