-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fixing DatabricksNotebookOperator invalid dependency graph issue #48492
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
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)
|
|
@dheerajkumar-solanki, nice stuff! I was going to approach this in a bit of a different way, but I like what you did. Question; were you able to run that DAG that you originally had posted? |
|
There were a few other places where I think we might need to make changes. |
Which places, can you provide some context, so I can update the PR. |
Yes, I was able to run the failed DAG. |
|
This was one of the places I believe would need a change (and was part of the original PR that we believe may have been "breaking"). |
1de6d22 to
3b93d41
Compare
|
@dheerajkumar-solanki, how are things going on your side? Have those unit-tests been run? |
I checked the code and previous MR, there is no change required as we have not change the function definition of All the databricks unit test files are running fine. |
|
@pankajkoti @jroachgolf84 One of the tests pipeline is failing Tests / CI image checks / MyPy checks (mypy-providers) (pull_request), doesn't look like from the changes in this PR. Is it just due to my branch is not up to date with base branch or any other issue? Also, there is another MR on this same issue (#48623), I like his solution but I think some pieces are missing in the MR. what would be next step in this MR now? |
3b93d41 to
083240e
Compare
@potiuk rebase is done, waiting for workflow approval to run the test again. |
dc0956e to
8f3f8c2
Compare
8f3f8c2 to
f431994
Compare
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
|
Thanks! |


Fixing the DatabricksNotebookOperator invalid dependency graph issue, this comes when dependencies are defined in DatabricksWorkflowTaskGroup. Issue is child task DatabricksNotebookOperator and DatabricksTaskOperator task referencing itself. Issue in
_generate_databricks_task_keyfunction which is incorrectly generatingdepends_ontask value.closes: #47983
^ 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 airflow-core/newsfragments.