Add support for serverless job in Databricks operators #45188
Conversation
|
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 (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
@alexott Please could you give your review comments when possible. |
There was a problem hiding this comment.
Thanks for the great addition @HariGS-DB . I have few minor suggestions. Happy to take another pass once you add your thoughts or have addressed the comments.
Thanks @eladkal for requesting review here
providers/src/airflow/providers/databricks/operators/databricks.py
Outdated
Show resolved
Hide resolved
providers/src/airflow/providers/databricks/operators/databricks.py
Outdated
Show resolved
Hide resolved
providers/src/airflow/providers/databricks/operators/databricks.py
Outdated
Show resolved
Hide resolved
pankajkoti
changed the title
HariGS-DB
requested review from
ephraimbuddy,
uranusjr,
bbovenzi,
pierrejeambrun,
ryanahamilton,
jscheffl,
ashb,
potiuk,
jedcunningham,
bolkedebruin and
XD-DENG
as code owners
Fixed the test to include typing.Dict and removed the old skipif check.
Fixing this so main works with most of the XCom example. But proper fix will be part of apache#45231
* Add HTTP retry handling into task SDK api.client * Add logging of call failures * Prevent task sdk tests with LocalExecutor fail with retries * Review feedback * Review feedback, Adjust wording * Correct time parameters to float * Review Feedback
* Add directory put and get functions for sftp provider * Add test code * Add directory exists check * Fix merge conflict * Add path exists check
* delete a variable * success toaster * key * look * delete modal * rename * rename
* add the api * tests * ImportVariablesBody * action_if_exists * rename * add desc * fix * remove unwanted teardown
* initial structure setup * add editVariable * initial edit setup * decouple add variable with form * edit modal * fix checks * rename folder * bug fix * reset error * rename * rename add * not edit key
Hi @pankajkoti I have incorporated your review comments. I had to close and reopen the PR due to some merge conflicts which I have resolved. Please could you check and give me your new feedback |
|
@HariGS-DB some checks are failing in the CI. Could you please take care of that? And then we're good to merge I guess |
I have addressed some of the issues. Could you please re-check? I think it needs your approval before the complete tests are run. |
|
@pankajkoti I have resolved the issues and the error check in the test and docs. But I am still getting a mypy error in the CI for code which I have not even touched. I might need some advice on how should I resolve them |
|
@pankajkoti I see you have pushed a new change which seem to have fixed the CI pipelines. Is there anything else need to be done for merging this. Please let me know |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
|
Thanks, @HariGS-DB, for the fantastic contribution! I’ve merged the PR. Since this was your first contribution, the CI pipeline required maintainer approval to run. Going forward, for your subsequent PRs, the CI pipelines will run automatically without needing approval. 🎉 |
|
@pankajkoti Do you know when this PR will be released in the next airflow version? I will need to add another PR to introduce a validation for serverless (check for presence of environment_key in the json payload when submitting it for serverless ). I will submit this PR this week, is it possible to hold this PR in the next release until then? |
|
Hi @HariGS-DB, Providers are typically released on a bi-weekly cycle, as outlined here: https://github.com/apache/airflow/blob/main/PROVIDERS.rst#community-providers-release-process. While it’s not possible to delay a release for a single PR or skip a PR from the release candidate, you can cast a negative vote if the provider release includes this PR and it introduces a breaking change. Release managers initiate a voting thread once the provider release candidates are created. If you’re not already subscribed to the dev list (https://airflow.apache.org/community/#mailing-list), I recommend doing so to stay informed about upcoming releases, allowing you to test and vote accordingly. In this case, it looks like the change is an addition rather than a regression, so proceeding with the release seems reasonable. Additional validation can be incorporated in the next release if needed, without delaying the current one. Does that sound good to you? |
…e#45188)" (apache#46724) This reverts commit 0631ba7. Related apache#46569, apache#45138
closes: #45138
This PR enables running Databricks jobs as a serverless mode. It relaxes the check for the presence of both job_cluster_key and existing_cluster_id (as for serverless, both settings should not be present). It also added a named parameter for databricks environment
Also added relevant unit test cases and updated the documentation.
^ 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.rstor{issue_number}.significant.rst, in newsfragments.