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

Feature/support query tags in tests #210

Merged
merged 11 commits into from
Aug 22, 2022

Conversation

matt-winkler
Copy link
Contributor

@matt-winkler matt-winkler commented Jul 22, 2022

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 to dbt-core.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-snowflake next" section.

@cla-bot cla-bot bot added the cla:yes label Jul 22, 2022
@matt-winkler matt-winkler requested review from jtcohen6, a team and VersusFacit July 22, 2022 17:49
@@ -0,0 +1,49 @@
{%- materialization test, adapter='snowflake' -%}
Copy link
Contributor

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 %}

Copy link
Contributor Author

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

Copy link
Contributor Author

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?

Copy link
Contributor

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

@matt-winkler
Copy link
Contributor Author

I went ahead and added a test for this. As part of that, I'm seeing that the model in tests/integration/query_tag_tests/incremental_model_query_tag.sql is failing with the following error:

19:24:13  Completed with 1 error and 0 warnings:
19:24:13  
19:24:13  Compilation Error in model incremental_model_query_tag (models/incremental_model_query_tag.sql)
19:24:13    Invalid incremental strategy provided: None
19:24:13        Expected one of: 'merge', 'delete+insert'
19:24:13    
19:24:13    > in macro dbt_snowflake_validate_get_incremental_strategy (macros/materializations/incremental.sql)
19:24:13    > called by macro materialization_incremental_snowflake (macros/materializations/incremental.sql)
19:24:13    > called by model incremental_model_query_tag (models/incremental_model_query_tag.sql)

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 incremental_strategy config to the model as part of this PR? Or 2) create a new branch / PR to work from?

@github-actions
Copy link
Contributor

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.

@matt-winkler
Copy link
Contributor Author

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?

@McKnight-42 McKnight-42 requested a review from jtcohen6 August 16, 2022 16:51
@McKnight-42 McKnight-42 marked this pull request as ready for review August 18, 2022 19:40
@McKnight-42
Copy link
Contributor

@cla-bot[bot] check

@cla-bot
Copy link

cla-bot bot commented Aug 19, 2022

The cla-bot has been summoned, and re-checked this pull request!

@jtcohen6 jtcohen6 added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label Aug 19, 2022
@McKnight-42
Copy link
Contributor

@matt-winkler pinged @jtcohen6 about dbt-labs/dbt-core#5532 and it can be merged later on to give this pr more functionality then.

Copy link
Contributor

@McKnight-42 McKnight-42 left a 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 👍.

@dbeatty10 dbeatty10 self-requested a review August 19, 2022 22:27
@McKnight-42 McKnight-42 merged commit 25a7d22 into main Aug 22, 2022
@McKnight-42 McKnight-42 deleted the feature/support-query-tags-in-tests branch August 22, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-893] [CT-796] [Feature] enable query tagging for dbt tests on snowflake
3 participants