Check dbt-core version requirements when installing Hub packages #5651

jtcohen6 · 2022-08-12T11:36:36Z

Resolves #5648

Depends on dbt-labs/hubcap#146
For testing, check out dbt-labs/hub.getdbt.com#1770

As mentioned in the Hubcap PR above, I'm reusing the existing (and always empty) works_with field from the Hub API to represent the package's require-dbt-version. I'm not sure it's an old field being used for something in very old versions of dbt-core. This PR leverages a new API field added in the Hubcap PR above: require_dbt_version.

Description

Replace registry.get_available_versions with registry.get_compatible_versions, which only returns package versions whose require-dbt-version config is compatible with the currently installed version of dbt-core
If a user has disabled VERSION_CHECK (e.g. dbt --no-version-check deps), then dbt will not check for version compatibility
Update unit tests

Checklist

I have read the contributing guide and understand what's expected of me
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 opened an issue to add/update docs, or~~ docs changes are not required/relevant for this PR
I have run changie new to create a changelog entry

github-actions · 2022-08-12T11:36:58Z

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

gshank

LGTM

joellabes · 2022-08-17T18:03:52Z

core/dbt/exceptions.py

        "  Requested range: {}\n"
-        "  Available versions: {}"
+        "  Compatible versions: {}\n"
+        "  (Not shown: versions incompatible with installed version of dbt-core)"


Should we recommend using the no-version-check flag here, like we do when a dbt project fails to run due to mismatching versions?

dbeatty10

There are two main cases in regards to pre-release versions of dbt-core:

excluded, because it's not within the range specified in require_dbt_version.
included, because it is within the range specified in require_dbt_version.

It looks like the first case is covered in the tests, but I couldn't tell if the second case is covered or not. Could you shed light if both are covered or not?

dbeatty10 · 2022-08-17T17:32:02Z

core/dbt/deps/registry.py

        )
-        available_latest = installable[-1]
+        latest_compatible = installable[-1]


Is it possible for installable to be empty?

Good call! We should be sure to raise a good exception before we try to access an element. I'll rearrange the logic accordingly

dbeatty10 · 2022-08-17T17:32:36Z

core/dbt/exceptions.py

+        "Could not find a matching compatible version for package {}\n"
        "  Requested range: {}\n"
-        "  Available versions: {}"
+        "  Compatible versions: {}\n"
+        "  (Not shown: versions incompatible with installed version of dbt-core)"


Very nice 🤩

dbeatty10 · 2022-08-17T18:41:42Z

test/unit/test_deps.py

+    def package(self, package_name, registry_base_url=None):
        try:
-            pkg = self.packages[name]
+            pkg = self.packages[package_name]
        except KeyError:
            return []
-        return list(pkg)
+        return pkg


Nice add, especially registry_base_url=None.

jtcohen6 · 2022-08-18T13:31:25Z

Prereleases! A great callout.

We're using the exact same dbt.semver.versions_compatible method here, that we're already using to check require-dbt-version here. That already contains handling for how to perform version comparison with prereleases.

As you note, it's possible for a pre-release version of dbt-core to be included or excluded by the require-dbt-version config defined in a package. The current logic performs two checks:

First, are there any package versions incompatible with the installed version of dbt-core? If so, remove them from the compatible_versions list.
Second, is the user willing to install a prerelease version? If not, remove any prerelease versions from the compatible_versions list to produce the installable list.

After both checks, if there are no compatible + desired versions remaining in installable, we'll raise an exception saying as much.

dbeatty10 · 2022-08-18T14:59:00Z

As you note, it's possible for a pre-release version of dbt-core to be included or excluded by the require-dbt-version config defined in a package. The current logic performs two checks:

First, are there any package versions incompatible with the installed version of dbt-core? If so, remove them from the compatible_versions list.

Second, is the user willing to install a prerelease version? If not, remove any prerelease versions from the compatible_versions list to produce the installable list.

It sounds like there are two different contexts to consider prereleases (which of course makes it harder for me to think clearly, 😅 ):

Prereleases of dbt-core
Prereleases of a dbt package

I'm curious if the following case is represented in the continuous integration testing or not:

user has a prerelease version of dbt-core
the package has at least one version that is compatible with that particular prerelease version of dbt-core

