-
Notifications
You must be signed in to change notification settings - Fork 161
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
[fix] Use rendered query comment for job labels #955
[fix] Use rendered query comment for job labels #955
Conversation
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @kodaho |
@kodaho thank you for contributing this was working on something locally. It was a fantastic pull in updating the test as well. |
* [fix] Use rendered query comment for job labels * Add test from dbt-labs/dbt-query#872 * Valid JSON in test * Add changie entry
This reverts commit d5b4114.
resolves #863
Problem
A regression appears in the last release of
dbt-bigquery
. A user can previously add labels to a BigQuery job thanks to a macro returning a mapping. In the version 1.6.0, this was no longer possible even if the project is set to activate this feature.Solution
The proposed solution is to use, instead of
self.profile.query_comment
(which is the not-rendered Jinja macro),self.query_header.comment.query_comment
(the JSON rendered query comments).I added some safeguards to make sure that
self.query_header.comment.query_comment
is not null but during the runtime, it should never be the case because of the following succession of functions and methods calls during a run command:run
command, command decorated with the decoratorrequires.manifest
.ManifestLoader.get_full_manifest
is called.ManifestLoader().save_macros_to_adapter
is called.ManifestLoader().macro_hook
is called, knowing thatmacro_hook = adapter.connections.set_query_header
.Connection().set_query_header
makes sure thatself.query_header
is not null.Checklist