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

Declare pip dependency for cli extras #2207

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

jonbiemond
Copy link

The pipdeptree package includes references to pip but does not explicitly require it.
To ensure pip is installed in a users environment, include it as a dependency.

Fixes #2203

Copy link

netlify bot commented Jan 13, 2025

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 1d405ca
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/678f499fa46bda00083aeaf6

@jonbiemond jonbiemond force-pushed the fix/2203-declare-pip-dependency branch from 674ca09 to 496dfff Compare January 13, 2025 06:52
@jonbiemond
Copy link
Author

pip's versioning scheme is year.quarter.patch. I arbitrarily chose to pin dependencies at >=24.0.0 or the earliest release of 2024.

@rudolfix rudolfix requested a review from burnash January 14, 2025 14:39
@jonbiemond jonbiemond force-pushed the fix/2203-declare-pip-dependency branch from 496dfff to 116a4d5 Compare January 14, 2025 16:39
@burnash
Copy link
Collaborator

burnash commented Jan 20, 2025

Hey @jonbiemond, thank you for the fix and sorry for the delayed response. Could you please merge or rebase the dev branch and regenerate poetry.lock to fix conflicts?

The `pipdeptree` package includes references
to `pip` but does not explicitly require it.
To ensure `pip` is installed in the environment,
declare it as a dependency.
@jonbiemond jonbiemond force-pushed the fix/2203-declare-pip-dependency branch from 116a4d5 to e73037a Compare January 21, 2025 06:42
Some tests require pip.
@jonbiemond
Copy link
Author

Hi @burnash, no worries! I've rebased and regenerated the lock file.

There are also two tests failing in CI. Now that pip is explicitly declared as an optional dependency, poetry install will remove it if it exists and when no extras are specified. These two tests rely on pip, which is why they now fail.

I've added a commit to include pip in the dev dependencies as well. This should resolve the failing tests. Alternatively all dependencies could be installed in CI with the --all-extras flag.

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 this pull request may close these issues.

Deploy command fails even with extras installed
2 participants