Skip to content

Add apply_meta_as_tags macro#21

Merged
MartinGuindon merged 6 commits intoMontreal-Analytics:masterfrom
omnata-labs:master
Nov 9, 2022
Merged

Add apply_meta_as_tags macro#21
MartinGuindon merged 6 commits intoMontreal-Analytics:masterfrom
omnata-labs:master

Conversation

@jamesweakley
Copy link
Contributor

This PR adds the apply_meta_as_tags as discussed here: dbt-labs/dbt-snowflake#104

Documentation is available in the readme, I opted for a yaml-driven approach.

Tested on a sample model with dbt 1.3.0.

@MartinGuindon
Copy link
Collaborator

Thank you for your contribution @jamesweakley. I will aim to merge this in the next few days!

@gmatsonANSYS
Copy link

@jamesweakley I tested your macro using the yaml driven approach and the macro only executes if a table and column tag are set on a model. Are you able to have tags created/set using only the column meta tags?

the log file shows " meta': {'database_tags': None}, 'transient': False}" and skips

@jamesweakley
Copy link
Contributor Author

@gmatsonANSYS good pickup, I hadn't tried that scenario. Will take a look today

…e tags.

Improved documentation of internal macros and added clarification to readme.
@jamesweakley
Copy link
Contributor Author

@gmatsonANSYS it should now work even if you've only defined column tags

@gmatsonANSYS
Copy link

gmatsonANSYS commented Oct 31, 2022

Thanks James, the updates worked as expected. The only tweaks I had to make were around the existing tag filters:

Converted the table and column name to upper - similar to the tag_name pipe - to return matches.
{%- set existing_tag_for_table = existing_tags|selectattr('0','equalto','TABLE')|selectattr('1','equalto',table_name|upper)|selectattr('2','equalto',column_name|upper)|selectattr('3','equalto',tag_name|upper)|list -%}

Updated to the fifth column index for the tag value comparison
{% if existing_tag_for_table|length > 0 and existing_tag_for_table[0][4]==desired_tag_value %}

@jamesweakley
Copy link
Contributor Author

@gmatsonANSYS thanks for the code snippets, I've pushed some more changes and tested various matching/non-matching scenarios and it seems consistent now

@jamesweakley
Copy link
Contributor Author

@gmatsonANSYS hopefully this all works in your test scenario now?

@gmatsonANSYS
Copy link

@jamesweakley runs smooth as silk. Thank you!

@MartinGuindon
Copy link
Collaborator

Awesome, if there are no further updates to this PR I'll be merging it shortly.

@MartinGuindon MartinGuindon self-requested a review November 9, 2022 18:53
Copy link
Collaborator

@MartinGuindon MartinGuindon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamesweakley The macro should be documented in macros.yml, and there's a typo in the README. Other than that, looks pretty good to me . Once fixed I'll merge and issue a new version.

@jamesweakley
Copy link
Contributor Author

@MartinGuindon Thanks, I've pushed those changes now

@MartinGuindon MartinGuindon merged commit 80fdddd into Montreal-Analytics:master Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants