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

Upgrade dbt generate artifact types #2598

Merged
merged 7 commits into from
May 16, 2023

Conversation

atvaccaro
Copy link
Contributor

@atvaccaro atvaccaro commented May 15, 2023

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

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.

  • No action required
  • Actions required (specified below)
    Merge outstanding Sentry issues into new issues created with new fingerprint.

atvaccaro and others added 3 commits May 15, 2023 13:02
* 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
@atvaccaro atvaccaro self-assigned this May 15, 2023
@github-actions
Copy link

github-actions bot commented May 15, 2023

Warehouse report 📦

New models 🌱

Changed models 🔀

staging.audit.stg_audit__cloudaudit_googleapis_com_data_access

@atvaccaro atvaccaro marked this pull request as ready for review May 15, 2023 18:38
@atvaccaro atvaccaro added the do-not-merge Do not merge, even if approved label May 15, 2023
Copy link
Contributor

@lauriemerrell lauriemerrell left a 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"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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):
Copy link
Contributor

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

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor Author

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

@atvaccaro
Copy link
Contributor Author

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

ah yes thanks; I need to use the string in the fingerprint, and then also clean up the resulting Sentry changes

@atvaccaro atvaccaro removed the do-not-merge Do not merge, even if approved label May 16, 2023
@atvaccaro atvaccaro merged commit e6ddfc9 into main May 16, 2023
4 checks passed
@atvaccaro atvaccaro deleted the upgrade-dbt-generate-artifact-types branch May 16, 2023 15:43
atvaccaro added a commit that referenced this pull request May 17, 2023
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-create dbt artifact Pydantic classes from official jsonschema
2 participants