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

Fideslang 3.0 #186

Merged
merged 7 commits into from
Dec 15, 2023
Merged

Fideslang 3.0 #186

merged 7 commits into from
Dec 15, 2023

Conversation

adamsachs
Copy link
Contributor

@adamsachs adamsachs commented Dec 8, 2023

Closes PROD-1490

Description Of Changes

Updates to remove generally unused/deprecated concepts. This will be a significant breaking change, so it justifies a major upgrade, i.e. 3.0.

Code Changes

  • remove data qualifiers
  • remove deprecated fideslang fields (a lot of them are from the Compass updates - they are marked as deprecated)
  • remove the registry concept - optional

Steps to Confirm

  • extensive integration testing with fides and fidesplus!
  • update docs and references for CLI and general taxonomy
  • ...

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

* feat: remove data qualifiers

* feat: more qualifier removals
Copy link
Contributor Author

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

@pattisdr would you mind taking an initial look at this and associated fides PR ethyca/fides#4502?

i still will need to do some more testing, and i'll need to work in a fidesplus PR and make any adjustments as needed there, but i'd like to get your initial thoughts to make sure there's nothing sticking out as missing!

src/fideslang/models.py Show resolved Hide resolved
src/fideslang/models.py Show resolved Hide resolved
@pattisdr
Copy link
Contributor

Starting review...

Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

Nice Adam 👍

I'm so happy you're deprecating registries too. It's a cause of 500 errors when patching Systems via the API directly because there's actually a mismatch between the database type and fideslang type - Registries was never used as intended and it just causes problems from time to time!

src/fideslang/models.py Show resolved Hide resolved
src/fideslang/models.py Show resolved Hide resolved
@adamsachs adamsachs marked this pull request as ready for review December 14, 2023 15:58
@adamsachs
Copy link
Contributor Author

adamsachs commented Dec 14, 2023

marking as ready for review since i've now done a fair amount of testing with fides and fidesplus and generally things are looking good, feeling comfortable that this is a stable update that has what we need 👍

one thing i need to follow up on - anything to check for generated docs/webpages/visuals? any guidance that you have @pattisdr is appreciated, but i can also dig a bit!

@adamsachs adamsachs self-assigned this Dec 14, 2023
Copy link
Contributor

@pattisdr pattisdr left a comment

Choose a reason for hiding this comment

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

This is great 🏆

@adamsachs adamsachs merged commit ffe60ac into main Dec 15, 2023
31 checks passed
@adamsachs adamsachs deleted the asachs/PROD-1490 branch December 15, 2023 15:29

Choose a reason for hiding this comment

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

@adamsachs Hi, can you point me to the rationale for removing the 4th classification group from the ontology? I still see 4 referenced at certain places in the documentation. Thanks in advance for your attention!

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.

4 participants