-
Notifications
You must be signed in to change notification settings - Fork 14
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
Validate granularity names in saved query where filters #359
Conversation
Validations for time spines in WHERE filters in Saved Queries. This mimics the where filter time spine validation for metrics and applies it to saved queries. This also bumps the version. (I assume this is necessary?)
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @theyostalservice |
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @theyostalservice and the rest of your teammates on Graphite |
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. |
254fa29
to
b7f84b8
Compare
This is not *exactly* what was described in Courtney's comments, but I think there was a little roughness in that plan once I began implementing (but I'm happy to change course if reviewers disagree!). This commit promotes the `WhereFiltersAreParseable` to be `WhereFiltersAreParseableRule` (a free-standing 'rule' in a separate file). It was weird to be passing things in from other classes but somehow centralizing the manifest, so instead, I just moved ALL of the relevant checks here. This moves the tests for where filters to a new file specifically for this rule (again, I'm open to the idea that this would be better just divided amongst the existing tests, but they share so much conceptually that it seems nice to group them together and to have a test that is pointed 1:1 at a rule where possible). Finally, I also moved some of the test support functions (`check_only_one_error_with_message`, for example) to `dbt_semantic_interfaces.test_utils` because they seem useful and reusable.
b7f84b8
to
ad329c5
Compare
For visibility - I wanted to make sure the explanation of where I expanded on the feedback from @courtneyholcomb was visible and explain the reasoning, so I'm copying the commit message from the latest update to a comment: Centralize Where filter validation + commentsThis is not exactly what was described in Courtney's comments, but I This commit promotes the This moves the tests for where filters to a new file specifically Finally, I also moved some of the test support functions |
dbt_semantic_interfaces/validations/semantic_manifest_validator.py
Outdated
Show resolved
Hide resolved
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.
Looks great thank you!!
Resolves #360
Description
Validate time spines in saved query where filters
Validations for time spines in WHERE filters in Saved Queries.
This mimics the where filter time spine validation for metrics and applies it to saved queries.
This also bumps the version. (I assume this is necessary?)
Checklist
changie new
to create a changelog entry