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

Handle "meta:" key in schema/sources.yml #2015

Merged

Conversation

tayloramurphy
Copy link

@tayloramurphy tayloramurphy commented Dec 18, 2019

Closes #1362

This PR adds the ability for people to use the key meta: to add arbitrary information to their schema.yml and sources.yml files.

I have tested locally by having this in my schema.yml

version: 2
models:
  - name: sfdc_account
    description: base model for SFDC Accounts tayloramurphy
    meta:
      owned: "@tayloramurphy"
    columns:
      - name: account_id
        meta:
          sensitivity: 1

and in my sources.yaml

version: 2

sources:
  - name: salesforce
    database: '{{ env_var("SNOWFLAKE_LOAD_DATABASE") }}'
    schema: salesforce_stitch
    loader: Stitch
    loaded_at_field: _sdc_batched_at
    meta:
      owner: "@tayloramurphy_source_data"
    
    quoting:
      database: true
      schema: false
      identifier: false

    freshness:
        warn_after: {count: 8, period: hour}
        error_after: {count: 24, period: hour}

    tables: 
      - name: account
        description: '{{ doc("sfdc_account_source") }}'
        meta:
          sensitivity: "@tayloramurphy_source_data_table"

And was able to see in the manifest.json the data written appropriately. dbt also ran just fine.

@cla-bot
Copy link

cla-bot bot commented Dec 18, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Taylor A. Murphy.
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@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@tayloramurphy
Copy link
Author

I just signed the CLA.

@beckjake @drewbanin do I need to add anything to the tests for this? I saw several schema.yml files in the integration tests section but wasn't sure where this would best fit in.

@drewbanin
Copy link
Contributor

Thanks for opening this PR @tayloramurphy! (@cla-bot check)

I just added a comment to the discussion on naming this field in the issue. Let me know what you think about that suggestion in the issue thread.

I'll check out the code changes here. Kicking off the tests presently.

@cla-bot
Copy link

cla-bot bot commented Dec 18, 2019

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Taylor A. Murphy.
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@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Dec 18, 2019

The cla-bot has been summoned, and re-checked this pull request!

@cla-bot cla-bot bot added the cla:yes label Dec 19, 2019
@tayloramurphy tayloramurphy changed the title Handle "data:" key in schema/sources.yml Handle "meta:" key in schema/sources.yml Dec 19, 2019
@tayloramurphy
Copy link
Author

I see the failing tests - will work on updating.

@tayloramurphy
Copy link
Author

@drewbanin pg integration tests are failing but it looks like it's on the install postgres step. I tried submitting another commit to see if it was a one-off but maybe there's something deeper? Please advise.

@drewbanin
Copy link
Contributor

@tayloramurphy this is definitely weird - I see:

[NuGet] The operation has timed out

It looks like it's related to an incident over here: https://status.chocolatey.org/

I'll kick off these tests again when it looks like things have been resolved w/ chocolatey

@beckjake
Copy link
Contributor

I saw this happen for a few days in a row with another PR, but could not repro on my Windows box. It mysteriously went away after a time - I guess chocolatey is having some problems.

@drewbanin
Copy link
Contributor

/azp run

@tayloramurphy
Copy link
Author

I'll get back to this after the Christmas holiday!

@tayloramurphy
Copy link
Author

@drewbanin looks like postgres integration tests are passing but AFAICT the BQ, Snowflake, and Redshift integration tests are failing b/c it can't connect. Is this still related to the chocolately outage? https://status.chocolatey.org/issues/2019-12-18-timeouts-when-searching-and-installing-packages/

@tayloramurphy
Copy link
Author

Yes! Tests are passing! @drewbanin or @beckjake officially submitting for review 😄

@drewbanin
Copy link
Contributor

Thanks @tayloramurphy! I gave this a quick functional test and it looks really good to me :)

I also just queued up this issue to render these eventual values in the docs site: dbt-labs/dbt-docs#66

Nice work with the tests here - I know that updating node contracts in the test suite can be a real slog!

Adding @beckjake for code review

Copy link
Contributor

@beckjake beckjake left a comment

Choose a reason for hiding this comment

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

looks great!

@drewbanin drewbanin merged commit 0a5b436 into dbt-labs:dev/barbara-gittings Jan 7, 2020
@drewbanin
Copy link
Contributor

Just merged - thanks for your contribution to dbt @tayloramurphy :D

@tayloramurphy
Copy link
Author

Thank you both for the review! I'm pumped to get this feature in 😄

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.

3 participants