Skip to content

Correct the retry_db_transaction docstring about provide_session - #71311

Open
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:retry-decorator-docstring
Open

Correct the retry_db_transaction docstring about provide_session#71311
1fanwang wants to merge 1 commit into
apache:mainfrom
1fanwang:retry-decorator-docstring

Conversation

@1fanwang

@1fanwang 1fanwang commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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:

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 (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) 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:

@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})
$ 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.

Docstring-only change, so there is no behavior to regress and no test added.

The docstring says the decorator should not be used with @provide_session,
but three call sites in airflow-core stack the two, and the combination is
the only way to retry a read that needs a session created for it.

The real constraint is ordering. The retry wrapper looks up the session when
it runs, so it has to sit inside @provide_session; the opposite order raises
TypeError because no session exists yet.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant