Skip to content

Type the durable default of the common.compat ResumableJobMixin stand-in as bool | None - #75271

Open
dabla wants to merge 2 commits into
apache:mainfrom
dabla:fix/common-compat-resumable-job-mixin-durable-type
Open

dabla wants to merge 2 commits into
apache:mainfrom
dabla:fix/common-compat-resumable-job-mixin-durable-type

Conversation

@dabla

@dabla dabla commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #74673, addressing @amoghrajesh's post-merge review comment.

The common.compat ResumableJobMixin stand-in annotated durable: bool but defaulted it to an object() sentinel, typed Any only to keep mypy quiet. It now defaults to None:

  • The annotation tells the truth. bool | None is what the argument actually takes. bool | Any or Any would also be honest, but a union with Any switches type checking off for the argument, so durable="yes" would pass.
  • One convention for "not set". The operators using the mixin (Glue, Redshift Data, Livy, Spark Submit, Databricks, BigQuery, Snowflake) already declare durable: bool | None = None and only forward durable when it is not None. The stand-in now reads "not set" the same way.

No behaviour change: below Airflow 3.3, leaving durable out stays silent and an explicit durable=True/False still warns that it has no effect.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code (Opus 5.5) following the guidelines

…-in as bool | None

Follow-up to apache#74673, addressing the post-merge review comment
(apache#74673 (review)).

The stand-in annotated `durable: bool` but defaulted it to an `object()` sentinel that was typed `Any`
to keep mypy quiet. It now defaults to `None`, the value the operators already use for "not set"
(they only forward `durable` when it is not `None`), so the annotation is accurate and mypy keeps
checking the argument. No behaviour change: only an explicit `durable=True/False` warns below 3.3.

@amoghrajesh amoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preemptively approving with a nit.

…ixin stand-in

The no-warning test only covered durable left out, so a change back to warning on an explicit
None would have passed. It now runs with durable left out and with durable=None.
@dabla

dabla commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Thx @amoghrajesh for the review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants