-
Notifications
You must be signed in to change notification settings - Fork 504
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 not_empty_string test #634
Conversation
We discussed on the original issue also adding a |
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.
I like where this is going! Thank you for your patience 🙏 a couple of comments/questions where I'm not sure that I'm understanding it correctly, but otherwise let's get this merged
trim_whitespace: True | ||
config: | ||
severity: error | ||
error_if: "<1" |
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.
Should these be greater than signs?
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.
No, they shouldn't! This is a very clever way to force the tests to check the negative and positive case.
I really like this 🧠
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.
All credit goes to @dbeatty10 on this approach, but it feels good knowing that we're testing both the pass and fails.
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.
@epapineau this looks great! just checking if you meant to change the postgres env vars?
This is a:
All pull requests from community contributors should target the
main
branch (default).Description & motivation
Resolves #632
Checklist
star()
source)limit_zero()
macro in place of the literal string:limit 0
dbt_utils.type_*
macros instead of explicit datatypes (e.g.dbt_utils.type_timestamp()
instead ofTIMESTAMP