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

DatabricksWorkflowOperator do not update ACL on workflow reset #45738

Open
1 of 2 tasks
adamgorkaextbi opened this issue Jan 17, 2025 · 5 comments · May be fixed by #47827
Open
1 of 2 tasks

DatabricksWorkflowOperator do not update ACL on workflow reset #45738

adamgorkaextbi opened this issue Jan 17, 2025 · 5 comments · May be fixed by #47827

Comments

@adamgorkaextbi
Copy link

Apache Airflow Provider(s)

databricks

Versions of Apache Airflow Providers

apache-airflow-providers-databricks==7.0.0

Apache Airflow version

2.10.4

Operating System

Linux

Deployment

Official Apache Airflow Helm Chart

Deployment details

official Helm Chart

What happened

DatabricksWorkflowOperator do not update ACL on workflow reset, this is done only during creation of workflow (Databricks API 2.0 implementation)
one more call is needed to Databricks API 2.0

What you think should happen instead

DatabricksWorkflowOperator on workflow reset should also update ACL in next api call

How to reproduce

try to modify ACL after creation of workflow

Anything else

Proposed solution:
after this line:


add this code:

            if "access_control_list" in job_spec.keys():
                access_control_list = {"access_control_list": job_spec["access_control_list"]}
                self.log.info(
                    "Updating ACL of Databricks workflow job %s with spec %s",
                    self.job_name,
                    json.dumps(access_control_list, indent=2),
                )
                self._hook.update_job_permission(job_id, access_control_list)

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@adamgorkaextbi adamgorkaextbi added area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Jan 17, 2025
@rawwar
Copy link
Contributor

rawwar commented Mar 9, 2025

@adamgorkaextbi , you haven't explained why it needs to update ACL on workflow reset. If you don't update ACL, does it fail? What's the consequence?

@arollet22
Copy link

Maybe I can jump in to answer that question. We are also experiencing the same issue. What happens generally is the Airflow dev forgets to set the correct ACL during the first run, faces a permission issue on the Databricks UI and doesn't understand why its updated ACL configuration doesn't fix the problem. (Usually Airflow workflows/jobs are created using Service Principals which do not grant any rights by default to the group / users).
One other use case I can imagine, although not faced yet, is if you were to migrate a workflow ownership to another group.

The consequence is not that the job doesn't run, that works well, but if one task fails then the dev isn't able to check the logs to correct the error.

@rawwar rawwar added good first issue and removed kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet pending-response labels Mar 13, 2025
@rawwar
Copy link
Contributor

rawwar commented Mar 13, 2025

@hardeybisey , would you like to work on this?

@hardeybisey
Copy link
Contributor

@hardeybisey , would you like to work on this?

Sure, I will be happy to pick this up. You can assign it to me.

@adamgorkaextbi
Copy link
Author

@adamgorkaextbi , you haven't explained why it needs to update ACL on workflow reset. If you don't update ACL, does it fail? What's the consequence?

When you update permissions in the input JSON, the operator does not apply the changes automatically. You need to update the ACL manually through the UI or by using CLI commands. This lead to bugs and unwanted behavior and manual work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants