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.

Sign up for GitHub

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

Jump to bottom

Generated dataset fides_keys are unique #1786

Merged
merged 5 commits into from
Nov 20, 2022
Merged

Generated dataset fides_keys are unique #1786

merged 5 commits into from
Nov 20, 2022

Conversation

SteveDMurphy
Copy link
Contributor

@SteveDMurphy SteveDMurphy commented Nov 15, 2022 •
edited
Loading

Closes fidesplus-licensing#62

Code Changes

  • Added function to create repeatable hash
  • Update generated datasets to have a repeatable uuid as suffix of fides_key. Also include hash components in meta

Steps to Confirm

  • Generated datasets both through the API and CLI successfully
  • Validate the args passed to classify are updated to reflect new fides_key values as well

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

Currently this is geared heavily towards datasets but could be more agnostic to allow for uniqueness in other resources (i.e. Systems) as well

@SteveDMurphy
append a repeatable uuid to fides_key for dataset
befbd95 
As part of the generation of datasets this is particularly important to help in building fides resources at scale. This should allow for a repeatable ID to be created and reference schemas with common names
@SteveDMurphy
changelog
4b2d01a
@SteveDMurphy SteveDMurphy self-assigned this Nov 15, 2022
@SteveDMurphy SteveDMurphy marked this pull request as ready for review November 15, 2022 20:30
@SteveDMurphy
Copy link
Contributor Author

Wanted to bump this if you have time @ThomasLaPiana, also feel free to reassign!

@ThomasLaPiana
Copy link
Contributor

sorry @SteveDMurphy , getting to it now!! This is a great change 🙂

ThomasLaPiana
ThomasLaPiana approved these changes Nov 18, 2022
View reviewed changes
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.

Approving, but left some nits/food for thought :)
Great fix here, very valuable feature!

@SteveDMurphy SteveDMurphy merged commit 39b83a5 into main Nov 20, 2022
@SteveDMurphy SteveDMurphy deleted the SteveDMurphy-62-dataset-unique-key branch November 20, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomasLaPiana ThomasLaPiana ThomasLaPiana approved these changes

Assignees

@SteveDMurphy SteveDMurphy

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SteveDMurphy @ThomasLaPiana