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

Could the dataclasses requirement be relaxed? #3150

Closed
bastienboutonnet opened this issue Mar 7, 2021 · 2 comments · Fixed by #3151
Closed

Could the dataclasses requirement be relaxed? #3150

bastienboutonnet opened this issue Mar 7, 2021 · 2 comments · Fixed by #3151
Labels
dependencies Changes to the version of dbt dependencies duplicate This issue or pull request already exists enhancement New feature or request

Comments

@bastienboutonnet
Copy link
Contributor

bastienboutonnet commented Mar 7, 2021

Describe the feature

Not really a feature... not really a bug

the hard requirement on dataclasses (https:/fishtown-analytics/dbt/blob/develop/core/setup.py#L72) makes dbt not really friendly if it has to cohabit with https:/willmcgugan/rich which requires dataclasses >=0.7.

I don't think it's a problem for a lot of people but it has come up when I wanted to make dbt a requirement of a CLI dbt helper I'm building https:/bitpicky/dbt-sugar

It should be fine for most people as they will most likely have installed dbt prior to installing dbt-sugar so the dependency resolver shouldn't go haywire and we instruct people to install dbt sugar using pipx which installs the tool in its own venv but makes it available globally but it could be funky for some users... Who knows.

Anyway, I thought I'd raise this and if dbt doesn't need this very specific version the requirement could be relaxed to something like >=0.6,<0.9 or so.

@bastienboutonnet bastienboutonnet added enhancement New feature or request triage labels Mar 7, 2021
@jtcohen6 jtcohen6 added duplicate This issue or pull request already exists dependencies Changes to the version of dbt dependencies and removed triage labels Mar 8, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Mar 8, 2021

Hey @bastienboutonnet, this is totally reasonable! The slightly tricky piece here is that the pinned dataclasses dependency is coming from a different library, hologram; the nice thing is that we also maintain it.

If you wanted to bump the dataclasses dependency in dbt, you could:

  1. Fork hologram, bump its dataclasses dependency, open a PR against it
  2. Open a PR against fishtown-analytics/dbt that replaces the versioned hologram dependency with a git+ pointer to your fork branch
  3. Assuming all CI checks pass in the dbt PR, we can merge the hologram PR, cut a new version, and update the dbt PR to point to that new version.

By the by, both of the dataclasses dependencies (in hologram and in dbt-core) are written as:

'dataclasses==0.6;python_version<"3.7"'

The dataclass functionality is native to python 3.7 and above; the dataclasses library is just a backport for python 3.6. So users should only be experiencing version conflict if they're using python 3.6. Also, python 3.6 end of life is December 2021 (9 months from now).

I'm going to close this issue only because it's a duplicate of dbt-labs/hologram#39, which I transferred from here to there in January. I'll also add a comment there with the same information I've mentioned above.

@bastienboutonnet
Copy link
Contributor Author

@jtcohen6 cool makes sense. I'll do all that at some point this week. And yeah I was aware of the shenanigans for only p36 but I guess until support for 36 is dropped in dbt it probably makes sense to still PR this change for the time being.

Thanks for the clear instructions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changes to the version of dbt dependencies duplicate This issue or pull request already exists enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants