1fanwang opened a new pull request, #71311: URL: https://github.com/apache/airflow/pull/71311
`retry_db_transaction`'s docstring says: > It should not be used with `@provide_session`. Three call sites in `airflow-core` do exactly that, and have for years: - [`renderedtifields.py:241`](https://github.com/apache/airflow/blob/274011b437/airflow-core/src/airflow/models/renderedtifields.py#L241) - [`dagwarning.py:78`](https://github.com/apache/airflow/blob/274011b437/airflow-core/src/airflow/models/dagwarning.py#L78) - [`manager.py:701`](https://github.com/apache/airflow/blob/274011b437/airflow-core/src/airflow/dag_processing/manager.py#L701) ## Why it matters The guidance is the opposite of what the code requires, so a contributor who follows it either avoids a valid pattern or stacks the decorators the one way that breaks. That combination is also the only way to retry a read whose session is created for it, which is what those three call sites need. The wording has been there since the decorator landed in [#14109](https://github.com/apache/airflow/pull/14109) (2021). ## What the constraint actually is Ordering, not incompatibility. The retry wrapper resolves `session` from the call's arguments when it runs ([`retries.py:91-96`](https://github.com/apache/airflow/blob/274011b437/airflow-core/src/airflow/utils/retries.py#L91-L96)) and rolls it back between attempts, so the session must already be bound. `@provide_session` therefore goes on the outside. The docstring now says that and shows the shape. ## Testing Done Both orderings, against a real SQLAlchemy session, on a clean checkout of `main`: ```python @provide_session @retry_db_transaction def provide_session_outer(value: int, *, session: Session = NEW_SESSION) -> int: return session.scalar(text("SELECT :v"), {"v": value}) @retry_db_transaction @provide_session def retry_outer(value: int, *, session: Session = NEW_SESSION) -> int: return session.scalar(text("SELECT :v"), {"v": value}) ``` ```console $ python verify_decorator_order.py @provide_session OUTER, @retry_db_transaction INNER -> OK, returned 7 @retry_db_transaction OUTER, @provide_session INNER -> TypeError: session is a required argument for retry_outer ``` The documented order works and returns the queried value. The order the old docstring implied fails at call time with the `TypeError` raised at [`retries.py:96`](https://github.com/apache/airflow/blob/274011b437/airflow-core/src/airflow/utils/retries.py#L96). Docstring-only change, so there is no behavior to regress and no test added. -- 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]
