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

[Feature] An explicitly specified prerelease package should install without requiring install-prerelease #4243

Closed
1 task done
joellabes opened this issue Nov 9, 2021 · 2 comments · Fixed by #4295
Closed
1 task done
Labels
deps dbt's package manager enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors!

Comments

@joellabes
Copy link
Contributor

Is there an existing feature request for this?

  • I have searched the existing issues

Describe the Feature

I opened #4224 because I tried to install a prerelease version of dbt utils without the install-prerelease: True setting:

packages:
  - package: dbt-labs/dbt_utils
    version: 0.7.4-b1 

Requesting a prerelease package by name should be enough to trigger its installation.

Describe alternatives you've considered

No response

Who will this benefit?

Test drivers of beta packages

Are you interested in contributing this feature?

Only if no one else beats me to it

Anything else?

A potential extra for experts detailed here is to also add a --pre subcommand to dbt deps which behaves like pip install x --pre and installs prerelease versions of all first-order packages (i.e. not if you're getting dbt-utils by dependency instead of a direct reference).

@joellabes joellabes added enhancement New feature or request triage labels Nov 9, 2021
@jtcohen6 jtcohen6 added good_first_issue Straightforward + self-contained changes, good for new contributors! and removed triage labels Nov 16, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Nov 16, 2021

Yes, we should absolutely do this!

I was afraid the logic here would be pretty gnarly, but after a quick dive in, it's much simpler than I thought:

installable = semver.filter_installable(
available,
self.install_prerelease
)

Becomes:

        # if any prelease version is explicitly specified in any way,
        # assume we're good to use prereleases for this package
        prerelease_version_specified = any(
            bool(version.prerelease) for version in self.versions
        )
        installable = semver.filter_installable(
            available,
            self.install_prerelease or prerelease_version_specified
        )

This would have the effect of supporting both of the following:

packages:
  - package: dbt-labs/dbt_utils
    version: 0.7.4-b1 
packages:
  - package: dbt-labs/dbt_utils
    version: [">=0.7.4-b1", "<0.7.5"]
$ dbt deps
20:07:06 | [ info  ] | Running with dbt=1.0.0-rc1
20:07:06 | [ info  ] | Installing dbt-labs/[email protected]
20:07:06 | [ info  ] |   Installed from version 0.7.4
20:07:06 | [ info  ] |   Up to date!

I'd welcome a PR for this!


A larger question: to what extent should we try to follow PEP 404 to the letter? (I stumbled across some interesting related conversation here: pypa/pip#5503).

Namely:

  • If a user explicitly specifies a prerelease version for a package, are they opting into that prerelease version? YES
  • If a user includes any prerelease marker in their version specification for a package, are they opting into prereleases for that package? I say YES
  • If a prerelease version is the only valid and available version, should dbt install it? This feels a bit more controversial; I think yes, but it's worth discussing more. It would require a modification to the logic in filter_installable, to prepare both installable and installable_with_prereleases, and hand back the latter if the former is unavailable.

@jtcohen6 jtcohen6 added the packages Functionality for interacting with installed packages label Nov 16, 2021
@joellabes
Copy link
Contributor Author

If a prerelease version is the only valid and available version, should dbt install it? This feels a bit more controversial; I think yes, but it's worth discussing more. It would require a modification to the logic in filter_installable, to prepare both installable and installable_with_prereleases, and hand back the latter if the former is unavailable.

I don't have strong feelings on this. Two options:

  • silently succeed - install it because they asked for joels-cool-package and the prerelease version is the only option
  • audibly fail - "joels-cool-package only has prerelease versions available. To install, set install_prerelease to true"

@jtcohen6 jtcohen6 added deps dbt's package manager and removed packages Functionality for interacting with installed packages labels Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps dbt's package manager enhancement New feature or request good_first_issue Straightforward + self-contained changes, good for new contributors!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants