Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry param in databrics async operator #30744

Merged
merged 2 commits into from
Apr 23, 2023

Conversation

pankajastro
Copy link
Member

@pankajastro pankajastro commented Apr 19, 2023

currently, we do not set the tries parameter in Databricks
while initialising the hook from the trigger. This is problematic
if the Databricks API throw error in the trigger and the error is retriable.
In this case, currently, we have a retry limit and the delay is fixed.
This PR allows the user to modify the retry limit and the delay as required.
In addition, I'm also pushing the job id in xcom for DatabricksSubmitRunDeferrableOperator
so that other tasks can use the existing job_id from xcom if require.
also, add logs while waiting for completion


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@pankajastro pankajastro force-pushed the add_retry_databricks_async_op branch 3 times, most recently from 5d6ff51 to 7a8b92a Compare April 20, 2023 08:12
@pankajastro pankajastro marked this pull request as ready for review April 20, 2023 09:38
@pankajastro pankajastro force-pushed the add_retry_databricks_async_op branch from 7a8b92a to 2ad5d17 Compare April 20, 2023 11:08
@potiuk
Copy link
Member

potiuk commented Apr 22, 2023

@alexott - WDYT? It looks good to me

Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

let unify retry logic in sync & async versions.

Also, was it tested on the real Databricks instance?

airflow/providers/databricks/hooks/databricks.py Outdated Show resolved Hide resolved
@pankajastro
Copy link
Member Author

Also, was it tested on the real Databricks instance?

yes, I have tested this.

@pankajastro pankajastro force-pushed the add_retry_databricks_async_op branch from c05eebb to 1e4efa7 Compare April 23, 2023 18:20
Copy link
Contributor

@alexott alexott left a comment

Choose a reason for hiding this comment

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

thank you Pankaj!

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.

3 participants