Check dbt-core version requirements when installing Hub packages (dbt…

…-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]>
Gousto · Sep 16, 2022 · 541b337 · 541b337
1 parent ab5bfc0
commit 541b337
Show file tree

Hide file tree

Showing 6 changed files with 141 additions and 28 deletions.
diff --git a/.changes/unreleased/Features-20220815-134312.yaml b/.changes/unreleased/Features-20220815-134312.yaml
@@ -0,0 +1,7 @@
+kind: Features
+body: Check dbt-core version requirements when installing Hub packages
+time: 2022-08-15T13:43:12.965143+01:00
+custom:
+  Author: jtcohen6
+  Issue: "5648"
+  PR: "5651"
diff --git a/core/dbt/clients/registry.py b/core/dbt/clients/registry.py
@@ -14,6 +14,7 @@
 )
 from dbt.utils import memoized, _connection_exception_retry as connection_exception_retry
 from dbt import deprecations
+from dbt import semver
 import os
 
 if os.getenv("DBT_PACKAGE_HUB_URL"):
@@ -125,10 +126,37 @@ def package_version(package_name, version, registry_base_url=None) -> Dict[str,
     return response[version]
 
 
-def get_available_versions(package_name) -> List["str"]:
+def is_compatible_version(package_spec, dbt_version) -> bool:
+    require_dbt_version = package_spec.get("require_dbt_version")
+    if not require_dbt_version:
+        # if version requirements are missing or empty, assume any version is compatible
+        return True
+    else:
+        # determine whether dbt_version satisfies this package's require-dbt-version config
+        if not isinstance(require_dbt_version, list):
+            require_dbt_version = [require_dbt_version]
+        supported_versions = [
+            semver.VersionSpecifier.from_version_string(v) for v in require_dbt_version
+        ]
+        return semver.versions_compatible(dbt_version, *supported_versions)
+
+
+def get_compatible_versions(package_name, dbt_version, should_version_check) -> List["str"]:
     # returns a list of all available versions of a package
     response = package(package_name)
-    return list(response)
+
+    # if the user doesn't care about installing compatible versions, just return them all
+    if not should_version_check:
+        return list(response)
+
+    # otherwise, only return versions that are compatible with the installed version of dbt-core
+    else:
+        compatible_versions = [
+            pkg_version
+            for pkg_version, info in response.items()
+            if is_compatible_version(info, dbt_version)
+        ]
+        return compatible_versions
 
 
 def _get_index(registry_base_url=None):

diff --git a/core/dbt/deps/registry.py b/core/dbt/deps/registry.py
@@ -3,6 +3,8 @@
 from typing import List
 
 from dbt import semver
+from dbt import flags
+from dbt.version import get_installed_version
 from dbt.clients import registry, system
 from dbt.contracts.project import (
     RegistryPackageMetadata,
@@ -125,20 +127,27 @@ def resolved(self) -> RegistryPinnedPackage:
             new_msg = "Version error for package {}: {}".format(self.name, e)
             raise DependencyException(new_msg) from e
 
-        available = registry.get_available_versions(self.package)
+        should_version_check = bool(flags.VERSION_CHECK)
+        dbt_version = get_installed_version()
+        compatible_versions = registry.get_compatible_versions(
+            self.package, dbt_version, should_version_check
+        )
         prerelease_version_specified = any(bool(version.prerelease) for version in self.versions)
         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)
+            # raise an exception if no installable target version is found
+            package_version_not_found(self.package, range_, installable, should_version_check)
+        latest_compatible = installable[-1]
         return RegistryPinnedPackage(
-            package=self.package, version=target, version_latest=available_latest
+            package=self.package, version=target, version_latest=latest_compatible
         )
diff --git a/core/dbt/deps/resolver.py b/core/dbt/deps/resolver.py
@@ -120,7 +120,6 @@ def resolve_packages(
 ) -> List[PinnedPackage]:
     pending = PackageListing.from_contracts(packages)
     final = PackageListing()
-
     renderer = DbtProjectYamlRenderer(config, config.cli_vars)
 
     while pending:

diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py
@@ -776,13 +776,25 @@ def package_not_found(package_name):
     raise_dependency_error("Package {} was not found in the package index".format(package_name))
 
 
-def package_version_not_found(package_name, version_range, available_versions):
+def package_version_not_found(
+    package_name, version_range, available_versions, should_version_check
+):
     base_msg = (
-        "Could not find a matching version for package {}\n"
+        "Could not find a matching compatible version for package {}\n"
         "  Requested range: {}\n"
-        "  Available versions: {}"
+        "  Compatible versions: {}\n"
     )
-    raise_dependency_error(base_msg.format(package_name, version_range, available_versions))
+    addendum = (
+        (
+            "\n"
+            "  Not shown: package versions incompatible with installed version of dbt-core\n"
+            "  To include them, run 'dbt --no-version-check deps'"
+        )
+        if should_version_check
+        else ""
+    )
+    msg = base_msg.format(package_name, version_range, available_versions) + addendum
+    raise_dependency_error(msg)
 
 
 def invalid_materialization_argument(name, argument):

diff --git a/test/unit/test_deps.py b/test/unit/test_deps.py
@@ -1,3 +1,5 @@
+from copy import deepcopy
+
 import unittest
 from unittest import mock
 
@@ -6,6 +8,7 @@
 from dbt.deps.git import GitUnpinnedPackage
 from dbt.deps.local import LocalUnpinnedPackage
 from dbt.deps.registry import RegistryUnpinnedPackage
+from dbt.clients.registry import is_compatible_version
 from dbt.deps.resolver import resolve_packages
 from dbt.contracts.project import (
     LocalPackage,
@@ -15,6 +18,7 @@
 
 from dbt.contracts.project import PackageConfig
 from dbt.semver import VersionSpecifier
+from dbt.version import get_installed_version
 
 from dbt.dataclass_schema import ValidationError
 
@@ -114,13 +118,13 @@ def setUp(self):
         self.patcher = mock.patch('dbt.deps.registry.registry')
         self.registry = self.patcher.start()
         self.index_cached = self.registry.index_cached
-        self.get_available_versions = self.registry.get_available_versions
+        self.get_compatible_versions = self.registry.get_compatible_versions
         self.package_version = self.registry.package_version
 
         self.index_cached.return_value = [
             'dbt-labs-test/a',
         ]
-        self.get_available_versions.return_value = [
+        self.get_compatible_versions.return_value = [
             '0.1.2', '0.1.3', '0.1.4a1'
         ]
         self.package_version.return_value = {
@@ -235,11 +239,11 @@ def test_resolve_missing_version(self):
         with self.assertRaises(dbt.exceptions.DependencyException) as exc:
             a.resolved()
         msg = (
-            "Could not find a matching version for package "
+            "Could not find a matching compatible version for package "
             "dbt-labs-test/a\n  Requested range: =0.1.4, =0.1.4\n  "
-            "Available versions: ['0.1.2', '0.1.3']"
+            "Compatible versions: ['0.1.2', '0.1.3']\n"
         )
-        self.assertEqual(msg, str(exc.exception))
+        assert msg in str(exc.exception)
 
     def test_resolve_conflict(self):
         a_contract = RegistryPackage(
@@ -509,12 +513,19 @@ def __init__(self, packages):
     def index_cached(self, registry_base_url=None):
         return sorted(self.packages)
 
-    def get_available_versions(self, name):
+    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
+
+    def get_compatible_versions(self, package_name, dbt_version, should_version_check):
+        packages = self.package(package_name)
+        return [
+            pkg_version for pkg_version, info in packages.items()
+            if is_compatible_version(info, dbt_version)
+        ]
 
     def package_version(self, name, version):
         try:
@@ -524,7 +535,13 @@ def package_version(self, name, version):
 
 
 class TestPackageSpec(unittest.TestCase):
-    def setUp(self):
+    def setUp(self):    
+        dbt_version = get_installed_version()
+        next_version = deepcopy(dbt_version)
+        next_version.minor = str(int(next_version.minor) + 1)
+        next_version.prerelease = None
+        require_next_version = ">" + next_version.to_version_string()
+
         self.patcher = mock.patch('dbt.deps.registry.registry')
         self.registry = self.patcher.start()
         self.mock_registry = MockRegistry(packages={
@@ -556,6 +573,36 @@ def setUp(self):
                         'extra': 'field',
                     },
                     'newfield': ['another', 'value'],
+                },
+                '0.1.4a1': {
+                    'id': 'dbt-labs-test/a/0.1.3a1',
+                    'name': 'a',
+                    'version': '0.1.4a1',
+                    'packages': [],
+                    '_source': {
+                        'blahblah': 'asdfas',
+                    },
+                    'downloads': {
+                        'tarball': 'https://example.com/invalid-url!',
+                        'extra': 'field',
+                    },
+                    'newfield': ['another', 'value'],
+                },
+                '0.2.0': {
+                    'id': 'dbt-labs-test/a/0.2.0',
+                    'name': 'a',
+                    'version': '0.2.0',
+                    'packages': [],
+                    '_source': {
+                        'blahblah': 'asdfas',
+                    },
+                    # this one shouldn't be picked!
+                    'require_dbt_version': require_next_version,
+                    'downloads': {
+                        'tarball': 'https://example.com/invalid-url!',
+                        'extra': 'field',
+                    },
+                    'newfield': ['another', 'value'],
                 }
             },
             'dbt-labs-test/b': {
@@ -577,7 +624,7 @@ def setUp(self):
         })
 
         self.registry.index_cached.side_effect = self.mock_registry.index_cached
-        self.registry.get_available_versions.side_effect = self.mock_registry.get_available_versions
+        self.registry.get_compatible_versions.side_effect = self.mock_registry.get_compatible_versions
         self.registry.package_version.side_effect = self.mock_registry.package_version
 
     def tearDown(self):
@@ -596,3 +643,14 @@ def test_dependency_resolution(self):
         self.assertEqual(resolved[0].version, '0.1.3')
         self.assertEqual(resolved[1].name, 'dbt-labs-test/b')
         self.assertEqual(resolved[1].version, '0.2.1')
+
+    def test_dependency_resolution_allow_prerelease(self):
+        package_config = PackageConfig.from_dict({
+            'packages': [
+                {'package': 'dbt-labs-test/a', 'version': '>0.1.2', 'install_prerelease': True},
+                {'package': 'dbt-labs-test/b', 'version': '0.2.1'},
+            ],
+        })
+        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')