diff --git a/core/dbt/clients/jinja.py b/core/dbt/clients/jinja.py index e254d3a0780..7180fb79f71 100644 --- a/core/dbt/clients/jinja.py +++ b/core/dbt/clients/jinja.py @@ -3,8 +3,9 @@ import os import tempfile from contextlib import contextmanager +from dataclasses import dataclass from typing import ( - List, Union, Set, Optional, Dict, Any, Callable, Iterator, Type, NoReturn + List, Union, Set, Optional, Dict, Any, Iterator, Type, NoReturn ) import jinja2 @@ -164,6 +165,18 @@ def call_macro(self, *args, **kwargs): return e.value +@dataclass +class MacroProxy: + generator: 'MacroGenerator' + + @property + def node(self): + return self.generator.node + + def __call__(self, *args, **kwargs): + return self.generator.call_macro(*args, **kwargs) + + class MacroGenerator(BaseMacroGenerator): def __init__(self, node, context: Optional[Dict[str, Any]] = None) -> None: super().__init__(context) @@ -185,9 +198,9 @@ def exception_handler(self) -> Iterator[None]: e.stack.append(self.node) raise e - def __call__(self, context: Dict[str, Any]) -> Callable: + def __call__(self, context: Dict[str, Any]) -> MacroProxy: self.context = context - return self.call_macro + return MacroProxy(self) class QueryStringGenerator(BaseMacroGenerator): diff --git a/core/dbt/config/profile.py b/core/dbt/config/profile.py index adaf4d35192..618aecd8054 100644 --- a/core/dbt/config/profile.py +++ b/core/dbt/config/profile.py @@ -147,7 +147,8 @@ def _credentials_from_profile( @staticmethod def pick_profile_name( - args_profile_name: str, project_profile_name: Optional[str] = None, + args_profile_name: Optional[str], + project_profile_name: Optional[str] = None, ) -> str: profile_name = project_profile_name if args_profile_name is not None: @@ -370,7 +371,7 @@ def render_from_args( threads_override = getattr(args, 'threads', None) target_override = getattr(args, 'target', None) raw_profiles = read_profile(args.profiles_dir) - profile_name = cls.pick_profile_name(args.profile, + profile_name = cls.pick_profile_name(getattr(args, 'profile', None), project_profile_name) return cls.from_raw_profiles( diff --git a/core/dbt/context/configured.py b/core/dbt/context/configured.py index 6260a96cd13..9acdd1c1d24 100644 --- a/core/dbt/context/configured.py +++ b/core/dbt/context/configured.py @@ -1,5 +1,6 @@ -from typing import Callable, Any, Dict, Iterable, Union +from typing import Any, Dict, Iterable, Union +from dbt.clients.jinja import MacroProxy from dbt.contracts.connection import AdapterRequiredConfig from dbt.contracts.graph.manifest import Manifest from dbt.contracts.graph.parsed import ParsedMacro @@ -8,6 +9,7 @@ from dbt.context.base import contextproperty from dbt.context.target import TargetContext +from dbt.exceptions import raise_duplicate_macro_name class ConfiguredContext(TargetContext): @@ -23,31 +25,33 @@ def project_name(self) -> str: return self.config.project_name +Namespace = Union[Dict[str, MacroProxy], MacroProxy] + + class _MacroNamespace: def __init__(self, root_package, search_package): self.root_package = root_package self.search_package = search_package - self.globals: Dict[str, Callable] = {} - self.locals: Dict[str, Callable] = {} - self.packages: Dict[str, Dict[str, Callable]] = {} + self.globals: Dict[str, MacroProxy] = {} + self.locals: Dict[str, MacroProxy] = {} + self.packages: Dict[str, Dict[str, MacroProxy]] = {} def add_macro(self, macro: ParsedMacro, ctx: Dict[str, Any]): macro_name: str = macro.name - macro_func: Callable = macro.generator(ctx) + macro_func: MacroProxy = macro.generator(ctx) - # put plugin macros into the nam + # put plugin macros into the root namespace if macro.package_name in PACKAGES: namespace: str = GLOBAL_PROJECT_NAME else: namespace = macro.package_name if namespace not in self.packages: - value: Dict[str, Callable] = {} + value: Dict[str, MacroProxy] = {} self.packages[namespace] = value - # TODO: if macro_name exists already, that means you had duplicate - # names in the same namespace (probably a plugin vs core/multiple - # plugins issue). That should be an error, right? + if macro_name in self.packages[namespace]: + raise_duplicate_macro_name(macro_func.node, macro, namespace) self.packages[namespace][macro_name] = macro_func if namespace == self.search_package: @@ -60,7 +64,7 @@ def add_macros(self, macros: Iterable[ParsedMacro], ctx: Dict[str, Any]): self.add_macro(macro, ctx) def get_macro_dict(self) -> Dict[str, Any]: - root_namespace: Dict[str, Union[Dict[str, Callable], Callable]] = {} + root_namespace: Dict[str, Namespace] = {} root_namespace.update(self.packages) root_namespace.update(self.globals) diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index ed7732c88b8..a5a4034700f 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -753,9 +753,10 @@ def resolve_ref( # first pass: look for models in the current_project # second pass: look for models in the node's package # final pass: look for models in any package - # todo: exclude the packages we have already searched. overriding - # a package model in another package doesn't necessarily work atm - candidates = [current_project, node_package, None] + if current_project == node_package: + candidates = [current_project, None] + else: + candidates = [current_project, node_package, None] for candidate in candidates: target_model = self.find_refable_by_name( target_model_name, diff --git a/core/dbt/contracts/graph/parsed.py b/core/dbt/contracts/graph/parsed.py index 2f66394f878..9695ebaec25 100644 --- a/core/dbt/contracts/graph/parsed.py +++ b/core/dbt/contracts/graph/parsed.py @@ -10,7 +10,6 @@ Tuple, NewType, MutableMapping, - Callable, ) from hologram import JsonSchemaMixin @@ -507,7 +506,7 @@ def local_vars(self): return {} @property - def generator(self) -> Callable[[Dict[str, Any]], Callable]: + def generator(self) -> MacroGenerator: """ Returns a function that can be called to render the macro results. """ diff --git a/core/dbt/exceptions.py b/core/dbt/exceptions.py index e80aeb98bfa..86727bf418e 100644 --- a/core/dbt/exceptions.py +++ b/core/dbt/exceptions.py @@ -643,6 +643,29 @@ def approximate_relation_match(target, relation): relation=relation)) +def raise_duplicate_macro_name(node_1, node_2, namespace) -> NoReturn: + duped_name = node_1.namespace + if node_1.package_name != node_2.package_name: + extra = ( + ' ({} and {} are both in the {} namespace)' + .format(node_1.package_name, node_2.package_name, namespace) + ) + else: + extra = '' + + raise_compiler_error( + 'dbt found two macros with the name "{}" in the namespace "{}"{}. ' + 'Since these macros have the same name and exist in the same ' + 'namespace, dbt will be unable to decide which to call. To fix this, ' + 'change the name of one of these macros:\n- {} ({})\n- {} ({})' + .format( + duped_name, namespace, extra, + node_1.unique_id, node_1.original_file_path, + node_2.unique_id, node_2.original_file_path + ) + ) + + def raise_duplicate_resource_name(node_1, node_2): duped_name = node_1.name diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index bbbce8f0bfe..8e15fca73a1 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -226,10 +226,10 @@ def _choose_target_name(self, profile_name: str): renderer = ConfigRenderer(generate_base_context(self.cli_vars)) target_name, _ = Profile.render_profile( - raw_profile, - self.profile_name, - getattr(self.args, 'target', None), - renderer + raw_profile=raw_profile, + profile_name=profile_name, + target_override=getattr(self.args, 'target', None), + renderer=renderer ) return target_name @@ -256,7 +256,7 @@ def _load_profile(self): renderer = ConfigRenderer(generate_base_context(self.cli_vars)) for profile_name in profile_names: try: - profile: Profile = QueryCommentedProfile.from_args( + profile: Profile = QueryCommentedProfile.render_from_args( self.args, renderer, profile_name ) except dbt.exceptions.DbtConfigError as exc: diff --git a/test/unit/test_context.py b/test/unit/test_context.py index c55ca1086a4..8375c4226b8 100644 --- a/test/unit/test_context.py +++ b/test/unit/test_context.py @@ -3,7 +3,13 @@ from typing import Set, Dict, Any from unittest import mock -from dbt.contracts.graph.parsed import ParsedModelNode, NodeConfig, DependsOn, ParsedMacro +import pytest + +# make sure 'postgres' is in PACKAGES +from dbt.adapters import postgres # noqa +from dbt.contracts.graph.parsed import ( + ParsedModelNode, NodeConfig, DependsOn, ParsedMacro +) from dbt.context import base, target, configured, providers, docs from dbt.node_types import NodeType import dbt.exceptions @@ -317,28 +323,31 @@ def mock_model(): ) -import pytest - - @pytest.fixture def get_adapter(): with mock.patch.object(providers, 'get_adapter') as patch: yield patch -def test_query_header_context(): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +@pytest.fixture +def config(): + return config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) + + +@pytest.fixture +def manifest(config): + return mock_manifest(config) + +def test_query_header_context(config, manifest): ctx = configured.generate_query_header_context( config=config, - manifest=mock_manifest(config), + manifest=manifest, ) assert_has_keys(REQUIRED_QUERY_HEADER_KEYS, MAYBE_KEYS, ctx) -def test_macro_parse_context(get_adapter): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) - manifest = mock_manifest(config) +def test_macro_parse_context(config, manifest, get_adapter): ctx = providers.generate_parser_macro( macro=manifest.macros['macro.root.macro_a'], config=config, @@ -348,9 +357,7 @@ def test_macro_parse_context(get_adapter): assert_has_keys(REQUIRED_MACRO_KEYS, MAYBE_KEYS, ctx) -def test_macro_runtime_context(get_adapter): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) - manifest = mock_manifest(config) +def test_macro_runtime_context(config, manifest, get_adapter): ctx = providers.generate_runtime_macro( macro=manifest.macros['macro.root.macro_a'], config=config, @@ -360,34 +367,45 @@ def test_macro_runtime_context(get_adapter): assert_has_keys(REQUIRED_MACRO_KEYS, MAYBE_KEYS, ctx) -def test_model_parse_context(get_adapter): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +def test_model_parse_context(config, manifest, get_adapter): ctx = providers.generate_parser_model( model=mock_model(), config=config, - manifest=mock_manifest(config), + manifest=manifest, source_config=mock.MagicMock(), ) assert_has_keys(REQUIRED_MODEL_KEYS, MAYBE_KEYS, ctx) -def test_model_runtime_context(get_adapter): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +def test_model_runtime_context(config, manifest, get_adapter): ctx = providers.generate_runtime_model( model=mock_model(), config=config, - manifest=mock_manifest(config), + manifest=manifest, ) assert_has_keys(REQUIRED_MODEL_KEYS, MAYBE_KEYS, ctx) -def test_docs_parse_context(): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +def test_docs_parse_context(config): ctx = docs.generate_parser_docs(config, mock_model(), []) assert_has_keys(REQUIRED_DOCS_KEYS, MAYBE_KEYS, ctx) -def test_docs_runtime_context(): - config = config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +def test_docs_runtime_context(config): ctx = docs.generate_runtime_docs(config, mock_model(), [], 'root') assert_has_keys(REQUIRED_DOCS_KEYS, MAYBE_KEYS, ctx) + + +def test_macro_namespace(config, manifest): + mn = configured._MacroNamespace('root', 'search') + mn.add_macros(manifest.macros.values(), {}) + + # same pkg, same name + with pytest.raises(dbt.exceptions.CompilationException): + mn.add_macros(manifest.macros.values(), {}) + + mn.add_macro(mock_macro('some_macro', 'dbt'), {}) + + # same namespace, same name (different pkg!) + with pytest.raises(dbt.exceptions.CompilationException): + mn.add_macro(mock_macro('some_macro', 'dbt_postgres'), {})