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

Support BigQuery 'labels' and 'hours_to_expiration' configs for seeds #133

Merged
merged 23 commits into from
Dec 21, 2022

Conversation

darrylng
Copy link
Contributor

@darrylng darrylng commented Mar 10, 2022

resolves #125

Description

This PR enables +labels: and +hours_to_expiration: configs for seeds configuration. These labels are already implemented and used by models configuration to set table/view labels and expiration time, but was not done for seeded tables, reasons explained in issue #125.

The initial change was to only implement labels config but on further dev, I noticed the addition of ALTER TABLE .. SET OPTION() supports other options other than labels, so the pytest was added to include hours_to_expiration config test as well.

These are the only two non-configs properties that are currently implemented in dbt-bigquery that will start to take effect with the change in seed.sql.

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-bigquery next" section.

@cla-bot cla-bot bot added the cla:yes label Mar 10, 2022
@darrylng darrylng marked this pull request as ready for review March 10, 2022 00:09
@McKnight-42
Copy link
Contributor

Hi @darrylng sorry for the wait on review if you could update to current main and push up to clear up current conflicts that would be great as it keeps your local in step with your pushed up pr.

@McKnight-42
Copy link
Contributor

@darrylng it seems that we have added the pre-commit since you last updated, if you could update your local and run the pre-commit checks to make sure its passing should clear up the code-quality test. sorry for the delay in response.

@McKnight-42
Copy link
Contributor

@darrylng really liking this tested locally and tests are passing, not sure why more aren't triggering here. if you could please update to latest in main and re push up (want to just keep your remote and local up to date with each other). but this is looking very good thus far great work!

@darrylng
Copy link
Contributor Author

I have merged main branch into my forked branch but I believe the three tests that aren't running is because I'm a first time contributor and the workflow requires a maintainer to approve running it.
Reference:
https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

@jtcohen6 jtcohen6 added the ready_for_review Externally contributed PR has functional approval, ready for code review from Core engineering label Jun 15, 2022
@darrylng
Copy link
Contributor Author

Any chance we can get this reviewed? I'd love to see this feature soon as the current patchwork involves running macro to add labels after on-run-end which runs for all dbt commands. Thanks for your support all!

@matthieu-lombard
Copy link

matthieu-lombard commented Nov 16, 2022

Any news on this request ?
It would be appreciated to clean seeds after ci jobs.
Thanks!

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.

@darrylng thank you for working on this sorry its taken longer than anticipated to get it in great work on this!

@McKnight-42 McKnight-42 merged commit aa31c8b into dbt-labs:main Dec 21, 2022
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-264] [CT-262] [Feature] Support BigQuery labels for seeded models
5 participants