Skip to content

Add DatabricksWorkflowPlugin - #40153

Merged
pankajkoti merged 16 commits into
apache:mainfrom
astronomer:add-databricks-plugin
Jul 9, 2024
Merged

Add DatabricksWorkflowPlugin#40153
pankajkoti merged 16 commits into
apache:mainfrom
astronomer:add-databricks-plugin

Conversation

@pankajkoti

@pankajkotipankajkoti commented Jun 9, 2024

Copy link
Copy Markdown
Member

The DatabricksWorkflowPlugin provides with links in the Airflow
UI for tasks that allow us to see the Databricks job run in the
Databricks workspace, additionally it also provides link to
repair task(s) in the workflow.

Databricks does not allow repairing jobs with single tasks launched
outside the workflow, hence we just provide the link for the job run.
Screenshot 2024-06-24 at 4 05 53 PM

Within the workflow, for each of the task, we provide links to the
job run and repair link for the single task
Screenshot 2024-06-24 at 5 40 27 PM

And at the workflow level, for the job launch task, we provide a
link to repair all failed tasks along with the link for job run in
the Databricks workspace that can be used to monitor the job
in the Databricks account.
Screenshot 2024-06-24 at 5 40 56 PM


^ 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.

@pankajkoti
pankajkotiforce-pushed the add-databricks-plugin branch 3 times, most recently from 872034d to e72470cCompareJune 20, 2024 13:55
@pankajkoti
pankajkoti marked this pull request as ready for review June 21, 2024 05:48
@pankajkotipankajkoti changed the title [WIP] Add DatabricksWorkflowPluginAdd DatabricksWorkflowPluginJun 21, 2024
@phanikumv
phanikumv requested a review from Lee-WJune 21, 2024 06:45
@pankajkoti
pankajkotiforce-pushed the add-databricks-plugin branch 2 times, most recently from 1804ecf to a5e694fCompareJune 24, 2024 15:29
@pankajkoti

pankajkoti commented Jun 25, 2024

Copy link
Copy Markdown
MemberAuthor

The failing 1 test Provider checks / Compat 2.8.4:P3.8 provider check is unrelated and coming from the Amazon provider tests

=========================== short test summary info ============================
FAILED tests/providers/amazon/aws/auth_manager/test_aws_auth_manager.py::TestAwsAuthManager::test_aws_auth_manager_index - TypeError: is_authorized_custom_view() got an unexpected keyword argument 'fab_action_name'
============ 1 failed, 3059 passed, 5 skipped in 358.28s (0:05:58) =============

Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated

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

Great work, @pankajkoti !

This feature looks better than the original implementation, thanks for fixing the behaviour of monitoring the last attempt to run the task and also when we show the repair button (not for standalone NotebookOperator.

Please add documentation with screenshots - this will help end-users.

What do you think about adding a follow-up task to support repair when Airflow attempts to retry a failed task?

@phanikumvphanikumv 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, except missing documentation about when/why users should utilize this

Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
@pankajkoti
pankajkotiforce-pushed the add-databricks-plugin branch from ebc01b6 to 9c63f9bCompareJune 30, 2024 09:47
Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
eladkal
eladkal previously requested changes Jun 30, 2024

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

temporary block to avoid merge till #40153 (comment) is resolved.

Comment threadairflow/providers/databricks/plugins/databricks_workflow.py Outdated
@pankajkoti
pankajkotiforce-pushed the add-databricks-plugin branch from 17fd0c7 to b98b55bCompareJuly 8, 2024 09:24
@potiuk

Copy link
Copy Markdown
Member

@eladkal ?

@eladkal

Copy link
Copy Markdown
Contributor

@eladkal ?

I will get to it only in few days.
Feel free to dismiss my request changes if the concern I raised was handled

@potiuk

Copy link
Copy Markdown
Member

I think there is a side-effect of registering plugin that should be removed (so all tests that register plugins should deregister them in setup/teardown - thugh not sure how to do it :)

@pankajkoti

Copy link
Copy Markdown
MemberAuthor

The failing tests report

FAILED tests/providers/microsoft/azure/operators/test_container_instances.py::TestACIOperator::test_execute_with_spot_discount - AttributeError: 'ContainerGroup' object has no attribute 'priority'

and

FAILED tests/providers/amazon/aws/auth_manager/test_aws_auth_manager.py::TestAwsAuthManager::test_aws_auth_manager_index - TypeError: is_authorized_custom_view() got an unexpected keyword argument 'fab_action_name'

Tried re-running them, but they still fail. They are unrelated to this PR and hence I'm going ahead with merging the PR.

@pankajkoti
pankajkoti merged commit 22ec726 into apache:mainJul 9, 2024
@pankajkoti
pankajkoti deleted the add-databricks-plugin branch July 9, 2024 16:52
jscheffl added a commit that referenced this pull request Jul 11, 2024
pankajkoti pushed a commit that referenced this pull request Jul 11, 2024
pankajkoti added a commit that referenced this pull request Jul 12, 2024
The DatabricksWorkflowPlugin provides with links in the Airflow
UI for tasks that allow us to see the Databricks job run in the
Databricks workspace, additionally it also provides link to repair task(s) in the workflow.
Databricks does not allow repairing jobs with single tasks launched
outside the workflow, hence we just provide the link for the job run.
Within the workflow, for each of the task, we provide links to the
job run and repair link for the single task
And at the workflow level, for the job launch task, we provide a
link to repair all failed tasks along with the link for job run in the Databricks workspace that can be used to monitor the job
in the Databricks account.
This PR is the second attempt on adding the DatabricksWorkflowPlugin,
the previous attempt being #40153. However, there were some concerns raised in #40708 and hence it was reverted in #40714. This newer PR attempts to address those concerns.
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
The DatabricksWorkflowPlugin provides with links in the Airflow
UI for tasks that allow us to see the Databricks job run in the
Databricks workspace, additionally it also provides link to repair task(s) in the workflow.
Databricks does not allow repairing jobs with single tasks launched
outside the workflow, hence we just provide the link for the job run.
<img width="1342" alt="Screenshot 2024-06-24 at 4 05 53 PM" src="https://github.com/apache/airflow/assets/10206082/c1ded2b7-90fb-4a3c-980d-8043cc5a459f">
Within the workflow, for each of the task, we provide links to the
job run and repair link for the single task
<img width="1368" alt="Screenshot 2024-06-24 at 5 40 27 PM" src="https://github.com/apache/airflow/assets/10206082/d98081b8-8014-4d18-8747-e5b3382db416">
And at the workflow level, for the job launch task, we provide a
link to repair all failed tasks along with the link for job run in the Databricks workspace that can be used to monitor the job
in the Databricks account.
<img width="1421" alt="Screenshot 2024-06-24 at 5 40 56 PM" src="https://github.com/apache/airflow/assets/10206082/9d7ad7ae-9bbf-4fef-aa52-16ac5366edf3">
---------
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
The DatabricksWorkflowPlugin provides with links in the Airflow
UI for tasks that allow us to see the Databricks job run in the
Databricks workspace, additionally it also provides link to repair task(s) in the workflow.
Databricks does not allow repairing jobs with single tasks launched
outside the workflow, hence we just provide the link for the job run.
Within the workflow, for each of the task, we provide links to the
job run and repair link for the single task
And at the workflow level, for the job launch task, we provide a
link to repair all failed tasks along with the link for job run in the Databricks workspace that can be used to monitor the job
in the Databricks account.
This PR is the second attempt on adding the DatabricksWorkflowPlugin,
the previous attempt being apache#40153. However, there were some concerns raised in apache#40708 and hence it was reverted in apache#40714. This newer PR attempts to address those concerns.
Sign up for freeto 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.

7 participants

@pankajkoti@potiuk@eladkal@ashb@tatiana@Lee-W@phanikumv