-
Notifications
You must be signed in to change notification settings - Fork 14
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
Upgrade dbt generate artifact types #2598
Conversation
* upgrade dbt-bigquery * create gen_artifacts.sh and use it * wip on horrific code * continue subclassing/patching generated models * start working on mypy compatibility * remove empty test block * continue working on mypy passing * make mypy pass * fixes post-upgrade * address comments
Warehouse report 📦 New models 🌱Changed models 🔀staging.audit.stg_audit__cloudaudit_googleapis_com_data_access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we say last time that there should be a follow-up action to clean up Sentry because changing the types here nukes the fingerprinting
DbtMaterializationType.table, | ||
DbtMaterializationType.incremental, | ||
): | ||
if self.config and self.config.materialized in ("table", "incremental"): | ||
fillcolor = "aquamarine" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still used? I feel like I haven't been seeing the visualizations in CI. Are they currently suppressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's turned off currently but it will still run locally. There's actually a solution described in iterative/cml#1377 that I can implement soon-ish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
follow-up PR here #2605
runtime_error = "runtime error" | ||
|
||
|
||
class PeriodEnum(Enum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it would be more trouble than it's worth to try to dedupe but it's a bit weird to have identical enums defined over and over again across files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I've been meaning to open an upstream issue asking why they were defined like this in the json schema
""" | ||
Super useful with https://www.convertjson.com/html-table-to-json.htm | ||
|
||
Originally used to produce dbt docs from https://gtfs.org/reference/static#field-definitions | ||
|
||
This won't work without updates as the artifacts have changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you maybe rename the file to like _outdated
or something more obvious to indicate this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed offline, going to leave as-is since it could be useful in the future if updated to use the new types
ah yes thanks; I need to use the string in the fingerprint, and then also clean up the resulting Sentry changes |
* upgrade dbt-bigquery and re-generate artifact types (#2545) * upgrade dbt-bigquery * create gen_artifacts.sh and use it * wip on horrific code * continue subclassing/patching generated models * start working on mypy compatibility * remove empty test block * continue working on mypy passing * make mypy pass * fixes post-upgrade * address comments * upgrade dbt to 1.6 preview, and fix data access model full refresh * just determine partitions dynamically * allow extra fields in generated artifact models * make mypy pass
Description
Describe your changes and why you're making them. Please include the context, motivation, and relevant dependencies.
Builds on #2545 (which had been reverted)
Resolves #2546
Pre-requisite for #2540 and #2346 (need dbt 1.5)
The preview version 1.6.0b1 contains a fix for the policy tags regression. I also changed our pydantic generation to allow for extra fields, since 1.6 includes some new fields not yet in the publish schemas.
Type of change
How has this been tested?
Include commands/logs/screenshots as relevant.
Verified that the failed models pass with 1.6.0b1; I will be running a full refresh in staging.
Post-merge follow-ups
Document any actions that must be taken post-merge to deploy or otherwise implement the changes in this PR (for example, running a full refresh of some incremental model in dbt). If these actions will take more than a few hours after the merge or if they will be completed by someone other than the PR author, please create a dedicated follow-up issue and link it here to track resolution.
Merge outstanding Sentry issues into new issues created with new fingerprint.