In schema tests, pass rendered items (#2149) #2220
Conversation
beckjake
force-pushed
the
feature/schema-tests-pass-rendered
branch
from
ad899d1 to
6308d08
Compare
|
The azure tests are passing, I don't know why they're showing as failed. https://dev.azure.com/fishtown-analytics/dbt/_build/results?buildId=1658&view=results |
Render test arguments in the native env and pass them along to the context added/fixed tests Update changelog
beckjake
force-pushed
the
feature/schema-tests-pass-rendered
branch
from
6308d08 to
96cfc49
Compare
core/dbt/adapters/sql/impl.py
Outdated
| @@ -227,6 +227,12 @@ def list_relations_without_caching( | |||
| def quote(self, identifier): | |||
| return '"{}"'.format(identifier) | |||
|
|
|||
| @classmethod | |||
| def unquote(cls, identifier: str) -> str: | |||
There was a problem hiding this comment.
Feels to me like plugins should specify a quote character so that we don't need to re-implement this method on BigQuery (or others). Agree/disagree?
There was a problem hiding this comment.
We actually don't need this method at all, I think - I'll remove it.
| if isinstance(value, str): | ||
| if re.match(looks_like_func, value) is not None: | ||
| # curly braces to make rendering happy | ||
| value = f'{{{{ {value} }}}}' |
There was a problem hiding this comment.
Do we have the ability to reach into the context and call these functions directly? I think the approach shown here is a-ok for now, but I'd like to remove this jinja hackery in the future if we can!
There was a problem hiding this comment.
we could, but it would be quite tricky to handle things like ref(var('foo')) because you have to recursively descend into the arguments
There was a problem hiding this comment.
oh, interesting - that's fair. Thanks!
core/dbt/clients/jinja.py
Outdated
| ) | ||
| # this is an ugly way of checking if the value was a quoted str | ||
| # (we don't want to unquote column names!) | ||
| if not (len(value) >= 2 and new_value == value[1:-1]): |
There was a problem hiding this comment.
I don't think i understand what this check does -- how can get_rendered remove quotes around column names?
There was a problem hiding this comment.
If you take the string "my_str" and render it with jinja (edit: in the native context only!), jinja will strip the quotes as part of rendering.
| @@ -160,33 +181,6 @@ def compile_node(self, node, manifest, extra_context=None): | |||
|
|
|||
| injected_node, _ = prepend_ctes(compiled_node, manifest) | |||
|
|
|||
| should_wrap = {NodeType.Test, NodeType.Operation} | |||
Co-Authored-By: Drew Banin <drew@fishtownanalytics.com>
beckjake
force-pushed
the
feature/schema-tests-pass-rendered
branch
from
e9a053c to
701b3b4
Compare
beckjake
force-pushed
the
feature/schema-tests-pass-rendered
branch
from
701b3b4 to
1268c11
Compare
|
I think #38bcc2b looks like it works really well here! |
|
Yeah. playing around locally, it seems to behave like I'd expect. |
|
This PR also resolves #2227 |
resolves #2149
Description
In schema tests, render parameters and pass them in as rendered items.
This PR formally splits data tests and schema tests - data tests don't get a column name or test metadata since that's nonsensical for them.
This PR includes some backwards compatibility changes, so there should be no regressions. Initially I was going to have the "legacy form" arguments (like
ref("my_model")instead of{{ ref("my_model") }}) emit a deprecation warning, but it's actually really annoying and overly verbose, so I think I'd like to keep the shorthand form.Edit: I added jinja2 2.11.x as a requirement for this PR, as it fixes some issues with quoting in the native renderer.
Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.