-
Notifications
You must be signed in to change notification settings - Fork 179
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
Feature/support query tags in tests #210
Conversation
@@ -0,0 +1,49 @@ | |||
{%- materialization test, adapter='snowflake' -%} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd hope that all of this could just be:
{%- materialization test, adapter='snowflake' -%}
{% set original_query_tag = set_query_tag() %}
{% set relations = materialization_test_default() %}
{% do unset_query_tag(original_query_tag) %}
{{ return(relations) }}
{% endmaterialization %}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right on! had that initially, then went to the full copy from dbt-core. Will shift back, test etc. Thanks @jtcohen6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jtcohen6 with this set up, I can configure query tags to run on tests from the dbt_project.yml
:
tests:
dev_project_snowflake:
example:
+query_tag: mw_dev_test
BUT, I can't set them from a schema.yml
:
- name: my_second_dbt_model
description: "A starter dbt model"
columns:
- name: id
description: "The primary key for this table"
tests:
- unique:
config:
query_tag: mw_dev_test #nothing happens on snowflake with this setup
- not_null
Should I be able to make this setting in both places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matt-winkler Good catch. Unfortunately, this will have to come as a separate unit of work: dbt-labs/dbt-core#5532
I went ahead and added a test for this. As part of that, I'm seeing that the model in
This seems to be unrelated to any of the query tagging work, but perhaps there is something going on that I'm missing. Assuming it's unrelated, should we: 1) add the |
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the dbt-snowflake contributing guide. |
Hey @dbt-labs/core-adapters team, this is ready for review. Do you all prefer to tie this to dbt-labs/dbt-core#5532 or let it stand on it's own? |
@cla-bot[bot] check |
The cla-bot has been summoned, and re-checked this pull request! |
@matt-winkler pinged @jtcohen6 about dbt-labs/dbt-core#5532 and it can be merged later on to give this pr more functionality then. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matt-winkler Thanks for working on this, was able to test it locally via integration test and in snowflake project 👍.
resolves #211
Description
Add query tagging capabilities to the
test
materialization in snowflake. Believe this requires creating a new materialization in this repo vs. a change todbt-core
.Checklist
CHANGELOG.md
and added information about my change to the "dbt-snowflake next" section.