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

Handling an invalidly generated fides_key #761

Merged
merged 4 commits into from
Jun 15, 2022

Conversation

SteveDMurphy
Copy link
Contributor

@SteveDMurphy SteveDMurphy commented Jun 14, 2022

Closes #749

Code Changes

  • Validate the fides_key as part of generating a dataset
  • Provide an automated way to sanitize the fides_key
  • Provide an option to manually intervene with the fides_key removed from this for further thought (i.e. option for API)
  • provide a CLI option to accept all sanitized versions? (May be too early for this) yes, too early - see above for reasons pulled

Steps to Confirm

  • So far my testing has been limited to importing the new function and throwing some invalid strings at it (i.e. check_fides_key("sample@"))

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

This bug was found while working with a design partner. Further steps may include looking at if the fides_key already exists and providing an option to accept all suggested values.

After chatting with the team, decided against providing a manual intervention option at this time. Two questions remain prior to merging:

  1. Do we also apply this to the different generate command options (i.e. systems)
  2. Can we put this in a minor release 1.6.1

Due to the myriad of names allowed by different style databases, we need a method that allows some intervention without breaking. This is a start to handle invalid characters when generating a dataset. The function should be allowed to be repurposed when generating other types of resources as well.

A further step would be to also validate if the fides_key already exists due and how we may want to handle that.
@SteveDMurphy SteveDMurphy self-assigned this Jun 15, 2022
@SteveDMurphy SteveDMurphy added the bug Something isn't working label Jun 15, 2022
@SteveDMurphy SteveDMurphy marked this pull request as ready for review June 15, 2022 01:53
@SteveDMurphy SteveDMurphy requested a review from a team June 15, 2022 01:53
@SteveDMurphy SteveDMurphy mentioned this pull request Jun 15, 2022
10 tasks
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

Naming nit, but otherwise looks great!

@SteveDMurphy SteveDMurphy added this to the 1.7.0 milestone Jun 15, 2022
@SteveDMurphy SteveDMurphy merged commit 338b729 into main Jun 15, 2022
@SteveDMurphy SteveDMurphy deleted the SteveDMurphy-749-fides-key-error-handling branch June 15, 2022 13:17
SteveDMurphy added a commit that referenced this pull request Jun 15, 2022
* validate the generated fides_key

Due to the myriad of names allowed by different style databases, we need a method that allows some intervention without breaking. This is a start to handle invalid characters when generating a dataset. The function should be allowed to be repurposed when generating other types of resources as well.

A further step would be to also validate if the fides_key already exists due and how we may want to handle that.

* remove manual intervention and cli specific steps

* testing

* rename variable, fix docs from removed manual step
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fidesctl generate dataset ... key validation error
2 participants