Montreal-Analytics / dbt-snowflake-utils

Snowflake-specific utility macros for dbt projects.
Apache License 2.0
107 stars 37 forks source link

feat: add macro apply_cached_tag #36

Closed gilboare closed 1 year ago

gilboare commented 1 year ago

This PR suggests a way to address the gap between the time a model run is complete and the dbt run is complete. by using the previous tag during this gap. This way, the tag's is always available.

jamesweakley commented 1 year ago

@JFrackson / @MartinGuindon this LGTM, it's a new macro so shouldn't be able to impact any existing users

JGustavo0 commented 1 year ago

Hey! Thanks for your work here. Are you considering merging this into the future release?

gilboare commented 1 year ago

This seems like an interesting and worthwhile addition. One question about how to expose this, mostly so that I can understand the use case, would this macro and the apply_meta_as_tags macro both be run on your example project? Would a wrapper of some sort help simplify that for new adopters?

If the two macros are dependent on its more like a 1a and 1b, then including this in a more integrated type of macro could be valuable.

If I'm mistaking the use case, however, please let me know and I'd be happy to merge this as is to get a new release out.

Yes @JFrackson, the macros apply_meta_as_tags and apply_cached_tag go hand in hand. A user can use apply_meta_as_tags without using apply_cached_tag but not vice versa. Can you please elaborate about in a more integrated type of macro ?

JFrackson commented 1 year ago

@gilboare ah my mistake: I wasn't seeing that one runs on-run-end and the other is a post hook. If both were the same then a single wrapper macro or having your new macro as an optional param would be a nice integrated option. With their different places in the larger workflow, however, this seems like a sufficient implementation.

On an unrelated note, I also see that we now maintain some doc in the README and in other MD. I'll make an issue to fix that.

I'll approve and merge this into a new release today.

gilboare commented 1 year ago

@gilboare ah my mistake: I wasn't seeing that one runs on-run-end and the other is a post hook. If both were the same then a single wrapper macro or having your new macro as an optional param would be a nice integrated option. With their different places in the larger workflow, however, this seems like a sufficient implementation.

On an unrelated note, I also see that we now maintain some doc in the README and in other MD. I'll make an issue to fix that.

I'll approve and merge this into a new release today.

cool, thanks @JFrackson 💯

JGustavo0 commented 1 year ago

Thanks for the updates 💪