Re: [PR] fix(providers/fab): add session cleanup middleware to FAB FastAPI app [airflow]

2026-02-04 Thread via GitHub 


boring-cyborg[bot] commented on PR #61480:
URL: https://github.com/apache/airflow/pull/61480#issuecomment-3850920128

   Congratulations on your first Pull Request and welcome to the Apache Airflow 
community! If you have any issues or are unsure about any anything please check 
our Contributors' Guide 
(https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (ruff, mypy and type 
annotations). Our [prek-hooks]( 
https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks)
 will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in 
`docs/` directory). Adding a new operator? Check this short 
[guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst)
 Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze 
environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst)
 for testing locally, it's a heavy docker but it ships with a working Airflow 
and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get 
the final approval from Committers.
   - Please follow [ASF Code of 
Conduct](https://www.apache.org/foundation/policies/conduct) for all 
communication including (but not limited to) comments on Pull Requests, Mailing 
list and Slack.
   - Be sure to read the [Airflow Coding style]( 
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
   - Always keep your Pull Requests rebased, otherwise your build might fail 
due to changes not related to your commits.
   Apache Airflow is a community-driven project and together we are making it 
better 🚀.
   In case of doubts contact the developers at:
   Mailing List: [email protected]
   Slack: https://s.apache.org/airflow-slack
   


-- 
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]

[PR] fix(providers/fab): add session cleanup middleware to FAB FastAPI app [airflow]

2026-02-04 Thread via GitHub 


yulit0738 opened a new pull request, #61480:
URL: https://github.com/apache/airflow/pull/61480

   ## Summary
   Fix `PendingRollbackError` on FAB admin pages (`/auth/users/list/`, 
`/auth/roles/list/`) by adding session cleanup middleware to the FAB auth 
manager's FastAPI app.
   ## Problem
   FAB auth manager's FastAPI app has this route structure:
   - `/token`, `/logout` → FastAPI routes
   - `/users/*`, `/roles/*` → FastAPI API routes
   - `/*` (catch-all) → `WSGIMiddleware` → Flask App (FAB views)
   The **Flask AppBuilder views** (e.g., `/users/list/`, `/roles/list/`) use 
`settings.Session` (SQLAlchemy `scoped_session`) for database access. In a 
native Flask app, `teardown_appcontext` automatically calls `Session.remove()` 
after each request. However, when Flask is mounted via `WSGIMiddleware` inside 
FastAPI, **Flask's teardown hooks do not trigger**.
   This causes sessions to remain in **"idle in transaction"** state. When the 
database connection times out (e.g., PostgreSQL's 
`idle_in_transaction_session_timeout`), subsequent requests that reuse the 
invalidated session raise `PendingRollbackError` (500 error).
   ### Steps to reproduce
   1. Deploy Airflow 3.x with FAB auth manager and PostgreSQL
   2. Access `/auth/users/list/` or `/auth/roles/list/`
   3. Wait for PostgreSQL to timeout idle transactions
   4. Refresh the page → **500 PendingRollbackError**
   ## Solution
   Add a FastAPI HTTP middleware in `get_fastapi_app()` that calls 
`Session.remove()` in the `finally` block after every request. This ensures 
session cleanup for **all** requests, including those forwarded to Flask via 
WSGI — closing the gap left by the missing `teardown_appcontext`.
   ### Why this is safe
   - `settings.Session` is a `scoped_session` (thread-local) — `remove()` only 
affects the current thread
   - The `finally` block runs after the response is fully constructed, so it 
doesn't interfere with request processing
   - All 585 existing FAB provider tests pass with this change
   ## Testing
   - Added `TestFabAuthManagerSessionCleanup` with 2 tests:
 - `test_session_cleanup_middleware_on_wsgi_route`: Verifies 
`Session.remove()` is called after WSGI (Flask) route requests — the exact 
scenario that caused the bug
 - `test_session_cleanup_middleware_on_fastapi_route`: Verifies cleanup 
also runs after FastAPI route requests
   - Verified fix resolves the issue in a production environment with 
PostgreSQL (idle transactions drop to 0 after patch)
   ## AI Disclosure
   This PR was developed with AI assistance (Claude).


-- 
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]