From dd2633dfcb385cbdd166163e8e81b03d003da386 Mon Sep 17 00:00:00 2001 From: Jeremy Cohen Date: Sun, 2 May 2021 18:01:57 -0400 Subject: [PATCH] Include parent adapters in dispatch (#3296) * Add test, expect fail * Include parent adapters in dispatch * Use adapter type, not credentials type * Adjust adapter_macro deprecation test * fix test/unit/test_context.py to use postgres profile * Add changelog note * Redshift default column encoding now AUTO Co-authored-by: Gerda Shank --- CHANGELOG.md | 1 + core/dbt/context/providers.py | 13 +++++---- .../test_deprecations.py | 17 ++++++++++++ .../dispatch-inheritance-models/model.sql | 2 ++ .../016_macro_tests/macros/my_macros.sql | 17 +++++++++++- .../016_macro_tests/test_macros.py | 14 ++++++++++ .../test_docs_generate.py | 2 +- test/unit/test_context.py | 27 ++++++++++++++++--- 8 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 test/integration/016_macro_tests/dispatch-inheritance-models/model.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 808ac291e68..366a37c95af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ - Switch from externally storing parsing state in ParseResult object to using Manifest ([#3163](http://github.com/fishtown-analytics/dbt/issues/3163), [#3219](https://github.com/fishtown-analytics/dbt/pull/3219)) - Switch from loading project files in separate parsers to loading in one place([#3244](http://github.com/fishtown-analytics/dbt/issues/3244), [#3248](https://github.com/fishtown-analytics/dbt/pull/3248)) - Update schema/generic tests to use test materialization when executing. ([#3192](https://github.com/fishtown-analytics/dbt/issues/3192), [#3286](https://github.com/fishtown-analytics/dbt/pull/3286)) +- For adapters that inherit from other adapters (e.g. `dbt-postgres` → `dbt-redshift`), `adapter.dispatch()` will now include parent macro implementations as viable candidates ([#2923](https://github.com/fishtown-analytics/dbt/issues/2923), [#3296](https://github.com/fishtown-analytics/dbt/pull/3296)) Contributors: - [@yu-iskw](https://github.com/yu-iskw) ([#2928](https://github.com/fishtown-analytics/dbt/pull/2928)) diff --git a/core/dbt/context/providers.py b/core/dbt/context/providers.py index 7f85db245ad..ba14adabb62 100644 --- a/core/dbt/context/providers.py +++ b/core/dbt/context/providers.py @@ -8,7 +8,9 @@ from dbt import deprecations from dbt.adapters.base.column import Column -from dbt.adapters.factory import get_adapter, get_adapter_package_names +from dbt.adapters.factory import ( + get_adapter, get_adapter_package_names, get_adapter_type_names +) from dbt.clients import agate_helper from dbt.clients.jinja import get_rendered, MacroGenerator, MacroStack from dbt.config import RuntimeConfig, Project @@ -107,10 +109,11 @@ def commit(self): return self._adapter.commit_if_has_connection() def _get_adapter_macro_prefixes(self) -> List[str]: - # a future version of this could have plugins automatically call fall - # back to their dependencies' dependencies by using - # `get_adapter_type_names` instead of `[self.config.credentials.type]` - search_prefixes = [self._adapter.type(), 'default'] + # order matters for dispatch: + # 1. current adapter + # 2. any parent adapters (dependencies) + # 3. 'default' + search_prefixes = get_adapter_type_names(self._adapter.type()) + ['default'] return search_prefixes def dispatch( diff --git a/test/integration/012_deprecation_tests/test_deprecations.py b/test/integration/012_deprecation_tests/test_deprecations.py index 1a42a14da60..a8376be1145 100644 --- a/test/integration/012_deprecation_tests/test_deprecations.py +++ b/test/integration/012_deprecation_tests/test_deprecations.py @@ -110,6 +110,14 @@ def test_postgres_adapter_macro_fail(self): @use_profile('redshift') def test_redshift_adapter_macro(self): + self.assertEqual(deprecations.active_deprecations, set()) + # pick up the postgres macro + self.run_dbt(strict=False) + expected = {'adapter-macro'} + self.assertEqual(expected, deprecations.active_deprecations) + + @use_profile('bigquery') + def test_bigquery_adapter_macro(self): self.assertEqual(deprecations.active_deprecations, set()) # picked up the default -> error with self.assertRaises(dbt.exceptions.CompilationException) as exc: @@ -147,6 +155,15 @@ def test_postgres_adapter_macro_pkg_fail(self): @use_profile('redshift') def test_redshift_adapter_macro_pkg(self): + self.assertEqual(deprecations.active_deprecations, set()) + # pick up the postgres macro + self.assertEqual(deprecations.active_deprecations, set()) + self.run_dbt(strict=False) + expected = {'adapter-macro'} + self.assertEqual(expected, deprecations.active_deprecations) + + @use_profile('bigquery') + def test_bigquery_adapter_macro_pkg(self): self.assertEqual(deprecations.active_deprecations, set()) # picked up the default -> error with self.assertRaises(dbt.exceptions.CompilationException) as exc: diff --git a/test/integration/016_macro_tests/dispatch-inheritance-models/model.sql b/test/integration/016_macro_tests/dispatch-inheritance-models/model.sql new file mode 100644 index 00000000000..7b8c49be345 --- /dev/null +++ b/test/integration/016_macro_tests/dispatch-inheritance-models/model.sql @@ -0,0 +1,2 @@ +{{ dispatch_to_parent() }} +select 1 as id diff --git a/test/integration/016_macro_tests/macros/my_macros.sql b/test/integration/016_macro_tests/macros/my_macros.sql index d29007557d9..a51cee51e5e 100644 --- a/test/integration/016_macro_tests/macros/my_macros.sql +++ b/test/integration/016_macro_tests/macros/my_macros.sql @@ -15,8 +15,23 @@ {% endmacro %} -{# there is no no default__dispatch_to_nowhere! #} +{# there is no default__dispatch_to_nowhere! #} {% macro dispatch_to_nowhere() %} {% set macro = adapter.dispatch('dispatch_to_nowhere') %} {{ macro() }} {% endmacro %} + + +{% macro dispatch_to_parent() %} + {% set macro = adapter.dispatch('dispatch_to_parent') %} + {{ macro() }} +{% endmacro %} + +{% macro default__dispatch_to_parent() %} + {% set msg = 'No default implementation of dispatch_to_parent' %} + {{ exceptions.raise_compiler_error(msg) }} +{% endmacro %} + +{% macro postgres__dispatch_to_parent() %} + {{ return('') }} +{% endmacro %} diff --git a/test/integration/016_macro_tests/test_macros.py b/test/integration/016_macro_tests/test_macros.py index 4a554e313cc..3b1e94ace29 100644 --- a/test/integration/016_macro_tests/test_macros.py +++ b/test/integration/016_macro_tests/test_macros.py @@ -95,6 +95,20 @@ def test_postgres_invalid_macro(self): assert "In dispatch: No macro named 'dispatch_to_nowhere' found" in str(exc.value) +class TestDispatchMacroUseParent(DBTIntegrationTest): + @property + def schema(self): + return "test_macros_016" + + @property + def models(self): + return "dispatch-inheritance-models" + + @use_profile('redshift') + def test_redshift_inherited_macro(self): + self.run_dbt(['run']) + + class TestMacroOverrideBuiltin(DBTIntegrationTest): @property def schema(self): diff --git a/test/integration/029_docs_generate_tests/test_docs_generate.py b/test/integration/029_docs_generate_tests/test_docs_generate.py index 4ba66b2c811..ac34932a6e1 100644 --- a/test/integration/029_docs_generate_tests/test_docs_generate.py +++ b/test/integration/029_docs_generate_tests/test_docs_generate.py @@ -204,7 +204,7 @@ def _redshift_stats(self): "encoded": { "id": "encoded", "label": "Encoded", - "value": "Y", + "value": AnyStringWith('AUTO'), "description": "Indicates whether any column in the table has compression encoding defined.", "include": True }, diff --git a/test/unit/test_context.py b/test/unit/test_context.py index b9098d943cc..23473c989d4 100644 --- a/test/unit/test_context.py +++ b/test/unit/test_context.py @@ -242,6 +242,22 @@ def assert_has_keys( }, } +POSTGRES_PROFILE_DATA = { + 'target': 'test', + 'quoting': {}, + 'outputs': { + 'test': { + 'type': 'postgres', + 'host': 'localhost', + 'schema': 'analytics', + 'user': 'test', + 'pass': 'test', + 'dbname': 'test', + 'port': 1, + } + }, +} + PROJECT_DATA = { 'name': 'root', 'version': '0.1', @@ -368,6 +384,9 @@ def get_include_paths(): def config(): return config_from_parts_or_dicts(PROJECT_DATA, PROFILE_DATA) +@pytest.fixture +def config_postgres(): + return config_from_parts_or_dicts(PROJECT_DATA, POSTGRES_PROFILE_DATA) @pytest.fixture def manifest_fx(config): @@ -399,8 +418,8 @@ def redshift_adapter(config, get_adapter): @pytest.fixture -def postgres_adapter(config, get_adapter): - adapter = postgres.PostgresAdapter(config) +def postgres_adapter(config_postgres, get_adapter): + adapter = postgres.PostgresAdapter(config_postgres) inject_adapter(adapter, postgres.Plugin) get_adapter.return_value = adapter yield adapter @@ -521,13 +540,13 @@ def test_resolve_specific(config, manifest_extended, redshift_adapter, get_inclu 'dbt', 'root']).macro is rs_macro -def test_resolve_default(config, manifest_extended, postgres_adapter, get_include_paths): +def test_resolve_default(config_postgres, manifest_extended, postgres_adapter, get_include_paths): dbt_macro = manifest_extended.macros['macro.dbt.default__some_macro'] package_macro = manifest_extended.macros['macro.root.default__some_macro'] ctx = providers.generate_runtime_model( model=mock_model(), - config=config, + config=config_postgres, manifest=manifest_extended, )