Skip to content

Commit

Permalink
Merge pull request #2030 from fishtown-analytics/fix/detect-deps-same…
Browse files Browse the repository at this point in the history
…-name-as-root

detect deps with the same name as the root
  • Loading branch information
beckjake authored Jan 5, 2020
2 parents 1235b3f + f1ef68c commit 39f92e5
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 8 deletions.
18 changes: 13 additions & 5 deletions core/dbt/deps/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from dbt.exceptions import raise_dependency_error, InternalException

from dbt.config import Project
from dbt.deps.base import BasePackage, PinnedPackage, UnpinnedPackage
from dbt.deps.local import LocalUnpinnedPackage
from dbt.deps.git import GitUnpinnedPackage
Expand Down Expand Up @@ -95,21 +96,28 @@ def __iter__(self) -> Iterator[UnpinnedPackage]:


def _check_for_duplicate_project_names(
final_deps: List[PinnedPackage], config
final_deps: List[PinnedPackage], config: Project
):
seen = set()
for package in final_deps:
project_name = package.get_project_name(config)
if project_name in seen:
raise_dependency_error(
'Found duplicate project {}. This occurs when a dependency'
' has the same project name as some other dependency.'
.format(project_name))
f'Found duplicate project "{project_name}". This occurs when '
'a dependency has the same project name as some other '
'dependency.'
)
elif project_name == config.project_name:
raise_dependency_error(
'Found a dependency with the same name as the root project '
f'"{project_name}". Package names must be unique in a project.'
' Please rename one of these packages.'
)
seen.add(project_name)


def resolve_packages(
packages: List[PackageContract], config
packages: List[PackageContract], config: Project
) -> List[PinnedPackage]:
pending = PackageListing.from_contracts(packages)
final = PackageListing()
Expand Down
11 changes: 10 additions & 1 deletion core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from dbt.config import Project, RuntimeConfig
from dbt.contracts.graph.compiled import CompileResultNode
from dbt.contracts.graph.manifest import Manifest, FilePath, FileHash
from dbt.exceptions import raise_compiler_error
from dbt.parser.base import BaseParser
from dbt.parser.analysis import AnalysisParser
from dbt.parser.data_test import DataTestParser
Expand Down Expand Up @@ -421,7 +422,15 @@ def load_all_projects(config) -> Mapping[str, Project]:
internal_project_names(),
_project_directories(config)
)
all_projects.update(_load_projects(config, project_paths))
for project_name, project in _load_projects(config, project_paths):
if project_name in all_projects:
raise_compiler_error(
f'dbt found more than one package with the name '
f'"{project_name}" included in this project. Package names '
f'must be unique in a project. Please rename one of these '
f'packages.'
)
all_projects[project_name] = project
return all_projects


Expand Down
5 changes: 4 additions & 1 deletion core/dbt/task/deps.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from typing import Optional

import dbt.utils
import dbt.deprecations
import dbt.exceptions

from dbt.config import Project
from dbt.deps.base import downloads_directory
from dbt.deps.resolver import resolve_packages

Expand All @@ -12,7 +15,7 @@


class DepsTask(ProjectOnlyTask):
def __init__(self, args, config=None):
def __init__(self, args, config: Optional[Project] = None):
super().__init__(args=args, config=config)

def track_package_install(self, package_name, source_type, version):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: 'test'
version: '1.0'

profile: 'default'
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from test.integration.base import DBTIntegrationTest, use_profile
import os
import json
import shutil
from unittest import mock

import dbt.semver
Expand Down Expand Up @@ -171,3 +173,35 @@ def test_postgres_hook_dependency(self):
results = self.run_dbt(["run", '--vars', cli_vars])
self.assertEqual(len(results), 2)
self.assertTablesEqual('actual', 'expected')


class TestSimpleDependencyDuplicateName(DBTIntegrationTest):
@property
def schema(self):
return "local_dependency_006"

@property
def models(self):
return "local_models"

@property
def packages_config(self):
return {
"packages": [
{
'local': 'duplicate_dependency'
}
]
}

@use_profile('postgres')
def test_postgres_local_dependency_same_name(self):
with self.assertRaises(dbt.exceptions.DependencyException):
self.run_dbt(['deps'], expect_pass=False)

@use_profile('postgres')
def test_postgres_local_dependency_same_name_sneaky(self):
os.makedirs('dbt_modules')
shutil.copytree('./duplicate_dependency', './dbt_modules/duplicate_dependency')
with self.assertRaises(dbt.exceptions.CompilationException):
self.run_dbt(['compile'], expect_pass=False)
2 changes: 1 addition & 1 deletion test/unit/test_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ def test_dependency_resolution(self):
{'package': 'fishtown-analytics-test/b', 'version': '0.2.1'},
],
})
resolved = resolve_packages(package_config.packages, None)
resolved = resolve_packages(package_config.packages, mock.MagicMock(project_name='test'))
self.assertEqual(len(resolved), 2)
self.assertEqual(resolved[0].name, 'fishtown-analytics-test/a')
self.assertEqual(resolved[0].version, '0.1.3')
Expand Down

0 comments on commit 39f92e5

Please sign in to comment.