-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Tags for sources and columns (#1906, #1586) #2039
Conversation
227d7b2
to
19c015f
Compare
19c015f
to
88e2bdc
Compare
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.
@beckjake this overall looks very good to me!
It would be great to bubble column-level tags up to their parent models. I'd love to be able to run
dbt run -m tag:pii+
to re-run all models that contain columns containing PII data, for instance. Would something like this require a big re-architecting of how we parse nodes?
I do see that the columns
dict in model nodes contains a tags
attribute. Can we either:
- collect column-level tags into a node-level
column_tags
list, then check that in node selection? - loop through column tags when selecting nodes to run, or
- something else?
My preference would be the first one (adding a column_tags
list to model
, seed
, and snapshot
nodes) as it would make rendering column-level tags in the docs a little bit easier IMO.
What do you think?
We can do that, but then you can't run tests on a given column/set of columns alone, and instead can only run tests on all models that have any columns with that tag. Is that ok? Edit: I guess we could special-case selection so tests worked even more differently from |
88e2bdc
to
88e2f43
Compare
This is a really great point - I hadn't considered this! I do think special casing selection would work here, but I'm not super inclined to make that code any more complicated than it already is. My instinct is just that |
Ok, following up on our IRL chat:
|
Allow column-level tags, which are inherited by their tests
88e2f43
to
2ad5122
Compare
- edited a test to exercise it
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.
One comment about coercing user-supplied values, but this LVGTM otherwise!
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.
LGTM when the tests pass!
Fixes #1906
Fixes #1586 (mostly)
Add support for tags on sources and columns.
Source tags can be applied at both the 'source' and 'table' level, and a source's tags are the combination of the two.
Column tags are applied at the column level and are only applied to tests of that column (and they apply to all tests of that column!). That means if you have no tests, column tags do nothing.
There's no way to tag a subset of a column's tests, and no way to tag tests defined at the table level beyond the usual behavior of selecting all models downstream from a model/source