-
Notifications
You must be signed in to change notification settings - Fork 72
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
Support connectors in datamap api #2249
Support connectors in datamap api #2249
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay reviewing this @TheAndrewJackson I kept getting sidetracked on other things. Good set of changes here to link connectors to systems in a non-aggressive way. Requested changes should be quick to address.
Also thanks for your github comments on various code lines. I always appreciate it when devs take time to annotate their code to help reviewers.
src/fides/api/ctl/migrations/versions/5b4b9c2d1c93_add_system_foreign_key_to_connection_.py
Outdated
Show resolved
Hide resolved
While doing this commit I found some issues with the previous patching commit. Fixing them required making a new connections util module to avoid circular dependencies
@pattisdr Thanks for the awesome review! All of your feedback should be implemented now. I'm not sure why so many of the CI jobs are failing. It looks like none of the tests are actually running. I'll look into it |
@pattisdr Everything is fixed now 👍 Now the only issue is the code climate stuff I mentioned |
thanks @TheAndrewJackson will take a look |
src/fides/api/ctl/migrations/versions/5b4b9c2d1c93_add_system_foreign_key_to_connection_.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work @TheAndrewJackson thanks for those refactors. just some housekeeping with the changelog and it's good to go
Closes https://github.com/ethyca/fidesplus/issues/498
Code Changes
ctl_systems
foreign key to theconnnectionconfig
tableget_connections
route to have a filter for connections not linked with a systemSteps to Confirm
New routes
nox -s dev
Updated route
nox -s dev
get_connections
route withorphaned_from_system=true
as the query param and make sure it returns all of the connections without linked systems.Pre-Merge Checklist
CHANGELOG.md
Description Of Changes
These changes are for future UI updates that will have connectors integrate with the datamap UI. This PR will also support future changes to system and connector creation. Eventually the UI will enforce that all new connections are linked to a system upon creation. The new patch route will enforce that from the backend.
The new
orphaned_from_system
query param is to give us a way to help users migrate their orphaned connections and link them with systems.