-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[CT-36] [Bug] Airflow v2.1, v2.2 constraints incompatible with dbt v1.0.0, v1.0.1 on codependency click. #4566
Comments
Good suggestion and thanks for considering my practical needs. I am indeed considering Docker containers to wrap up different tools, including dbt. KubernetesPodOperator fits into that picture, though with its own baggage. I started digging down this path because Airflow and dbt installed from PyPI into a single virtual environment was intriguing. No containers to build and manage. No calling between different virtual environments. Simple operators from airflow-dbt wrapping the CLI. Looked like a simple and quick way to manage pure dbt workflows. Not wanting to waste an opportunity of "neat, it could just work" go by, I chased this option. dbt 0.21.1 did easily coexist in the same environment with Airflow compared to dbt 1.0.0+. The difference was this impossible click constraint. I guess I'm looking for an answer to the question "should dbt play well with Airflow in the same environment?" I lean towards yes because it's a good pairing and the experience of PyPI is nice. But "yes" is not free as it implies reconciling codependencies to the lowest common denominator. I'll probably make a patch on dbt just to see what supporting click v7 looks like and its implications. Actually landing such a change needs more thought. :) |
@twilly I just installed Line 56 in 1357143
Is that a change you'd be interested in contributing? If so, there's good cause to make it in a v1.0 patch release, so that folks can upgrade to v1.0 for use with Airflow, and don't need to wait until v1.1. The change feels pretty low-stakes since |
`dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue dbt-labs#4566
`dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue dbt-labs#4566
`dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue dbt-labs#4566
Thanks for jumping in! Sorry for being non-responsive here, I was digging around the source code of click and Funnily enough On a side note, it would be interesting if the integration testing environment did nothing but interact on stdin/out/err. Then you could control environments and run the test suite in different situations. Anywho, the PR is up at #4681. Give it a gander. |
* task init: support older click v7.0 `dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue #4566 * Update CHANGELOG.md Co-authored-by: Chenyu Li <chenyulee777@gmail.com> Co-authored-by: Chenyu Li <chenyulee777@gmail.com>
* task init: support older click v7.0 `dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue #4566 * Update CHANGELOG.md Co-authored-by: Chenyu Li <chenyulee777@gmail.com> Co-authored-by: Chenyu Li <chenyulee777@gmail.com> automatic commit by git-black, original commits: 72ecd1c
Following up on this, Airflow 2.2.4 and dbt-core 1.0.3 can co-exist with each other in the same virtual environment, at least on my machine with Python 3.9. I'll take that as a win. :) I'm closing this issue as there doesn't seem to be any obvious installation issues with the two together. That said, I wouldn't recommend making a policy of keeping co-existence a release blocker. It's not an easy thing to track and tackle without significant integration testing and using containers is a more viable, production-quality option. I'd say making the two happy together is an occasional effort thing: tackle co-dependency problems as they arise. Thanks for the discussion, guidance, and landing of my PR. |
* task init: support older click v7.0 `dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue #4566 * Update CHANGELOG.md Co-authored-by: Chenyu Li <chenyulee777@gmail.com> Co-authored-by: Chenyu Li <chenyulee777@gmail.com>
* task init: support older click v7.0 `dbt init` uses click for interactively setting up a project. The version constraints currently ask for click >= 8 but v7.0 has nearly the same prompt/confirm/echo API. prompt added a feature that isn't used. confirm has a behavior change if the default is None, but confirm(..., default=None) is not used. Long story short, we can relax the version constraint to allow installing with an older click library. Ref: Issue #4566 * Update CHANGELOG.md Co-authored-by: Chenyu Li <chenyulee777@gmail.com> Co-authored-by: Chenyu Li <chenyulee777@gmail.com> Co-authored-by: Tristan Willy <twilly@users.noreply.github.com> Co-authored-by: Chenyu Li <chenyulee777@gmail.com>
Is there an existing issue for this?
Current Behavior
NOTE: This is a minor bug regarding a common integration.
Airflow v2.1 and v2.2 transitively depends on click>=7.1,<9. Airflow requires Flask v1.1 and Flask v1.1.3+ further constrains click to >=5.1,<8.0.
dbt-core 1.0.0. and 1.0.1 depends on click>=8,<9 from commit 11436fe.
The combined click constraint of >=8,<8.0 is unsolvable, preventing the installation of Airflow with dbt v1.0.0 and v1.0.1.
Expected Behavior
The behavior is correct; it's just a nightmare to deal with. IMHO, it can be better by supporting click v7 in dbt v1.0.
Steps To Reproduce
Try installing the latest release of dbt-core and Flask together:
$ pip install dbt-core==1.0.1 Flask==1.1.4
Relevant log output
What database are you using dbt with?
No response
Additional Context
Apache Airflow provides constraint files of known-working configuration and has published constraints that pin Flask to 1.1.4 (e.g. v2.1, Py 3.9). More recent constraints with Airflow v2.2 pin Flask down to v1.1.2 (v2.2, Py 3.9).
I am unsure of Airflow's reasoning behind downgrading to Flask v1.1.2 in v2.2 and the mainline branch, though I suspect it's because Flask 1.1.3 added additional constraints: pallets/flask: Older versions (1.x.x) of flask pin some dependencies in a way that could cause issues. Interestingly David Lord commented that Flask doesn't follow semver though David doesn't mind setting a maximum version on click in Flask v1.1.3 given the v1.1 line is no longer actively maintained.
Airflow is no stranger to transitive dependency problems: old libraries in setup.py causing dependency resolution to pull old transitive constraints (3 years+). There's some good comments in there and I recommend reading the thread.
Commercial integrations suffer as well. Google Cloud Composer currently ships with Airflow v2.0 and v2.1 and also isn't happy installing dbt v1.0.x.
dbt's constraint on click >=8 may see breakage in pinned Airflow installs, either breaking Flask (implicit <8) or breaking dbt (explicit >=8). Given dbt's constraint is a recent change (Oct 20, 2021 in 11436fe), I'm thinking it may be possible to support click v7 and loosen dbt's constraint.
That's as far as I've dug in and would like some thoughts around dbt supporting click v7. I can make a patch if it'll help the conversation.
The text was updated successfully, but these errors were encountered: