From 3af88b06998f5e76a04df68162b83b4966b9d34e Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Mon, 29 Apr 2019 13:58:24 -0600 Subject: [PATCH 1/2] Update PackageListing to handle git packages having or not having .git First write wins, on the assumption that all matching URLs are valid --- core/dbt/task/deps.py | 38 +++++++++++++++++-- .../test_simple_dependency.py | 29 ++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 3e282c25d7b..44daaae9bfc 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -216,9 +216,17 @@ class GitPackage(Package): def __init__(self, *args, **kwargs): super(GitPackage, self).__init__(*args, **kwargs) + # strip trailing '.git's from urls self._checkout_name = hashlib.md5(six.b(self.git)).hexdigest() self.version = self._contents.get('revision') + @property + def other_name(self): + if self.git.endswith('.git'): + return self.git[:-4] + else: + return self.git + '.git' + @property def name(self): return self.git @@ -368,14 +376,38 @@ def _parse_package(dict_): class PackageListing(AttrDict): + def __contains__(self, package): + if isinstance(package, basestring): + return super(PackageListing, self).__contains__(package) + elif isinstance(package, GitPackage): + return package.name in self or package.other_name in self + else: + return package.name in self + + def __setitem__(self, key, value): + if isinstance(key, basestring): + super(PackageListing, self).__setitem__(key, value) + elif isinstance(key, GitPackage) and key.other_name in self: + self[key.other_name] = value + else: + self[key.name] = value + + def __getitem__(self, key): + if isinstance(key, basestring): + return super(PackageListing, self).__getitem__(key) + elif isinstance(key, GitPackage) and key.other_name in self: + return self[key.other_name] + else: + return self[key.name] def incorporate(self, package): if not isinstance(package, Package): package = _parse_package(package) - if package.name not in self: - self[package.name] = package + + if package in self: + self[package] = self[package].incorporate(package) else: - self[package.name] = self[package.name].incorporate(package) + self[package] = package @classmethod def create(cls, parsed_yaml): diff --git a/test/integration/006_simple_dependency_test/test_simple_dependency.py b/test/integration/006_simple_dependency_test/test_simple_dependency.py index 1eac417cab8..256db44a223 100644 --- a/test/integration/006_simple_dependency_test/test_simple_dependency.py +++ b/test/integration/006_simple_dependency_test/test_simple_dependency.py @@ -63,6 +63,35 @@ def test_simple_dependency_with_models(self): self.assertEqual(created_models['view_model'], 'view') self.assertEqual(created_models['view_summary'], 'view') + +class TestSimpleDependencyWithDuplicates(DBTIntegrationTest): + @property + def schema(self): + return "simple_dependency_006" + + @property + def models(self): + return "test/integration/006_simple_dependency_test/models" + + @property + def packages_config(self): + # dbt should convert these into a single dependency internally + return { + "packages": [ + { + 'git': 'https://github.com/fishtown-analytics/dbt-integration-project' + }, + { + 'git': 'https://github.com/fishtown-analytics/dbt-integration-project.git' + } + ] + } + + @use_profile('postgres') + def test_simple_dependency(self): + self.run_dbt(["deps"]) + + class TestSimpleDependencyBranch(DBTIntegrationTest): def setUp(self): From 12f0887d28d2c572f2235a0a363f33def1ebfd12 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 8 May 2019 10:15:37 -0600 Subject: [PATCH 2/2] PR feedback remove incorrect comment fix an issue where we looked deps up by the wrong key add a test --- core/dbt/task/deps.py | 9 +++-- .../test_simple_dependency.py | 36 ++++++++++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 44daaae9bfc..aaf37979ef6 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -216,7 +216,6 @@ class GitPackage(Package): def __init__(self, *args, **kwargs): super(GitPackage, self).__init__(*args, **kwargs) - # strip trailing '.git's from urls self._checkout_name = hashlib.md5(six.b(self.git)).hexdigest() self.version = self._contents.get('revision') @@ -520,16 +519,16 @@ def run(self): while pending_deps: sub_deps = PackageListing.create([]) - for name, package in pending_deps.items(): + for package in pending_deps.values(): final_deps.incorporate(package) - final_deps[name].resolve_version() - target_config = final_deps[name].fetch_metadata(self.config) + final_deps[package].resolve_version() + target_config = final_deps[package].fetch_metadata(self.config) sub_deps.incorporate_from_yaml(target_config.packages) pending_deps = sub_deps self._check_for_duplicate_project_names(final_deps) - for _, package in final_deps.items(): + for package in final_deps.values(): logger.info('Installing %s', package) package.install(self.config) logger.info(' Installed from %s\n', package.nice_version_name()) diff --git a/test/integration/006_simple_dependency_test/test_simple_dependency.py b/test/integration/006_simple_dependency_test/test_simple_dependency.py index 256db44a223..cd0ac9847fa 100644 --- a/test/integration/006_simple_dependency_test/test_simple_dependency.py +++ b/test/integration/006_simple_dependency_test/test_simple_dependency.py @@ -1,5 +1,7 @@ +import os from test.integration.base import DBTIntegrationTest, use_profile + class TestSimpleDependency(DBTIntegrationTest): def setUp(self): @@ -88,8 +90,40 @@ def packages_config(self): } @use_profile('postgres') - def test_simple_dependency(self): + def test_postgres_simple_dependency_deps(self): + self.run_dbt(["deps"]) + + +class TestRekeyedDependencyWithSubduplicates(DBTIntegrationTest): + @property + def schema(self): + return "simple_dependency_006" + + @property + def models(self): + return "test/integration/006_simple_dependency_test/models" + + @property + def packages_config(self): + # dbt-event-logging@0.1.5 requires dbt-utils.git@0.1.12, which the + # package config handling should detect + return { + 'packages': [ + { + 'git': 'https://github.com/fishtown-analytics/dbt-utils', + 'revision': '0.1.12', + }, + { + 'git': 'https://github.com/fishtown-analytics/dbt-event-logging.git', + 'revision': '0.1.5', + } + ] + } + + @use_profile('postgres') + def test_postgres_simple_dependency_deps(self): self.run_dbt(["deps"]) + self.assertEqual(len(os.listdir('dbt_modules')), 2) class TestSimpleDependencyBranch(DBTIntegrationTest):