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

Enable functionality to use common schema in model database #30

Merged
merged 9 commits into from
May 28, 2022

Conversation

jodybuz
Copy link
Contributor

@jodybuz jodybuz commented May 12, 2022

A new variable has been added use_common_masking_policy_schema_only which allows for a common schema within the model database to be specified for storing masking policies.

(Similar to existing functionality which uses use_common_masking_policy_db to set a common database and schema to store masking policies.)

@entechlog
Copy link
Owner

entechlog commented May 13, 2022

@jodybuz , Thank You for the PR. I will review/test and get back.

  • Can you also update the README.md with the details on how to use this new feature ?
  • I also see changes to "integration_tests/dbt_project.yml", was that required ?

@jodybuz
Copy link
Contributor Author

jodybuz commented May 13, 2022

Thanks @entechlog - I have made some updates to the README this morning. Hopefully it makes things easier to understand.

I've also removed the formatting in the "integration_tests/dbt_project.yml" file. (Sorry - these were added by my linter - so not necessary.) But I've kept in my new parameter to enable future testing - hope thats ok?

@entechlog
Copy link
Owner

Thank You @jodybuz , I will test this and get it merged

@jodybuz
Copy link
Contributor Author

jodybuz commented May 18, 2022

Hi @entechlog - Sorry, but I've added one last change into this PR (hope it's not too late.) We have custom materializations in our dbt project, so I've added a parameter which allows your package to apply masking policies to these types of models. I've updated the README with instructions on how to use this feature too.

@entechlog
Copy link
Owner

@jodybuz , Sure. Sorry I have been busy for last couple of days. I will try to take a look at them today evening

@nadesansiva
Copy link
Collaborator

@jodybuz , The tests ran fine, so going a head and merging this now.

@nadesansiva nadesansiva merged commit 7a20f74 into entechlog:main May 28, 2022
@jodybuz
Copy link
Contributor Author

jodybuz commented Jun 6, 2022

Thanks @entechlog and @nadesansiva - this is going to be so useful for us (and hopefully others too.)

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.

3 participants