Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix source test arg rendering (#2114) #2150

Merged
merged 2 commits into from
Feb 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## dbt next (Release TBD)

### Breaking changes
- Arguments to source tests are not parsed in the config-rendering context, and are passed as their literal unparsed values to macros ([#2150](https:/fishtown-analytics/dbt/pull/2150))

### Features
- Add a "docs" field to models, with a "show" subfield ([#1671](https:/fishtown-analytics/dbt/issues/1671), [#2107](https:/fishtown-analytics/dbt/pull/2107))
- Add a dbt-{dbt_version} user agent field to the bigquery connector ([#2121](https:/fishtown-analytics/dbt/issues/2121), [#2146](https:/fishtown-analytics/dbt/pull/2146))
Expand All @@ -8,6 +11,7 @@
- Fix issue where dbt did not give an error in the presence of duplicate doc names ([#2054](https:/fishtown-analytics/dbt/issues/2054), [#2080](https:/fishtown-analytics/dbt/pull/2080))
- Include vars provided to the cli method when running the actual method ([#2092](https:/fishtown-analytics/dbt/issues/2092), [#2104](https:/fishtown-analytics/dbt/pull/2104))
- Improved error messages with malformed packages.yml ([#2017](https:/fishtown-analytics/dbt/issues/2017), [#2078](https:/fishtown-analytics/dbt/pull/2078))
- Fix an issue where dbt rendered source test args, fix issue where dbt ran an extra compile pass over the wrapped SQL. ([#2114](https:/fishtown-analytics/dbt/issues/2114), [#2150](https:/fishtown-analytics/dbt/pull/2150))

Contributors:
- [@bubbomb](https:/bubbomb) ([#2080](https:/fishtown-analytics/dbt/pull/2080))
Expand Down
19 changes: 0 additions & 19 deletions core/dbt/compilation.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ def _is_writable(node):
def compile_node(adapter, config, node, manifest, extra_context, write=True):
compiler = Compiler(config)
node = compiler.compile_node(node, manifest, extra_context)
node = _inject_runtime_config(adapter, node, extra_context)

if write and _is_writable(node):
logger.debug('Writing injected SQL for node "{}"'.format(
Expand All @@ -261,21 +260,3 @@ def compile_node(adapter, config, node, manifest, extra_context, write=True):
)

return node


def _inject_runtime_config(adapter, node, extra_context):
wrapped_sql = node.wrapped_sql
context = _node_context(adapter, node)
context.update(extra_context)
sql = dbt.clients.jinja.get_rendered(wrapped_sql, context)
node.wrapped_sql = sql
return node


def _node_context(adapter, node):
if dbt.tracking.active_user is not None:
return {
"run_started_at": dbt.tracking.active_user.run_started_at,
"invocation_id": dbt.tracking.active_user.invocation_id,
}
return {} # this never happens, but make mypy happy
14 changes: 14 additions & 0 deletions core/dbt/config/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,25 @@ def _render_profile_data(self, value, keypath):
pass
return result

@staticmethod
def _is_schema_test(keypath) -> bool:
# we got passed an UnparsedSourceDefinition
if len(keypath) > 2 and keypath[0] == 'tables':
if keypath[2] == 'tests':
return True
elif keypath[2] == 'columns':
if len(keypath) > 4 and keypath[4] == 'tests':
return True
return False

def _render_schema_source_data(self, value, keypath):
# things to not render:
# - descriptions
# - test arguments
if len(keypath) > 0 and keypath[-1] == 'description':
return value
elif self._is_schema_test(keypath):
return value

return self.render_value(value)

Expand Down
36 changes: 18 additions & 18 deletions test/integration/029_docs_generate_tests/test_docs_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ def expected_seeded_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'seed.test.seed': {
'build_path': None,
Expand Down Expand Up @@ -1044,7 +1044,7 @@ def expected_seeded_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': '',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'test.test.not_null_model_id': {
'alias': 'not_null_model_id',
Expand Down Expand Up @@ -1373,7 +1373,7 @@ def expected_postgres_references_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'model.test.ephemeral_summary': {
'alias': 'ephemeral_summary',
Expand Down Expand Up @@ -1437,7 +1437,7 @@ def expected_postgres_references_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [ANY],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'model.test.view_summary': {
'alias': 'view_summary',
Expand Down Expand Up @@ -1500,7 +1500,7 @@ def expected_postgres_references_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'seed.test.seed': {
'alias': 'seed',
Expand Down Expand Up @@ -1579,7 +1579,7 @@ def expected_postgres_references_manifest(self, model_database=None):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': '',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'source.test.my_source.my_table': {
'columns': {
Expand Down Expand Up @@ -1952,7 +1952,7 @@ def expected_bigquery_complex_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'model.test.multi_clustered': {
'alias': 'multi_clustered',
Expand Down Expand Up @@ -2031,7 +2031,7 @@ def expected_bigquery_complex_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'model.test.nested_view': {
'alias': 'nested_view',
Expand Down Expand Up @@ -2111,7 +2111,7 @@ def expected_bigquery_complex_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'model.test.nested_table': {
'alias': 'nested_table',
Expand Down Expand Up @@ -2155,7 +2155,7 @@ def expected_bigquery_complex_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'seed.test.seed': {
'build_path': None,
Expand Down Expand Up @@ -2237,7 +2237,7 @@ def expected_bigquery_complex_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': '',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
},
'child_map': {
Expand Down Expand Up @@ -2462,7 +2462,7 @@ def expected_redshift_incremental_view_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'seed.test.seed': {
'build_path': None,
Expand Down Expand Up @@ -2544,7 +2544,7 @@ def expected_redshift_incremental_view_manifest(self):
'extra_ctes_injected': True,
'extra_ctes': [],
'injected_sql': ANY,
'wrapped_sql': 'None',
'wrapped_sql': None,
},
},
'parent_map': {
Expand Down Expand Up @@ -2784,7 +2784,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False,
'database': model_database,
'tags': [],
'unique_id': 'model.test.model',
'wrapped_sql': 'None'
'wrapped_sql': None
},
'thread_id': ANY,
'timing': [ANY, ANY],
Expand Down Expand Up @@ -2873,7 +2873,7 @@ def expected_run_results(self, quote_schema=True, quote_model=False,
'database': self.default_database,
'tags': [],
'unique_id': 'seed.test.seed',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'thread_id': ANY,
'timing': [ANY, ANY],
Expand Down Expand Up @@ -3163,7 +3163,7 @@ def expected_postgres_references_run_results(self):
'database': self.default_database,
'tags': [],
'unique_id': 'model.test.ephemeral_summary',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'thread_id': ANY,
'timing': [ANY, ANY],
Expand Down Expand Up @@ -3242,7 +3242,7 @@ def expected_postgres_references_run_results(self):
'database': self.default_database,
'tags': [],
'unique_id': 'model.test.view_summary',
'wrapped_sql': 'None',
'wrapped_sql': None,
},
'thread_id': ANY,
'timing': [ANY, ANY],
Expand Down Expand Up @@ -3331,7 +3331,7 @@ def expected_postgres_references_run_results(self):
'database': self.default_database,
'tags': [],
'unique_id': 'seed.test.seed',
'wrapped_sql': 'None'
'wrapped_sql': None
},
'thread_id': ANY,
'timing': [ANY, ANY],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ sources:
identifier: source
tests:
- relationships:
# this is invalid
# this is invalid (list of 3 1-key dicts instead of a single 3-key dict)
- column_name: favorite_color
- to: ref('descendant_model')
- field: favorite_color
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select * from {{ source('test_source', 'test_table') }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
version: 2
sources:
- name: test_source
schema: "{{ var('test_run_schema') }}"
tables:
- name: test_table
identifier: source
columns:
- name: favorite_color
tests:
- relationships:
to: ref('model')
# this will get rendered as its literal
field: "{{ 'favorite' ~ 'color' }}"
13 changes: 13 additions & 0 deletions test/integration/042_sources_test/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,19 @@ def test_postgres_malformed_schema_strict_will_break_run(self):
self.run_dbt_with_vars(['seed'], strict=True)


class TestRenderingInSourceTests(BaseSourcesTest):
@property
def models(self):
return "malformed_schema_tests"

@use_profile('postgres')
def test_postgres_render_in_source_tests(self):
self.run_dbt_with_vars(['seed'])
self.run_dbt_with_vars(['run'])
# syntax error at or near "{", because the test isn't rendered
self.run_dbt_with_vars(['test'], expect_pass=False)


class TestUnquotedSources(SuccessfulSourcesTest):
@property
def project_config(self):
Expand Down
2 changes: 0 additions & 2 deletions test/unit/test_contracts_graph_compiled.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ def test_basic_compiled(self):
'extra_ctes': [{'id': 'whatever', 'sql': 'select * from other'}],
'extra_ctes_injected': True,
'injected_sql': 'with whatever as (select * from other) select * from whatever',
'wrapped_sql': 'None',
}
node = self.ContractType(
package_name='test',
Expand All @@ -166,7 +165,6 @@ def test_basic_compiled(self):
extra_ctes=[InjectedCTE('whatever', 'select * from other')],
extra_ctes_injected=True,
injected_sql='with whatever as (select * from other) select * from whatever',
wrapped_sql='None',
)
self.assert_symmetric(node, node_dict)
self.assertFalse(node.empty)
Expand Down