Skip to content

Docs: clarify Databricks retry_delay uses exponential backoff - #68622

Merged
potiuk merged 3 commits into
apache:mainfrom
sduvvuri26:docs/clarify-databricks-retry-params
Jul 30, 2026
Merged

Docs: clarify Databricks retry_delay uses exponential backoff#68622
potiuk merged 3 commits into
apache:mainfrom
sduvvuri26:docs/clarify-databricks-retry-params

Conversation

@sduvvuri26

@sduvvuri26 sduvvuri26 commented Jun 16, 2026

Copy link
Copy Markdown

Summary

Clarifies databricks_retry_delay / retry_delay documentation for Databricks operators, hooks, and triggers that use BaseDatabricksHook.

The previous docs described a fixed number of seconds between retries. The default implementation uses exponential backoff with retry_delay as the minimum wait (wait_exponential in BaseDatabricksHook).

Note: databricks_retry_args can override the default wait strategy; this documents the default behavior only.

Test plan

  • Doc-only change (docstrings + repos operator RST tables)
  • Verified retry behavior manually with DatabricksSubmitRunOperator and a broken connection (retry_delay=1 and retry_delay=20)

Manual verification

Observed retry timing with DatabricksSubmitRunOperator, broken connection, databricks_retry_limit=10:

retry_delay Observed waits between attempts
1 1s, 2s, 4s, 8s, 16s, 32s, …
20 20s × 5, then 32s, 64s, 128s, …

This matches wait_exponential(min=retry_delay, max=2**retry_limit) in BaseDatabricksHook.


Was generative AI tooling used to co-author this PR?
  • Yes — Cursor

Generated-by: Cursor following the guidelines


Important

🛠️ Maintainer triage note for @sduvvuri26 · by @potiuk · 2026-06-17 18:53 UTC

Some review feedback from jroachgolf84 is waiting on you:

The ball is in your court — you've been assigned to this PR. Reply or push a fix in each thread, then mark them resolved.

Automated triage — may be imperfect; a maintainer takes the next look.

Document that retry_delay is the minimum wait between retryable attempts when using the default exponential backoff strategy.
@boring-cyborg

boring-cyborg Bot commented Jun 16, 2026

Copy link
Copy Markdown

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
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our 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 Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • 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: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@eladkal
eladkal requested a review from jroachgolf84 June 16, 2026 16:02

@SameerMesiah97 SameerMesiah97 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.

Just did a quick search in the databricks provider and it appears you have covered all relevant docstring entries. I would check once more before merge just in case any were missed.

@sduvvuri26

Copy link
Copy Markdown
Author

Just did a quick search in the databricks provider and it appears you have covered all relevant docstring entries. I would check once more before merge just in case any were missed.

@SameerMesiah97 Thanks for the review. Re-searched the databricks provider for the old retry_delay wording ("Number of seconds to wait between retries") and found no remaining entries. All relevant docstrings/RST tables appear covered.

@jroachgolf84 jroachgolf84 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed all files, LGTM.

@potiuk

potiuk commented Jun 25, 2026

Copy link
Copy Markdown
Member

@sduvvuri26 — you've pushed commits since the changes-requested review, but the review threads are still open. If you've addressed the feedback, reply to confirm and nudge the reviewer for another look; otherwise let us know what's still outstanding. Thanks!

See the PR quality criteria.

Automated first-pass triage note drafted by an AI-assisted tool — may get things wrong; once addressed, a real Apache Airflow maintainer takes the next look. (why automated)


Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting

@sduvvuri26

Copy link
Copy Markdown
Author

@potiuk, I’ve addressed the feedback, resolved the review threads, and re-requested review. @jroachgolf84 has reviewed and approved the PR on Jun 20.
Please let me know if anything else is needed from my side.

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 2, 2026
@eladkal

eladkal commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

cc @moomindani for Databricks team review

@moomindani moomindani 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.

LGTM. Verified the claim rather than just the wording.

BaseDatabricksHook builds its default retry with wait_exponential(min=self.retry_delay, max=(2 ** retry_limit)), so the old "number of seconds between retries" was simply wrong. I reproduced your table against tenacity directly and it matches, including the counter-intuitive retry_delay=20 shape (20, 20, 20, 20, 20, 32, 64, 128 — the 2**(n-1) series clipped at the floor until it overtakes it) and the 2 ** retry_limit cap.

On completeness, which was the open question earlier: current main has 17 occurrences of the old phrasing across 8 files, and this PR updates all 8 with matching per-file counts. Nothing left behind — the remaining databricks_retry_delay: decimal lines are the type column, not stale prose.

Worth a rebase before merge; the branch is ~1.5 months behind main, though I confirmed no new retry_delay docstrings landed in the meantime, so nothing is newly stale.


Drafted-by: Claude Code (Opus 5)

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Docs and docstrings only, and the claim matches the implementation — databricks_base.py builds wait_exponential(min=self.retry_delay, max=2 ** retry_limit), so "minimum wait, exponential backoff, capped at 2 ** retry_limit" is accurate. Thanks.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit ee8bb6d into apache:main Jul 30, 2026
80 of 81 checks passed
@boring-cyborg

boring-cyborg Bot commented Jul 30, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

potiuk added a commit that referenced this pull request Jul 30, 2026
…koff (#68622) (#70729)

(cherry picked from commit ee8bb6d)

Co-authored-by: sduvvuri26 <42085381+sduvvuri26@users.noreply.github.com>
potiuk added a commit that referenced this pull request Jul 30, 2026
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.

6 participants