-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
More operators for Databricks Repos #22422
Conversation
Can this functionality be done without adding a new operator? I am afraid of adding more operators when we do not even have plans on how we want to maintain them and test if they work properly. Commiets check that the change looks good. Sometimes they will look at the documentation and see if it matches what is implemeents, but it may not work properly. What can we do to improve the operational readiness of this provider, since we have recently had more changes in it? For Snowflake, we decided that we only have one operator, so it's easy to check that it's working properly. What would we like to do for Databricks? Do you have any plans? |
@mik-laj These operators are for specific functionality that is available via specific REST API, so alternative will be to get down to low-level API calls. It's a different from the Snowflake connector where you do everything via SQL - for Databricks we have similar We're planning to build a system test suite that will be used to test all operators. |
a5d8d41
to
1fca768
Compare
Yep. We are just about to merge first change from AIP-47 #22311 and we will literally prepare (automatically) issues to convert all the providers. I will ask everyone invlved to comment and will assign them to those issues. |
if repo_path is not None and not self.__repos_path_regexp__.match(repo_path): | ||
raise AirflowException( | ||
f"repo_path should have form of /Repos/{{folder}}/{{repo-name}}, got '{repo_path}'" | ||
) |
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.
Since repo_path
is a templated field, there is a chance that the arg passed in is a Jinja expression or an XComArg
in which case the regexp check would fail with a false-negative result potentially because templated values aren't evaluated until the task begins to execute.
Can you move this to the execute()
method scope?
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.
good point @josh-fell !
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.
thank you - I'll fix it later today.
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.
@potiuk @josh-fell I've addressed review comments...
c953678
to
5e07b20
Compare
e2879c9
to
109e3e3
Compare
This PR adds two additional operators for Databricks Repos:
DatabricksReposCreateOperator
- to create a new Databricks RepoDatabricksReposDeleteOperator
- to delete a Databricks Repo