-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add-showing-runtime-error-feature-to-DatabricksSubmitRunOperator #21709
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-showing-runtime-error-feature-to-DatabricksSubmitRunOperator #21709
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 Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll make sense to add a test for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ephraimbuddy added here, can you please check ? :c5846a8
|
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
|
Static checks are failing |
|
needs rebase |
c5846a8 to
7ce3499
Compare
|
@potiuk I rebased the pr, can you please run workflows ? |
|
Hi @potiuk, I could not see anything wrong which causes the static checks failure. If you could, can you point out that please ? |
|
Please take a look at the failed check and read both explanation of the error and instuctions how to fix it (via pre-commit): It is all there if you follow details: Look at explains it all. If you install pre-commit as:
You won't even have to fix it, because pre-commits will fix it for you. |
|
Hi @potiuk I run with pre-commit and pushed already , can you please run workflows? |
|
Hi @potiuk I run with pre-commit run --all-files again since in first it did not catch the one error , can you please run workflows? |
|
Hi @potiuk do you think that pr can be merged ? It looks like passed all the checks. |
|
Awesome work, congrats on your first merged pull request! |
|
Merged ! |


As its known in Airflow when we have a failed step (submitted a run for a Databricks notebook), we are not able to see the error in Airflow logs and need to visit the run page via the given link.
In order to see the notebook error directly in Airflow step logs as in below

, in airflow.providers.databricks.operators.databricks - DatabricksSubmitRunOperator, I enriched the AirflowException error message with the error key value from run output by using the Databricks api/2.0/jobs/runs/get-output API and additional method in databricks hook (airflow.providers.databricks.hooks.databricks - DatabricksHook). The operator and hook has been already tested with an example DAG in local setup by running test on both with succeed and fail cases.
The new operator has all the functionality of prev version DatabricksSubmitRunOperator. Besides it has the functionality that with the API endpoint fetches the notebook error and shows the notebook error directly in step logs in Airflow.