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

Adds quote parameter to accepted_values test #1874

Closed
wants to merge 7 commits into from
Closed

Adds quote parameter to accepted_values test #1874

wants to merge 7 commits into from

Conversation

clausherther
Copy link
Contributor

Adds quote parameter to accepted_values test. Closes #1873.

Users can now test for acceptable integer values by opting not to quote these values.

...
         - name: day_of_week
            description: 
            tests:
              - not_null
              - accepted_values:
                  values: [1, 2, 3, 4, 5, 6, 7]
                  quote: false
...

Results in:

with all_values as (
    select distinct
        day_of_week as value_field
    from dw.dim_date
),
validation_errors as (
    select
        value_field
    from 
        all_values
    where
        value_field not in (
            1,
            2,
            3,
            4,
            5,
            6,
            7
            )
)

select count(*)
from validation_errors

While the default behavior is to quote, e.g.

...
         - name: day_of_week_name
            description: 
            tests:
              - not_null
              - accepted_values:
                  values: [Sunday, Monday, Tuesday, Wednesday, Thursday, Friday, Saturday]
...
with all_values as (
    select distinct
        day_of_week_name as value_field
    from dw.dim_date
),
validation_errors as (
    select
        value_field
    from 
        all_values
    where
        value_field not in (
            'Sunday',
            'Monday',
            'Tuesday',
            'Wednesday',
            'Thursday',
            'Friday',
            'Saturday'
            )
)

select count(*)
from validation_errors

Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

One comment regarding a typo here, but this looks great otherwise :)

Since this is a built-in schema test, I think it would be a good idea to add a test for this change. That might be as easy as updating one of these schema tests to provide quote: true. It's not super important to test both the true and false branches -- I just want to make sure that we know if we ever accidentally break this thing in a seemingly unrelated part of the codebase (like .yml parsing).

@clausherther
Copy link
Contributor Author

Added a test to check unquoted accepted_values for id in table_copy, and am explicitly quoted test on table_copy.favorite_color. Hope that does it!

@cla-bot
Copy link

cla-bot bot commented Oct 29, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Claus Herther.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https:/settings/emails

@dbt-labs dbt-labs deleted a comment from cla-bot bot Oct 29, 2019
@dbt-labs dbt-labs deleted a comment from cla-bot bot Oct 29, 2019
@dbt-labs dbt-labs deleted a comment from cla-bot bot Oct 29, 2019
@cla-bot
Copy link

cla-bot bot commented Oct 29, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Claus Herther.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https:/settings/emails

@clausherther
Copy link
Contributor Author

Btw, I've tried rebasing to my earliest commit (fffdd342b17189bde8508de7c6a2b5ef6d58f4e8) a few times, but I don't seem to have any luck.
I rarely do this, so not sure, but I thought it was git rebase -i fffdd342b17189bde8508de7c6a2b5ef6d58f4e8 and then I should see the 5 or 6 commits in the editor. However, the git to do file that opens is just blank and nothing gets squashed.

@cla-bot
Copy link

cla-bot bot commented Oct 29, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Claus Herther.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https:/settings/emails

@clausherther
Copy link
Contributor Author

Hey @drewbanin I'm closing this and will reopen on another fork/PR to fix this author issue. Looks like the tests passed otherwise now, so the next one should be easier.

@clausherther clausherther deleted the add/quote-accepted-values branch October 29, 2019 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add quote parameter to accepted_values test
2 participants