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

Ensure meta is both at node top level and in node.config. Fix snapshots with schema config. #4726

Merged
merged 7 commits into from Feb 17, 2022

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Feb 15, 2022

resolves #4459, #4693

Description

This fixes a bug in schema file config processing, where an empty meta overwrote the root level meta. It also fixes a bug in snapshot schema config handling, where the snapshot SQL file config was lost.

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

@gshank gshank requested review from a team as code owners February 15, 2022 17:12
@gshank gshank requested review from ChenyuLInx and iknox-fa and removed request for a team February 15, 2022 17:12
@cla-bot cla-bot bot added the cla:yes label Feb 15, 2022
@gshank gshank requested review from emmyoop, jtcohen6 and nathaniel-may and removed request for ChenyuLInx and iknox-fa February 15, 2022 17:16
@jtcohen6
Copy link
Contributor

Thrilled that these look like quick fixes! Two quick meta notes (hah):

  • I'm going to remove myself from review — feel free to tag me back if there are questions about the original bugs, or anything specific you want me to look at
  • After merging, I would like to see us backport these fixes to 1.0.latest for inclusion in 1.0.3rc1. These aren't net-new bugs in v1.0 exactly, but it seems likely that they were net-new in v0.21, so folks will encounter them as part of their larger v1.0 upgrades. This will allow us to tell the Metadata team, with confidence, that meta fields are reliable for all v1.0+ versions of dbt.

@jtcohen6 jtcohen6 removed their request for review February 17, 2022 13:51
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment on the changelog but otherwise looks good.

CHANGELOG.md Outdated
@@ -33,6 +33,7 @@ Contributors:
- Restore previous log level (DEBUG) when a test depends on a disabled resource. Still WARN if the resource is missing ([#4594](https://github.com/dbt-labs/dbt-core/issues/4594), [#4647](https://github.com/dbt-labs/dbt-core/pull/4647))
- Add project name validation to `dbt init` ([#4490](https://github.com/dbt-labs/dbt-core/issues/4490),[#4536](https://github.com/dbt-labs/dbt-core/pull/4536))
- Support click versions in the v7.x series ([#4681](https://github.com/dbt-labs/dbt-core/pull/4681))
- Fix bug causing empty node level meta, snapshot config errors ([#4459](https://github.com/dbt-labs/dbt-core/issues/4459), [#4726](https://github.com/dbt-labs/dbt-core/pull/4726))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @jtcohen6's comment this should be moved to a 1.0.3 section.

@gshank gshank merged commit 18fef38 into main Feb 17, 2022
@gshank gshank deleted the ct-64-meta_copy branch February 17, 2022 17:15
timle2 pushed a commit to timle2/dbt-core that referenced this pull request Feb 21, 2022
…ts with schema config. (dbt-labs#4726)

* Do not overwrite node.meta with empty patch.meta

* Restore config_call_dict in snapshot node transform

* Test for snapshot with schema file config

* Test for meta in both toplevel node and node config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-64] Model meta missing from top-level node meta in manifest
3 participants