-
Notifications
You must be signed in to change notification settings - Fork 1.8k
propagate meta/tags on columns to top-level and config #11992
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
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11992 +/- ##
==========================================
- Coverage 91.95% 91.92% -0.03%
==========================================
Files 200 200
Lines 24451 24468 +17
==========================================
+ Hits 22483 22493 +10
- Misses 1968 1975 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
On the topic of source column tests appear to be missing out on this fixNoticed something interesting when attempting to test "state:modified"... # models/schema.yml
# Full project: https://github.com/jeremyyeo/dbt-basic/tree/qa-dbt-core-11992
sources:
- name: public
tables:
- name: source_old_way
columns:
- name: id
tags: ["my_tag"]
tests:
- not_null
- name: source_new_way
columns:
- name: id
config:
tags: ["my_tag"]
tests:
- not_null
models:
- name: mdl_from_old
columns:
- name: id
tags: ["my_tag"]
tests:
- not_null
- name: mdl_from_new
columns:
- name: id
config:
tags: ["my_tag"]
tests:
- not_null
seeds:
- name: seed_old
columns:
- name: id
tags: ["my_tag"]
tests:
- not_null
- name: seed_new
columns:
- name: id
config:
tags: ["my_tag"]
tests:
- not_null
snapshots:
- name: snap_old
relation: ref('seed_old')
config:
unique_key: id
strategy: check
check_cols: all
columns:
- name: id
tags: ["my_tag"]
tests:
- not_null
- name: snap_new
relation: ref('seed_new')
config:
unique_key: id
strategy: check
check_cols: all
columns:
- name: id
config:
tags: ["my_tag"]
tests:
- not_null 4 resource types (source/model/seed/snapshot) - each with a "new way" (tags in config) and an "old way" (tags as top level). Using $ dbt ls -s 'tag:my_tag'
04:13:57 Running with dbt=1.11.0-a1
04:13:57 Registered adapter: postgres=1.9.1-a0
04:13:57 Found 2 models, 2 seeds, 8 data tests, 2 snapshots, 2 sources, 434 macros
analytics.not_null_mdl_from_old_id
analytics.not_null_seed_old_id
analytics.not_null_snap_old_id
analytics.source_not_null_public_source_old_way_id Using $ dbt ls -s 'tag:my_tag'
04:16:53 Running with dbt=1.11.0-a2
04:16:54 Registered adapter: postgres=1.9.1-a0
04:16:54 Unable to do partial parsing because of a version mismatch
04:16:54 Found 2 models, 2 seeds, 8 data tests, 2 snapshots, 2 sources, 434 macros
analytics.not_null_mdl_from_new_id
analytics.not_null_mdl_from_old_id
analytics.not_null_seed_new_id
analytics.not_null_seed_old_id
analytics.not_null_snap_new_id
analytics.not_null_snap_old_id
analytics.source_not_null_public_source_old_way_id ^ We can see that we list 7 now - missing out on the source_new_way column test...
Checking the The source-using-the-new-way's test is still missing the top level tag: "test.analytics.source_not_null_public_source_new_way_id.4c9bb957a0": {
...
"tags": [],
...
},
"tags": [],
"description": "", But (models|seeds|snapshots)-using-the-new-way's test now have that propagated: "test.analytics.not_null_mdl_from_new_id.9c4f287810": {
...
"tags": [],
...
},
"tags": ["my_tag"],
"description": "", On the topic of "state:modified" false positiveWith the caveat of the above issue, it did not appear that this causes any false positives: $ dbt ls -s state:modified+ --defer --state target_old
04:45:05 Running with dbt=1.11.0-a2
04:45:05 Registered adapter: postgres=1.9.1-a0
04:45:05 Unable to do partial parsing because saved manifest not found. Starting full parse.
04:45:06 Found 2 models, 2 seeds, 8 data tests, 2 snapshots, 2 sources, 434 macros
04:45:06 The selection criterion 'state:modified+' does not match any enabled nodes
04:45:06 No nodes selected! ^ Other musingsI'm wondering if this is because "source" is a special type of node which we don't parse (since sources aren't really nodes that dbt "builds" per se (dbt doesn't actually materialize sources unlike models/snapshots/seeds))... which is linked to #11973 I put a Just to test some stuff. When I exclusively have source tests in my project... I don't seem to hit this breakpoint - but I do hit it with model tests. |
Thank you for doing this validation in addition to the false positive check! Turns out source tests were parsed separately and I added some handling for the tag propagation and confirmed + tested selection is working as expected now. |
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.
Minor NIT, but good to send it in my opinion 🚀
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.
🚢 🇮🇹
(cherry picked from commit b783c97)
Resolves #11984
Problem
Adding
tags
/meta
at the column config-level does not make the it through totag
selection for tests.Solution
🎩 for selection
🎩 for false positives
In both these situations, running
dbt ls state:modified --no-partial-parse
I get 'No nodes selected!', as expected. I've also tested this both on model and source columns, as well as added a test scenario in happy_path_project and this is validated through test.
I've confirmed the manifest.json does change such that config-level vs column-level tags/meta are propagated in either direction, neither influences state:modified selection.
column.tags
andcolumn.meta
does not currently influence state:modified for models or sources (only their own tags/meta do) - even when under config. and changingtags
on columns also does not influence state:modified selection of the tests on that column.Checklist