-
Notifications
You must be signed in to change notification settings - Fork 59
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
ADAP-381: adds query tagging support and materializations to dbt-redshift #379
Conversation
7044487
to
fd26935
Compare
Heya, thanks for kicking up a PR! A big one at that! I see you're adding several local implementations of macros in order to get this done. That's potentially all right, but that also means the reviews going to take longer because I need to evaluate some design considerations. The actual feature itself seems simple enough and as you pointed out, has precedence in snowflake. |
Hi @trouze @VersusFacit, I reviewed this for about 15 minutes. Agreed it's a large PR primarily because of the requirement of bringing in Check out this thread going on dbt-labs/dbt-bigquery#685 -- @dbeatty10 do you think this PR from @trouze is sufficiently similar? |
@boxysean thank you for having a look at this! I think it makes sense to have dbt-core utilize the query tagging concept in the base materializations there, and then have each adapter implement macros specific to how their database implements the query tagging functionality. Despite @dbeatty10 mention that |
Hi @trouze from my position, I think it's a good idea to raise your suggestion in dbt-labs/dbt-core and reference out to this issue and the BigQuery one. At least you will get some attention on your PR here, and a decision can be made faster, which I believe is your goal. |
This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days. |
This feature has came back up as something we are really interested in. Looks like it hasn't got much activity lately so wanted to check and see if there was another place this got moved or if there was alternative. |
@ddors3y I believe we were all in agreement with moving this to |
This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days. |
Although we are closing this PR as stale, it can still be reopened to continue development. Just add a comment to notify the maintainers. |
resolves #376
Description
This PR will add the ability to tag queries ran against Redshift with a string field added by a user in yaml or sql configs, much like the dbt-snowflake adapter supports.
Tradeoffs:
query_tag
consistent with that of dbt-snowflake, despite AWS naming this featurequery_group
. We can certainly go back on this convention, but obviously would recommend keeping the macros consistent with that of dbt-snowflake to support multiple dispatch.Pertinent links to this functionality and dbt-snowflake's implementation of this feature can be found in the issue discussion.
Checklist
changie new
to create a changelog entry