-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Made sqlalchemy dependency optional for Databricks provider #60110
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
Made sqlalchemy dependency optional for Databricks provider #60110
Conversation
|
I think, the compat tests are failing because of an interesting side-effect of making databricks-sqlalchemy optional. What happens is that we we are installing "dev" dependencies with airflow 3 -> so latest databrics-sqlalchemy is installed (2.0.8) - because it supports sqlalchemy 2 (and airflow 3.2 already supports it). Then when later airflow 2 or 3 versions are installed, they bring the sqlalchemy to 1 (because they do not support 2) but they do not bring databricks-sqlalchemy down, when databricks provider is installed then (without sqlalchemy extra) - it also will not bring the databricks-sqlalchemy down :( |
|
I wil send a workaround patch shortly for that "optional sqlalchemy extra" soon |
|
Thanks for chiming in. Will update the PR w the changes once you send the workaround patch over. |
|
Fixup pushed |
Thanks for that. I wasn't sure if I was meant to make the change myself, so I was waiting to update the |
ecd8139 to
7143f89
Compare
7143f89 to
00eb9cb
Compare
jscheffl
left a comment
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.
Looks good for me! Restarted CI - hopefully will turn green in a moment.
…0110) * Made sqlalchemy dependency optional for databricks provider * fixup! Made sqlalchemy dependency optional for databricks provider * rst file modified * switched dep depency to use airflow-providers sqa --------- Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
…0110) * Made sqlalchemy dependency optional for databricks provider * fixup! Made sqlalchemy dependency optional for databricks provider * rst file modified * switched dep depency to use airflow-providers sqa --------- Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Made sqlalchemy an optional dependency for the Databricks provider.
AirflowOptionalProviderFeatureException()gets raised if it's not installed. Just a quick note - Databricks uses thedatabricks-sqlalchemypackage, since it's now an optional dependency, I needed to add it to the dev dependencies to ensure the tests that rely on it work properly.closes: #59900
^ 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.