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

Add tags to SavedQuery #366

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Conversation

theyostalservice
Copy link
Contributor

@theyostalservice theyostalservice commented Nov 11, 2024

Resolves #369

Description

Addresses internal Linear issue SL-2896.

Users currently can execute parts of their DAG conditionally based on tags added to individual nodes (see documentation). This brings SavedQueries in line with other similar nodes and allows the use of tags as described in that documentation. (See the added tests for examples.)

It does not add any hierarchical behaviors for these tags.

A related PR #10987 is in progress in dbt-core.

Checklist

@cla-bot cla-bot bot added the cla:yes label Nov 11, 2024
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @theyostalservice and the rest of your teammates on Graphite Graphite

Copy link

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.

@theyostalservice theyostalservice force-pushed the patricky__add_tags_to_savedquery branch 2 times, most recently from 73dbdd1 to 7e5037e Compare November 13, 2024 18:35
@theyostalservice theyostalservice marked this pull request as ready for review November 13, 2024 19:07
@theyostalservice theyostalservice changed the title InProgress: Add tags to SavedQuery Add tags to SavedQuery Nov 13, 2024

Choose a reason for hiding this comment

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

@theyostalservice Could you add some test cases that verify that tags are "additive" for semantic models when relevant tag configs appear both within dbt_project.yml and stuff.yml files?

I'll explain more below 😎

"Clobbering" for enabled

Saved queries can have configuration added in dbt_project.yml as well as properties / "schema" YAML files.

Using the enabled config as an example:

dbt_project.yml

name: dbt_project_name_here

saved-queries:
  dbt_project_name_here:
    +enabled: false

models/_saved_queries.yml

saved_queries:
  - name: saved_query_name
    config:
      enabled: true

The above would make all saved queries not enabled by default ("disabled"). But then it would enable saved_query_name. This is the "clobbering" behavior that most dbt configs have.

"Additive" for tags

But for tags in semantic models, we might have config like this:

dbt_project.yml

saved-queries:
  dbt_project_name_here:
    +tags: ["tag_1", "tag_2"]

models/_saved_queries.yml

saved_queries:
  - name: saved_query_name
    config:
      tags: ["tag_3"]

The above would make the default tags for saved queries be the following by default:
["tag_1", "tag_2"]

But then saved_query_name would have:
["tag_1", "tag_2", "tag_3"]

This is the "additive" behavior of tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that dbt_project.yml behavior is driven here in dsi - i think it's handled in dbt-core. I can't even find "enabled" in this repo, tbh.

@courtneyholcomb - do you know if this is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I can answer this now.

@dbeatty10 - that sort of behavior is not managed in the dsi repo; the extra project-level config stuff is all handled in dbt-core. dbt-labs/dbt-core#10987 isn't ready for review, but in some manually run tests, the updated code there works as you described. :-)

(Specifically, the changes in core/dbt/parser/schema_yaml_readers.py produce the change you're looking for. :-) )

Copy link
Contributor

@DevonFulcher DevonFulcher left a comment

Choose a reason for hiding this comment

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

LGTM!

@courtneyholcomb
Copy link
Contributor

@theyostalservice do you know how urgent it is to get this change into core? Asking because we have a bit of a mess we need to detangle for the latest DSI release that will need to be done before this can be released - context here if you're curious (but you don't need to read that if the answer is just no rush!).

Copy link
Contributor Author

I have to step out briefly, but I will definitely look into that context and follow up with @Jstein77 re: urgency.

Copy link
Contributor Author

theyostalservice commented Nov 14, 2024

Merge activity

  • Nov 14, 8:32 AM PST: A user started a stack merge that includes this pull request via Graphite.
  • Nov 14, 8:33 AM PST: A user merged this pull request with Graphite.

@theyostalservice theyostalservice merged commit f3a50f0 into main Nov 14, 2024
23 checks passed
@theyostalservice theyostalservice deleted the patricky__add_tags_to_savedquery branch November 14, 2024 16:33
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.

[Feature] Support use of "tags" field for SavedQuery
4 participants