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

Pyproject toml builds dbt #6047

Closed
wants to merge 11 commits into from

Conversation

davidbloss
Copy link
Contributor

resolves #5696 and #4674 and #4446

Description

This update to pyproject.toml allows pip to install dbt-core with poetry in place of setuptools. dbt adapters, other dependent codebases, core devs, and users should not notice any changes from this update.

Notes

  • pip serves as the "frontend" of the build process while both setuptools and poetry run on the "backend"
  • Installing dbt with pip should behave the same between setuptools and poetry
  • poetry is automagically managed by pip -- devs and users do not need to separately install it for purposes of installing dbt-core. pip does not download poetry for later use unless specifically told to
  • setup.py and MANIFEST.in still remain but all their logic has been consolidated into pyproject.toml
  • Either poetry or setuptools can be used at this state
  • poetry.lock provides a deterministic, version controlled option for dependency management

Mapping old keys to new keys

setup.py pyproject.toml
long_description readme
long_description_content_type readme
author authors
author_email authors
url repository
entry_points [tool.poetry.scripts]
  • minimal python version enforced by python = "<version>" under [tool.poetry.dependencies] in place of if sys.version_info ... in setup.py

Dropped entries

test_suite="test" see deprecated setuptools feature
zip_safe=False see other deprecated setuptools feature

Added entries

license = "Apache-2.0"
homepage = "https://getdbt.com"
documentation = "https://docs.getdbt.com/"
Various links in [tool.poetry.urls]

Checklist

@cla-bot
Copy link

cla-bot bot commented Oct 11, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @davidbloss

@cla-bot
Copy link

cla-bot bot commented Oct 11, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @davidbloss

@cla-bot cla-bot bot added the cla:yes label Oct 11, 2022
@leahwicz
Copy link
Contributor

Hi @davidbloss! Thanks so much for opening this contribution up! We currently are a couple weeks away from scheduling work to revamp our package and release process for dbt-core and the adapters. Part of that work will involve looking at investing more in pyproject.toml like you are doing here. I would ask if you could please 🙏 be patient with us reviewing this PR until we get to that work in case it has any impact on our plans. I would hate to merge this and then immediately change something again depending on the work .
I actually would be interested in hearing more from you on if you have other suggestions or things you would like to see us do with changes around setup, packaging, etc. that we might be able to include in the scope of this upcoming work. It's still being planned out and designed so any feedback on our current setup would be appreciated!

@leahwicz leahwicz added the repo ci/cd Testing and continuous integration for dbt-core + adapter plugins label Oct 13, 2022
@davidbloss
Copy link
Contributor Author

Hi @davidbloss! Thanks so much for opening this contribution up! We currently are a couple weeks away from scheduling work to revamp our package and release process for dbt-core and the adapters. Part of that work will involve looking at investing more in pyproject.toml like you are doing here. I would ask if you could please 🙏 be patient with us reviewing this PR until we get to that work in case it has any impact on our plans. I would hate to merge this and then immediately change something again depending on the work . I actually would be interested in hearing more from you on if you have other suggestions or things you would like to see us do with changes around setup, packaging, etc. that we might be able to include in the scope of this upcoming work. It's still being planned out and designed so any feedback on our current setup would be appreciated!

Hi @leahwicz, I understand and please take your time!
After another look I realized that I did not include anything where poetry would also install dbt-postgres as is done with pip install -r requirements.txt. There are certainly some extra features I thought about including in this PR, like an optional dev-dependencies section and some way of handling dbt-postgres with poetry as well - while still leaving all existing install avenues intact.
Somewhat related, I noticed that the Dockerfile references a git url pointing to an egg. I think poetry only supports wheels and was not sure if there are plans to update this Dockerfile url to something like ghcr.io/dbt-labs/dbt-core:<version>.latest
But for the time being would it be best to update this to be a "Draft" PR until further notice?

@davidbloss davidbloss requested a review from a team as a code owner October 23, 2022 19:37
@jtcohen6
Copy link
Contributor

I hadn't seen this a month ago — very cool!

Does it simplify/unblock this work for us to first move dbt-postgres out into its own repository? (#6189)

We have been building our Docker images from specific release branches, rather than from the dist wheels built from those release branches. It does seem feasible that we could always build a wheel first. Not sure how tricky this is in practice / for our release process.

@davidbloss
Copy link
Contributor Author

Hey all, I am closing this since Poetry is IMO not the best path forward. Instead I will be reopening a similar branch that makes use of hatch - much better for CI and testing purposes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes repo ci/cd Testing and continuous integration for dbt-core + adapter plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-1067] [Spike] Impact/feasibility checks for pyproject.toml
3 participants