From 2441335a5c81fc3025b155f61f417e0bd2dca297 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Tue, 26 Apr 2022 11:42:50 -0400 Subject: [PATCH] Use yaml renderer (with target context) for rendering selectors (#5136) * Use yaml renderer (with target context) for rendering selectors * Changie * Convert cli_vars tests * Add test for var in profiles * Add test for cli vars in packages * Add test for vars in selectors (cherry picked from commit 1f898c859ae644b4a994a61be8cf9730a09b3c29) --- .../unreleased/Fixes-20220422-135645.yaml | 7 + core/dbt/config/renderer.py | 17 +- core/dbt/config/selectors.py | 6 +- core/dbt/config/utils.py | 2 + core/dbt/tests/fixtures/project.py | 3 + core/dbt/tests/util.py | 8 + .../models_complex/complex_model.sql | 6 - .../models_complex/schema.yml | 19 -- .../models_override/schema.yml | 9 - .../models_override/test_vars.sql | 3 - .../models_simple/schema.yml | 9 - .../models_simple/simple_model.sql | 4 - .../test_cli_var_override.py | 57 ----- .../028_cli_vars_tests/test_cli_vars.py | 68 ------ .../cli_vars/test_cli_var_override.py | 68 ++++++ tests/functional/cli_vars/test_cli_vars.py | 208 ++++++++++++++++++ 16 files changed, 302 insertions(+), 192 deletions(-) create mode 100644 .changes/unreleased/Fixes-20220422-135645.yaml delete mode 100644 test/integration/028_cli_vars_tests/models_complex/complex_model.sql delete mode 100644 test/integration/028_cli_vars_tests/models_complex/schema.yml delete mode 100644 test/integration/028_cli_vars_tests/models_override/schema.yml delete mode 100644 test/integration/028_cli_vars_tests/models_override/test_vars.sql delete mode 100644 test/integration/028_cli_vars_tests/models_simple/schema.yml delete mode 100644 test/integration/028_cli_vars_tests/models_simple/simple_model.sql delete mode 100644 test/integration/028_cli_vars_tests/test_cli_var_override.py delete mode 100644 test/integration/028_cli_vars_tests/test_cli_vars.py create mode 100644 tests/functional/cli_vars/test_cli_var_override.py create mode 100644 tests/functional/cli_vars/test_cli_vars.py diff --git a/.changes/unreleased/Fixes-20220422-135645.yaml b/.changes/unreleased/Fixes-20220422-135645.yaml new file mode 100644 index 00000000000..916e7ad7825 --- /dev/null +++ b/.changes/unreleased/Fixes-20220422-135645.yaml @@ -0,0 +1,7 @@ +kind: Fixes +body: Use yaml renderer (with target context) for rendering selectors +time: 2022-04-22T13:56:45.147893-04:00 +custom: + Author: gshank + Issue: "5131" + PR: "5136" diff --git a/core/dbt/config/renderer.py b/core/dbt/config/renderer.py index 8fd06e5e990..e6c2d4523a0 100644 --- a/core/dbt/config/renderer.py +++ b/core/dbt/config/renderer.py @@ -114,12 +114,10 @@ def __init__( def name(self): "Project config" + # Uses SecretRenderer def get_package_renderer(self) -> BaseRenderer: return PackageRenderer(self.ctx_obj.cli_vars) - def get_selector_renderer(self) -> BaseRenderer: - return SelectorRenderer(self.ctx_obj.cli_vars) - def render_project( self, project: Dict[str, Any], @@ -136,8 +134,7 @@ def render_packages(self, packages: Dict[str, Any]): return package_renderer.render_data(packages) def render_selectors(self, selectors: Dict[str, Any]): - selector_renderer = self.get_selector_renderer() - return selector_renderer.render_data(selectors) + return self.render_data(selectors) def render_entry(self, value: Any, keypath: Keypath) -> Any: result = super().render_entry(value, keypath) @@ -165,18 +162,10 @@ def should_render_keypath(self, keypath: Keypath) -> bool: return True -class SelectorRenderer(BaseRenderer): - @property - def name(self): - return "Selector config" - - class SecretRenderer(BaseRenderer): - def __init__(self, cli_vars: Optional[Dict[str, Any]] = None) -> None: + def __init__(self, cli_vars: Dict[str, Any] = {}) -> None: # Generate contexts here because we want to save the context # object in order to retrieve the env_vars. - if cli_vars is None: - cli_vars = {} self.ctx_obj = SecretContext(cli_vars) context = self.ctx_obj.to_dict() super().__init__(context) diff --git a/core/dbt/config/selectors.py b/core/dbt/config/selectors.py index 916723fb8ec..996a9fb9ead 100644 --- a/core/dbt/config/selectors.py +++ b/core/dbt/config/selectors.py @@ -3,7 +3,7 @@ from dbt.clients.yaml_helper import yaml, Loader, Dumper, load_yaml_text # noqa: F401 from dbt.dataclass_schema import ValidationError -from .renderer import SelectorRenderer +from .renderer import BaseRenderer from dbt.clients.system import ( load_file_contents, @@ -57,7 +57,7 @@ def selectors_from_dict(cls, data: Dict[str, Any]) -> "SelectorConfig": def render_from_dict( cls, data: Dict[str, Any], - renderer: SelectorRenderer, + renderer: BaseRenderer, ) -> "SelectorConfig": try: rendered = renderer.render_data(data) @@ -72,7 +72,7 @@ def render_from_dict( def from_path( cls, path: Path, - renderer: SelectorRenderer, + renderer: BaseRenderer, ) -> "SelectorConfig": try: data = load_yaml_text(load_file_contents(str(path))) diff --git a/core/dbt/config/utils.py b/core/dbt/config/utils.py index 87909f9ff08..728e558ebbd 100644 --- a/core/dbt/config/utils.py +++ b/core/dbt/config/utils.py @@ -62,6 +62,8 @@ def get_project_config( user_config = read_user_config(flags.PROFILES_DIR) # Update flags flags.set_from_args(args, user_config) + if cli_vars is None: + cli_vars = {} profile = Profile.render_from_args(args, ProfileRenderer(cli_vars), profile_name) # Generate a project project = Project.from_project_root( diff --git a/core/dbt/tests/fixtures/project.py b/core/dbt/tests/fixtures/project.py index bb42a00ab3a..8c9426c898c 100644 --- a/core/dbt/tests/fixtures/project.py +++ b/core/dbt/tests/fixtures/project.py @@ -107,6 +107,9 @@ def test_data_dir(request): # This contains the profile target information, for simplicity in setting # up different profiles, particularly in the adapter repos. +# Note: because we load the profile to create the adapter, this +# fixture can't be used to test vars and env_vars or errors. The +# profile must be written out after the test starts. @pytest.fixture(scope="class") def dbt_profile_target(): return { diff --git a/core/dbt/tests/util.py b/core/dbt/tests/util.py index e0f5839d637..2e37a900024 100644 --- a/core/dbt/tests/util.py +++ b/core/dbt/tests/util.py @@ -23,6 +23,7 @@ # read_file # get_artifact # update_config_file +# write_config_file # get_unique_ids_in_results # check_result_nodes_by_name # check_result_nodes_by_unique_id @@ -143,6 +144,13 @@ def update_config_file(updates, *paths): write_file(new_yaml, *paths) +# Write new config file +def write_config_file(data, *paths): + if type(data) is dict: + data = yaml.safe_dump(data) + write_file(data, *paths) + + # Get the unique_ids in dbt command results def get_unique_ids_in_results(results): unique_ids = [] diff --git a/test/integration/028_cli_vars_tests/models_complex/complex_model.sql b/test/integration/028_cli_vars_tests/models_complex/complex_model.sql deleted file mode 100644 index 1022c648100..00000000000 --- a/test/integration/028_cli_vars_tests/models_complex/complex_model.sql +++ /dev/null @@ -1,6 +0,0 @@ - -select - '{{ var("variable_1") }}'::varchar as var_1, - '{{ var("variable_2")[0] }}'::varchar as var_2, - '{{ var("variable_3")["value"] }}'::varchar as var_3 - diff --git a/test/integration/028_cli_vars_tests/models_complex/schema.yml b/test/integration/028_cli_vars_tests/models_complex/schema.yml deleted file mode 100644 index 7000d58c951..00000000000 --- a/test/integration/028_cli_vars_tests/models_complex/schema.yml +++ /dev/null @@ -1,19 +0,0 @@ -version: 2 -models: -- name: complex_model - columns: - - name: var_1 - tests: - - accepted_values: - values: - - abc - - name: var_2 - tests: - - accepted_values: - values: - - def - - name: var_3 - tests: - - accepted_values: - values: - - jkl diff --git a/test/integration/028_cli_vars_tests/models_override/schema.yml b/test/integration/028_cli_vars_tests/models_override/schema.yml deleted file mode 100644 index 44209f278b8..00000000000 --- a/test/integration/028_cli_vars_tests/models_override/schema.yml +++ /dev/null @@ -1,9 +0,0 @@ -version: 2 -models: -- name: test_vars - columns: - - name: field - tests: - - accepted_values: - values: - - override diff --git a/test/integration/028_cli_vars_tests/models_override/test_vars.sql b/test/integration/028_cli_vars_tests/models_override/test_vars.sql deleted file mode 100644 index a1c7827ec74..00000000000 --- a/test/integration/028_cli_vars_tests/models_override/test_vars.sql +++ /dev/null @@ -1,3 +0,0 @@ - - -select '{{ var("required") }}'::varchar as field diff --git a/test/integration/028_cli_vars_tests/models_simple/schema.yml b/test/integration/028_cli_vars_tests/models_simple/schema.yml deleted file mode 100644 index cda0fea6381..00000000000 --- a/test/integration/028_cli_vars_tests/models_simple/schema.yml +++ /dev/null @@ -1,9 +0,0 @@ -version: 2 -models: -- name: simple_model - columns: - - name: simple - tests: - - accepted_values: - values: - - abc diff --git a/test/integration/028_cli_vars_tests/models_simple/simple_model.sql b/test/integration/028_cli_vars_tests/models_simple/simple_model.sql deleted file mode 100644 index 084bd57012e..00000000000 --- a/test/integration/028_cli_vars_tests/models_simple/simple_model.sql +++ /dev/null @@ -1,4 +0,0 @@ - -select - '{{ var("simple") }}'::varchar as simple - diff --git a/test/integration/028_cli_vars_tests/test_cli_var_override.py b/test/integration/028_cli_vars_tests/test_cli_var_override.py deleted file mode 100644 index af3fe41d891..00000000000 --- a/test/integration/028_cli_vars_tests/test_cli_var_override.py +++ /dev/null @@ -1,57 +0,0 @@ -from test.integration.base import DBTIntegrationTest, use_profile - - -class TestCLIVarOverride(DBTIntegrationTest): - @property - def schema(self): - return "cli_vars_028" - - @property - def models(self): - return "models_override" - - @property - def project_config(self): - return { - 'config-version': 2, - 'vars': { - 'required': 'present', - }, - } - - @use_profile('postgres') - def test__postgres_overriden_vars_global(self): - self.use_default_project() - self.use_profile('postgres') - - # This should be "override" - self.run_dbt(["run", "--vars", "{required: override}"]) - self.run_dbt(["test"]) - - -class TestCLIVarOverridePorject(DBTIntegrationTest): - @property - def schema(self): - return "cli_vars_028" - - @property - def models(self): - return "models_override" - - @property - def project_config(self): - return { - 'config-version': 2, - 'vars': { - 'test': { - 'required': 'present', - }, - }, - } - - @use_profile('postgres') - def test__postgres_overriden_vars_project_level(self): - - # This should be "override" - self.run_dbt(["run", "--vars", "{required: override}"]) - self.run_dbt(["test"]) diff --git a/test/integration/028_cli_vars_tests/test_cli_vars.py b/test/integration/028_cli_vars_tests/test_cli_vars.py deleted file mode 100644 index d0873b6e107..00000000000 --- a/test/integration/028_cli_vars_tests/test_cli_vars.py +++ /dev/null @@ -1,68 +0,0 @@ -from test.integration.base import DBTIntegrationTest, use_profile -import yaml -import json - - -class TestCLIVars(DBTIntegrationTest): - @property - def schema(self): - return "cli_vars_028" - - @property - def models(self): - return "models_complex" - - @use_profile('postgres') - def test__postgres_cli_vars_longform(self): - self.use_profile('postgres') - self.use_default_project() - - cli_vars = { - "variable_1": "abc", - "variable_2": ["def", "ghi"], - "variable_3": { - "value": "jkl" - } - } - results = self.run_dbt(["run", "--vars", yaml.dump(cli_vars)]) - self.assertEqual(len(results), 1) - results = self.run_dbt(["test", "--vars", yaml.dump(cli_vars)]) - self.assertEqual(len(results), 3) - - -class TestCLIVarsSimple(DBTIntegrationTest): - @property - def schema(self): - return "cli_vars_028" - - @property - def models(self): - return "models_simple" - - @use_profile('postgres') - def test__postgres_cli_vars_shorthand(self): - self.use_profile('postgres') - self.use_default_project() - - results = self.run_dbt(["run", "--vars", "simple: abc"]) - self.assertEqual(len(results), 1) - results = self.run_dbt(["test", "--vars", "simple: abc"]) - self.assertEqual(len(results), 1) - - @use_profile('postgres') - def test__postgres_cli_vars_longer(self): - self.use_profile('postgres') - self.use_default_project() - - results = self.run_dbt(["run", "--vars", "{simple: abc, unused: def}"]) - self.assertEqual(len(results), 1) - results = self.run_dbt(["test", "--vars", "{simple: abc, unused: def}"]) - self.assertEqual(len(results), 1) - run_results = _read_json('./target/run_results.json') - self.assertEqual(run_results['args']['vars'], "{simple: abc, unused: def}") - - -def _read_json(path): - # read json generated by dbt. - with open(path) as fp: - return json.load(fp) diff --git a/tests/functional/cli_vars/test_cli_var_override.py b/tests/functional/cli_vars/test_cli_var_override.py new file mode 100644 index 00000000000..22b26697bbf --- /dev/null +++ b/tests/functional/cli_vars/test_cli_var_override.py @@ -0,0 +1,68 @@ +import pytest + +from dbt.tests.util import run_dbt + + +models_override__schema_yml = """ +version: 2 +models: +- name: test_vars + columns: + - name: field + tests: + - accepted_values: + values: + - override +""" + +models_override__test_vars_sql = """ +select '{{ var("required") }}'::varchar as field +""" + + +# Tests that cli vars override vars set in the project config +class TestCLIVarOverride: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_override__schema_yml, + "test_vars.sql": models_override__test_vars_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "vars": { + "required": "present", + }, + } + + def test__override_vars_global(self, project): + run_dbt(["run", "--vars", "{required: override}"]) + run_dbt(["test"]) + + +# This one switches to setting a var in 'test' +class TestCLIVarOverridePorject: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_override__schema_yml, + "test_vars.sql": models_override__test_vars_sql, + } + + @pytest.fixture(scope="class") + def project_config_update(self): + return { + "vars": { + "test": { + "required": "present", + }, + }, + } + + def test__override_vars_project_level(self, project): + + # This should be "override" + run_dbt(["run", "--vars", "{required: override}"]) + run_dbt(["test"]) diff --git a/tests/functional/cli_vars/test_cli_vars.py b/tests/functional/cli_vars/test_cli_vars.py new file mode 100644 index 00000000000..c9b4f010c7a --- /dev/null +++ b/tests/functional/cli_vars/test_cli_vars.py @@ -0,0 +1,208 @@ +import pytest +import yaml + +from dbt.tests.util import run_dbt, get_artifact, write_config_file +from dbt.exceptions import RuntimeException, CompilationException + + +models_complex__schema_yml = """ +version: 2 +models: +- name: complex_model + columns: + - name: var_1 + tests: + - accepted_values: + values: + - abc + - name: var_2 + tests: + - accepted_values: + values: + - def + - name: var_3 + tests: + - accepted_values: + values: + - jkl +""" + +models_complex__complex_model_sql = """ +select + '{{ var("variable_1") }}'::varchar as var_1, + '{{ var("variable_2")[0] }}'::varchar as var_2, + '{{ var("variable_3")["value"] }}'::varchar as var_3 +""" + +models_simple__schema_yml = """ +version: 2 +models: +- name: simple_model + columns: + - name: simple + tests: + - accepted_values: + values: + - abc +""" + +models_simple__simple_model_sql = """ +select + '{{ var("simple") }}'::varchar as simple +""" + +really_simple_model_sql = """ +select 'abc' as simple +""" + + +class TestCLIVars: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_complex__schema_yml, + "complex_model.sql": models_complex__complex_model_sql, + } + + def test__cli_vars_longform(self, project): + cli_vars = { + "variable_1": "abc", + "variable_2": ["def", "ghi"], + "variable_3": {"value": "jkl"}, + } + results = run_dbt(["run", "--vars", yaml.dump(cli_vars)]) + assert len(results) == 1 + results = run_dbt(["test", "--vars", yaml.dump(cli_vars)]) + assert len(results) == 3 + + +class TestCLIVarsSimple: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_simple__schema_yml, + "simple_model.sql": models_simple__simple_model_sql, + } + + def test__cli_vars_shorthand(self, project): + results = run_dbt(["run", "--vars", "simple: abc"]) + assert len(results) == 1 + results = run_dbt(["test", "--vars", "simple: abc"]) + assert len(results) == 1 + + def test__cli_vars_longer(self, project): + results = run_dbt(["run", "--vars", "{simple: abc, unused: def}"]) + assert len(results) == 1 + results = run_dbt(["test", "--vars", "{simple: abc, unused: def}"]) + assert len(results) == 1 + run_results = get_artifact(project.project_root, "target", "run_results.json") + assert run_results["args"]["vars"] == "{simple: abc, unused: def}" + + +class TestCLIVarsProfile: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_simple__schema_yml, + "simple_model.sql": really_simple_model_sql, + } + + def test_cli_vars_in_profile(self, project, dbt_profile_data): + profile = dbt_profile_data + profile["test"]["outputs"]["default"]["host"] = "{{ var('db_host') }}" + write_config_file(profile, project.profiles_dir, "profiles.yml") + with pytest.raises(RuntimeException): + results = run_dbt(["run"]) + results = run_dbt(["run", "--vars", "db_host: localhost"]) + assert len(results) == 1 + + +class TestCLIVarsPackages: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_simple__schema_yml, + "simple_model.sql": really_simple_model_sql, + } + + @pytest.fixture(scope="class") + def packages_config(self): + return { + "packages": [ + { + "git": "https://github.com/dbt-labs/dbt-integration-project", + "revision": "1.1", + } + ] + } + + def test_cli_vars_in_packages(self, project, packages_config): + # Run working deps and run commands + run_dbt(["deps"]) + results = run_dbt(["run"]) + assert len(results) == 1 + + # Change packages.yml to contain a var + packages = packages_config + packages["packages"][0]["revision"] = "{{ var('dip_version') }}" + write_config_file(packages, project.project_root, "packages.yml") + + # Without vars args deps fails + with pytest.raises(RuntimeException): + run_dbt(["deps"]) + + # With vars arg deps succeeds + results = run_dbt(["deps", "--vars", "dip_version: 1.1"]) + assert results is None + + +initial_selectors_yml = """ +selectors: + - name: dev_defer_snapshots + default: "{{ target.name == 'dev' | as_bool }}" + definition: + method: fqn + value: '*' + exclude: + - method: config.materialized + value: snapshot +""" + +var_selectors_yml = """ +selectors: + - name: dev_defer_snapshots + default: "{{ var('snapshot_target') == 'dev' | as_bool }}" + definition: + method: fqn + value: '*' + exclude: + - method: config.materialized + value: snapshot +""" + + +class TestCLIVarsSelectors: + @pytest.fixture(scope="class") + def models(self): + return { + "schema.yml": models_simple__schema_yml, + "simple_model.sql": really_simple_model_sql, + } + + @pytest.fixture(scope="class") + def selectors(self): + return initial_selectors_yml + + def test_vars_in_selectors(self, project): + # initially runs ok + results = run_dbt(["run"]) + assert len(results) == 1 + + # Update the selectors.yml file to have a var + write_config_file(var_selectors_yml, project.project_root, "selectors.yml") + with pytest.raises(CompilationException): + run_dbt(["run"]) + + # Var in cli_vars works + results = run_dbt(["run", "--vars", "snapshot_target: dev"]) + assert len(results) == 1