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" and "meta" properties to exposure schema #3405

Merged
merged 6 commits into from
Jun 1, 2021

Conversation

stkbailey
Copy link
Contributor

@stkbailey stkbailey commented May 30, 2021

resolves #3404

Description

This PR enables users to use the tags and meta properties on exposures, in the exact same way they would with other resources. Given the "read-only" nature of exposures, the main use case is around enabling more metadata to be added to exposures, to make them a more authoritative record of data products.

I have not included any changes that would enable selection of exposures using tags, a la dbt ls --resource-type exposure --select tag:marketing but I would be open to adding these to the PR if that is desired.

I have also not altered anything in the docs server that would enable visualization there and not sure I'm qualified to do that work 😀

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 to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label May 30, 2021
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

I have not included any changes that would enable selection of exposures using tags, a la dbt ls --resource-type exposure --select tag:marketing but I would be open to adding these to the PR if that is desired.

No need: this just works.

$ dbt ls --resource-type exposure --select tag:my_test_tag
exposure:my_project.my_test_exposure

I have also not altered anything in the docs server that would enable visualization there and not sure I'm qualified to do that work 😀

The meta properties show up just fine for exposures, and I believe we'd only need to change one line to get tags to show up too. It's just a matter of removing the part that says exclude="['tags']". Is that a change you'd be up to contribute? If so, could you open a new (stub) issue and new PR over in the dbt-docs repo?

In any case, this change LGTM. Thanks so much for opening the issue, owning the contribution, and seeing this get over the finish line!

@@ -725,6 +722,7 @@ def same_url(self, old: 'ParsedExposure') -> bool:

def same_contents(self, old: Optional['ParsedExposure']) -> bool:
# existing when it didn't before is a change!
# metadata/tags changes are not "changes"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Agree with this.

Someday we may want to revisit the logic here (should a change to depends_on count?), but in a world where users run -m state:modified+ in CI, I don't think an exposure marked modified would even alter behavior there.

@jtcohen6 jtcohen6 merged commit d89e1d7 into dbt-labs:develop Jun 1, 2021
@stkbailey
Copy link
Contributor Author

Is that a change you'd be up to contribute? If so, could you open a new (stub) issue and new PR over in the dbt-docs repo?

Sure thing!

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.

Add "tags" and "meta" attributes to Exposure schema
2 participants