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

Reason for including dbt_utils in packages? #52

Closed
jpmmcneill opened this issue Jan 12, 2022 · 7 comments · Fixed by #91
Closed

Reason for including dbt_utils in packages? #52

jpmmcneill opened this issue Jan 12, 2022 · 7 comments · Fixed by #91

Comments

@jpmmcneill
Copy link
Contributor

Hello! James here from Conjura.

Not so much an issue and more a conversation point. I'm wondering what the reason is with including dbt_utils explicitly in the packages yaml is?

In my experience (this may have been resolved with dbt 1.0.0) DBT isn't the best at resolving requirements across packages. For example, I can see this method being brittle when trying to install other packages that have done the same, and could lead to a version collision error on a dbt deps.

Was the reasoning just for ease of transparency of requirements?

@jpmmcneill
Copy link
Contributor Author

jpmmcneill commented Jan 12, 2022

Interestingly, I see that spark_utils used to be included in the packages.yml and was removed.

@clausherther
Copy link
Contributor

So, including packages that you're using within your own package (vs packages that provide shims for other packages like we used to do with spark-utils) was the recommendation from the folks at dbt-lab. The versioning issues should be minor if we're all using dbt hub, since any version/dependency issues should be resolved there? E.g. if two packages includes dbt-utils, dbt won't try to import twice if both are using dbt hub.
For compatibility reasons, we do want to pin to particular version ranges for dbt-utils. For example, 0.9.0+ of dbt utils is not backward compatible with pre-1.0.0 versions of dbt, and thus will only work with dbt-date 0.5.0+ etc.
Does that make sense?
Happy to hear other thoughts though!

@jpmmcneill
Copy link
Contributor Author

(this package was nudged up in my head from your post in package-ecosystem over on slack - sorry for the slow response 😓)

makes sense to me. The only thing that has been incredibly painful to deal with for my teams has been when we migrated from fishtown-analytics to dbt-labs. This is hardly a common occurance these days, so I think everything you said makes sense.

Closing this issue now.

@tinomen
Copy link

tinomen commented Nov 18, 2022

Sadly, using dbt hub doesn't solve any of this. It's not your problem to solve but it sure is annoying. I can't use this package because fivetran_utils 0.3.9 requires [">=0.8.0", "<0.9.0"]

@clausherther
Copy link
Contributor

@tinomen Definitely, I hear you. Now that dbt-core contains a lot of the date functionality for which we used to rely on dbt-utils for, there's a path towards moving off this dependency. There's also downstream impact on dbt-expectations, but I'll think about some more how we can make this happen soon-ish. The only thing we use dbt-utils here these days is dbt_utils.date_spine but I think there's a case to be made to re-implement this in dbt-date instead.

@tinomen
Copy link

tinomen commented Nov 18, 2022

That would be amazing. Until I found a previous version to use.

Thank you for the macros.

@clausherther
Copy link
Contributor

FYI, a version without dependency on dbt-utils is now on the main branch if you're feeling adventurous. However, this won't work with the latest dbt-expectations release if you're also using that.

So, the plan is to:

  • release this change as dbt-date 0.7.0 since this is a breaking change.
  • update dbt-expectations to use dbt-date 0.7.0 and also remove its dependencies on dbt-utils
  • release that as dbt-expectations 0.8.0 since this is a breaking change.

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

Successfully merging a pull request may close this issue.

3 participants