Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

136 mysql query execution #168

Merged
merged 6 commits into from
Jan 25, 2022
Merged

136 mysql query execution #168

merged 6 commits into from
Jan 25, 2022

Conversation

eastandwestwind
Copy link
Contributor

@eastandwestwind eastandwestwind commented Jan 24, 2022

Purpose

Adds query execution support for MySQL Connector.

Changes

  • Adds integration access and erasure tests for MySQL, and 1 missing one for MsSQL
  • Overrides SQLConnector where appropriate to handle LegacyCursorResult return type from SQLAlchemy.
  • Updates Postman Collection and Postman Docs to reflect MySQL-specific config

Checklist

  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram
  • Good unit test/integration test coverage

Ticket

Fixes #136

@pattisdr pattisdr self-assigned this Jan 24, 2022
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.

thank you @eastandwestwind, well-organized, easy to follow, just a few changes requested ⭐

src/fidesops/service/connectors/sql_connector.py Outdated Show resolved Hide resolved
src/fidesops/service/connectors/sql_connector.py Outdated Show resolved Hide resolved
tests/integration_tests/test_sql_task.py Outdated Show resolved Hide resolved
tests/integration_tests/test_sql_task.py Show resolved Hide resolved
tests/integration_tests/test_sql_task.py Show resolved Hide resolved
@eastandwestwind
Copy link
Contributor Author

Thanks for the comments @pattisdr ! Ready for review again

@pattisdr pattisdr added the run unsafe ci checks Triggers running of unsafe CI checks label Jan 25, 2022
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.

Wonderful @eastandwestwind!

More of a general comment, related to Sean's testing work, it will be good when we are testing each datastore against every data type; we may be missing datastore-specific handling in places. But this PR definitely demonstrates basic MySQL access/erasures in line with how we introduced the other datastores and we can continue to improve our test matrix. Thanks again for adding MySQL to the postman collection!

@pattisdr pattisdr merged commit 7708b77 into main Jan 25, 2022
@pattisdr pattisdr deleted the 136-mysql-query-execution branch January 25, 2022 15:52
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query Execution for MySQL
2 participants