Also

Separately (but related), do we have functional tests that cover all these relevant permutations?

installed version of dbt-core is a prerelease -- (2) options: yes/no
there exists a package prerelease compatible with the installed version of dbt-core -- (2) options: yes/no
user is willing to install a prerelease version of the package -- (2) options: yes/no

This is 8 different permutations (if I'm doing the math correctly). Potentially, there are some permutations that are not relevant for some reason -- I didn't expand it out fully and consider each.

jtcohen6 · 2022-08-18T15:14:36Z

@dbeatty10 You're right to push on this! There may very well be an edge case we're missing.

There is definitely a distinction in this functionality between prerelease and final releases of dbt packages. That's already pretty well tested for.

For the purposes of this functionality, I don't actually think there's any distinction between running with a prerelease versus final release of dbt-core. The same method — dbt.semver.versions_compatible(current_dbt_version, require_dbt_version_range) — will return True or False for the current installed version of dbt-core, whether it's a prerelease or final release.

So for instance:

There's a new prerelease of dbt-utils with require-dbt-version: [">=1.3.0b1"]. Let's call it version 0.9.0a1
You're using dbt-core==1.3.0b1 locally
- You have install-prerelease: true, or a version: 0.9.0a1 spec that allows prereleases → you'd install that version
- You have install-prerelease: false in packages.yml → you won't install 0.9.0a1, instead you'll install the most recent compatible non-prerelease of dbt-utils
You're using dbt-core==1.2.1rc1 locally
- You will never install 0.9.0a1 because it is not compatible

dbeatty10 · 2022-08-19T17:04:24Z

test/unit/test_deps.py

+        })
+        resolved = resolve_packages(package_config.packages, mock.MagicMock(project_name='test'))
+        self.assertEqual(resolved[0].name, 'dbt-labs-test/a')
+        self.assertEqual(resolved[0].version, '0.1.4a1')


dbeatty10 · 2022-08-19T17:05:47Z

core/dbt/deps/registry.py

        installable = semver.filter_installable(
-            available, self.install_prerelease or prerelease_version_specified
+            compatible_versions, self.install_prerelease or prerelease_version_specified
        )
-        available_latest = installable[-1]
-
-        # for now, pick a version and then recurse. later on,
-        # we'll probably want to traverse multiple options
-        # so we can match packages. not going to make a difference
-        # right now.
-        target = semver.resolve_to_specific_version(range_, installable)
+        if installable:
+            # for now, pick a version and then recurse. later on,
+            # we'll probably want to traverse multiple options
+            # so we can match packages. not going to make a difference
+            # right now.
+            target = semver.resolve_to_specific_version(range_, installable)
+        else:
+            target = None
        if not target:
-            package_version_not_found(self.package, range_, installable)
+            package_version_not_found(self.package, range_, installable, should_version_check)
+        latest_compatible = installable[-1]


I might be misreading this, but it looks like this will throw an error if installable is empty or None when it tries to get the last item via installable[-1].

Do we have a test case for when there are no installable versions available after the semver filter?

Yeah! If installable is None or [], then we'll set target = None on line 146. That would cause us to raise the package_version_not_found exception, rather than proceed to get the indexed element [-1].

Does that logic feel too tricky? Another way you'd want to express it, or an additional annotation?

core/dbt/deps/registry.py

Co-authored-by: Doug Beatty <[email protected]>

dbeatty10

@jtcohen6 Ready to rock 🎸

* First cut at checking version compat for hub pkgs * Account for field rename * Add changelog entry * Update error message * Fix unit test * PR feedback * Try fixing test * Edit exception msg * Expand unit test to include pkg prerelease * Update core/dbt/deps/registry.py Co-authored-by: Doug Beatty <[email protected]> Co-authored-by: Doug Beatty <[email protected]>

…-labs#5651) * First cut at checking version compat for hub pkgs * Account for field rename * Add changelog entry * Update error message * Fix unit test * PR feedback * Try fixing test * Edit exception msg * Expand unit test to include pkg prerelease * Update core/dbt/deps/registry.py Co-authored-by: Doug Beatty <[email protected]> Co-authored-by: Doug Beatty <[email protected]>

First cut at checking version compat for hub pkgs

a5e5448

cla-bot bot added the cla:yes label Aug 12, 2022

