add --piionly flag #82
Conversation
| @@ -239,6 +239,8 @@ def write_sql(self, raw_schema, no_pii=False): | |||
| else: | |||
| if no_pii: | |||
| view_types = ["SAFE"] | |||
| elif pii_only: | |||
| view_types = ["PII"] | |||
| else: | |||
There was a problem hiding this comment.
might want to add an erro here if someone specifies both flags. Probably a ValueError ? The flags should be exclusive
There was a problem hiding this comment.
I just saw the below, though I think this could still benefit from something to prevent someone from accidentally making an api call to this with both flags.
There was a problem hiding this comment.
@dnuzz Sounds good, added an initial condition to raise an exception
| ], | ||
| "models": [], | ||
| } | ||
| if not pii_only: |
There was a problem hiding this comment.
maybe take the approach as below and just append to the sources if not pii_only just to make this a bit cleaner and easier to follow. I know I was the one who started this trend so I accept this is me asking you to fix my mistake 😬
There was a problem hiding this comment.
Sounds good, rearranged
|
Oh, can you also add a test to confirm the flag has the expected behavior/ output? |
Description: Adds --piionly flag
JIRA: Link to JIRA ticket
Dependencies: dependencies on other outstanding PRs, issues, etc.
Merge deadline: List merge deadline (if any)
Installation instructions: List any non-trivial installation
instructions.
Testing instructions:
Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.