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

Raise error on pip install dbt #4133

Merged
merged 11 commits into from
Nov 7, 2021
Merged

Raise error on pip install dbt #4133

merged 11 commits into from
Nov 7, 2021

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Oct 26, 2021

resolves #4100

  • Raise an explicit error on pip install dbt
  • Include richer metadata for dbt-core (in core/setup.py)
As of v1.0.0, `pip install dbt` is no longer supported.
  Instead, please use either:
    - `pip install dbt-core`, for core functionality
    - `pip install dbt-<adapter>`, to use dbt with your database, platform, or query engine
  See full list: https://docs.getdbt.com/docs/available-adapters

What's good

  • Simple, clear, to the point
  • No need for circular-ish dependencies (dbt-coredbt-snowflakedbt)
  • Rips off the band-aid

What's bad

  • This will break production pipelines that depend on pip install dbt
  • This will break any python programs that have unbounded dependencies on dbt (rather than dbt-core)
  • Cloning this repo + pip install . will not work, it needs to be cloning this repo + pip install -r requirements.txt (what dbt Cloud HEAD does) or pip install -r editable-requirements.txt (what we tell contributors to do)
  • pip install git+https://github.com/dbt-labs/dbt-core will not work, it needs to be git+https://github.com/dbt-labs/dbt.git#egg=dbt-core&subdirectory=core (we can document this)

Uncertainties

  • Will PyPi metadata still work? Will we be able to push and recognize this package as dbt?
  • I wasn't able to get clear deprecation warnings working, and I wasn't able to find very clear resources online, but that's not to say that it isn't possible

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@cla-bot cla-bot bot added the cla:yes label Oct 26, 2021
@jtcohen6 jtcohen6 changed the title Raise error on Requirement already satisfied: dbt in /Users/jerco/dev… Raise error on pip install dbt Oct 26, 2021
@jtcohen6 jtcohen6 marked this pull request as ready for review November 3, 2021 09:45
@jtcohen6 jtcohen6 requested review from kwigley and leahwicz November 3, 2021 09:45
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Nov 3, 2021

I'm pretty sure this change will break PyPi metadata, so we might need to hand-roll that instead. Maybe we could test by pushing a dbt==1.00b1 prerelease after merging?

@leahwicz
Copy link
Contributor

leahwicz commented Nov 3, 2021

@jtcohen6 we have the test PyPi account we can use to try this out

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Nov 3, 2021

@leahwicz ah really good thought!! I forgot all about that

@kwigley
Copy link
Contributor

kwigley commented Nov 5, 2021

@jtcohen6 tested locally and ended up pushing a small change to setup.py (cf3089d).

In order for this to work as expected, we should only build a source distribution when publishing to PyPI (python setup.py sdist). Distributing a wheel file won't run the code to warn the user.

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Nov 5, 2021

@kwigley Thanks for the magic one-liner there. I did some reorg enabled by that, including some edits to build-dist. Let me know if I'm being too clever/fancy there. Yes, I was. Opting to keep it simple + duplicative.

Let's try building dbt=1.0.0-b1 and pushing to Test PyPi, before we go live-live with this

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Nov 5, 2021

Now that I've reenabled the source-only distribution of dbt, it looks like I need to update some more pieces of the main.py CI test to avoid trying to install it, since it will raise an error :)

@jtcohen6 jtcohen6 merged commit dd84f9a into main Nov 7, 2021
@jtcohen6 jtcohen6 deleted the pip-install-dbt branch November 7, 2021 16:55
@joellabes
Copy link
Contributor

I'm not convinced that

  • pip install dbt-core, for core functionality

fulfils the "clear" component of

Simple, clear, to the point.

Why? Because I don't understand it 😅 . I'm reading it as "to access the critical functionality of dbt", but I think you want me to read it as "access the functionality of the dbt core project". Is that right?

Who is the target market for dbt-core on its own? I assume it's people building things that integrate with the dbt ecosystem, as opposed to analytics engineers building a dbt project? As a generic end user, "connect to a database" is definitely "core critical functionality" so I wouldn't necessarily be warned off of trying to install it.

Noodling on some alternatives:

  • pip install dbt-core, for core functionality without connecting to a database
  • pip install dbt-core, for developers of dbt integrations

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Nov 8, 2021

pip install dbt-core, for developers of dbt integrations

I like this! I'll also move it so that it's after pip install dbt-<adapter>, which we expect to be relevant in 95%+ of cases.

Just opened: #4229

iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
* Raise error on pip install dbt

* Fix relative path logic

* Do not build dist for dbt

* Fix long descriptions

* Trigger code checks

* Using root readme more trouble than good

* only fail on install, not build

* Edit dist script. Avoid README duplication

* jk, be less clever

* Ignore 'dbt' source distribution when testing

* Add changelog entry

Co-authored-by: Kyle Wigley <[email protected]>

automatic commit by git-black, original commits:
  dd84f9a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What should pip install dbt do after v1?
4 participants