jtcohen6 mentioned this pull request Aug 12, 2022

[CT-1035] [Feature] dbt deps should take require-dbt-version into account when resolving dependencies #5648

Closed

3 tasks

jtcohen6 added the Team:Language label Aug 12, 2022

jtcohen6 changed the title ~~First cut at checking version compat for hub pkgs~~ Check dbt-core version requirements when installing Hub packages Aug 12, 2022

jtcohen6 added 2 commits August 15, 2022 13:42

Account for field rename

181e75d

Add changelog entry

ab42c78

jtcohen6 mentioned this pull request Aug 15, 2022

Recreate dbt_utils specs with require_dbt_version dbt-labs/hub.getdbt.com#1770

Closed

jtcohen6 added 2 commits August 15, 2022 16:02

Update error message

8818bb9

Fix unit test

1e9f5d3

jtcohen6 marked this pull request as ready for review August 17, 2022 09:46

jtcohen6 requested review from a team as code owners August 17, 2022 09:46

jtcohen6 requested review from gshank and nathaniel-may August 17, 2022 09:46

gshank approved these changes Aug 17, 2022

View reviewed changes

jtcohen6 requested a review from dbeatty10 August 17, 2022 15:54

joellabes reviewed Aug 17, 2022

View reviewed changes

jtcohen6 mentioned this pull request Aug 17, 2022

Add require_dbt_version to Hub API dbt-labs/hubcap#146

Merged

dbeatty10 reviewed Aug 17, 2022

View reviewed changes

jtcohen6 added 3 commits August 18, 2022 15:33

PR feedback

243ce97

Try fixing test

a94b41c

Edit exception msg

3b5d53d

Expand unit test to include pkg prerelease

5de7117

dbeatty10 self-requested a review August 19, 2022 16:54

dbeatty10 reviewed Aug 19, 2022

View reviewed changes

core/dbt/deps/registry.py Show resolved Hide resolved

Update core/dbt/deps/registry.py

1c0e94a

Co-authored-by: Doug Beatty <[email protected]>

dbeatty10 approved these changes Aug 19, 2022

View reviewed changes

jtcohen6 merged commit 7f8d9a7 into main Aug 19, 2022

jtcohen6 deleted the feature/hub-install-compatible-versions branch August 19, 2022 21:13

jtcohen6 mentioned this pull request Sep 15, 2022

[CT-1182] [Feature] dbt_metrics should be part of Core #5840

Closed

3 tasks

dbeatty10 mentioned this pull request Dec 19, 2022

[CT-1670] [Feature] dbt deps should consider require-dbt-version when deciding which package version to install. #6454

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check dbt-core version requirements when installing Hub packages #5651

Check dbt-core version requirements when installing Hub packages #5651

jtcohen6 commented Aug 12, 2022 •

edited

Loading

github-actions bot commented Aug 12, 2022

gshank left a comment

joellabes Aug 17, 2022

jtcohen6 Aug 17, 2022

dbeatty10 left a comment

dbeatty10 Aug 17, 2022

jtcohen6 Aug 18, 2022

dbeatty10 Aug 17, 2022

dbeatty10 Aug 17, 2022

jtcohen6 commented Aug 18, 2022 •

edited

Loading

dbeatty10 commented Aug 18, 2022

jtcohen6 commented Aug 18, 2022

dbeatty10 Aug 19, 2022

dbeatty10 Aug 19, 2022

jtcohen6 Aug 19, 2022 •

edited

Loading

dbeatty10 left a comment

Check dbt-core version requirements when installing Hub packages #5651

Check dbt-core version requirements when installing Hub packages #5651

Conversation

jtcohen6 commented Aug 12, 2022 • edited Loading

Description

Checklist

github-actions bot commented Aug 12, 2022

gshank left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbeatty10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtcohen6 commented Aug 18, 2022 • edited Loading

dbeatty10 commented Aug 18, 2022

Also

jtcohen6 commented Aug 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtcohen6 Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

dbeatty10 left a comment

Choose a reason for hiding this comment

jtcohen6 commented Aug 12, 2022 •

edited

Loading

jtcohen6 commented Aug 18, 2022 •

edited

Loading

jtcohen6 Aug 19, 2022 •

edited

Loading