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

PROD-2823 - Add RDSMySQLConnector #5343

Merged
merged 15 commits into from
Oct 3, 2024
Merged

PROD-2823 - Add RDSMySQLConnector #5343

merged 15 commits into from
Oct 3, 2024

Conversation

andres-torres-marroquin
Copy link
Contributor

@andres-torres-marroquin andres-torres-marroquin commented Oct 1, 2024 •
edited
Loading

Closes #PROD-2823

Description Of Changes

Added RDSMySQLConnector so DnD Integration UI works.

Code Changes

  • Added RDSConnectorMixin
  • Added RDSMySQL to connector logos svgs
  • Added RDSMySQL to connection_types.json
  • Added rdsMySQLInfo.tsx
  • Added RDSMySQLConnector without access and erasure functionality

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation:
    • documentation complete, PR opened in fidesdocs
    • documentation issue created in fidesdocs
    • if there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md
  • For API changes, the Postman collection has been updated
  • If there are any database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!

@vercel Vercel
Copy link

vercel bot commented Oct 1, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 2:19pm

@cypress Cypress
Copy link

cypress bot commented Oct 1, 2024 •
edited
Loading

fides    Run #10272

Run Properties:  status check passed Passed #10272  •  git commit 21ddc43248 ℹ️: Merge a2f716c5dcf1455f8401e5bcd44ae32fcb13e71d into c0fc642c0b6abc9e66e32b26a13f...
Project fides
Branch Review refs/pull/5343/merge
Run status status check passed Passed #10272
Run duration 00m 39s
Commit git commit 21ddc43248 ℹ️: Merge a2f716c5dcf1455f8401e5bcd44ae32fcb13e71d into c0fc642c0b6abc9e66e32b26a13f...
Committer Andres Torres
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
View all changes introduced in this branch ↗︎

@andres-torres-marroquin andres-torres-marroquin changed the title PROD-2823 - MySQL on RDS as a DSR source PROD-2823 - Add RDSMySQLConnector Oct 1, 2024
@andres-torres-marroquin
Copy link
Contributor Author

Screenshot 2024-10-01 at 9 09 27 a m

andres-torres-marroquin
andres-torres-marroquin commented Oct 1, 2024
View reviewed changes
tests/fixtures/google_cloud_sql_mysql_fixtures.py Outdated
@@ -52,6 +52,20 @@ def google_cloud_sql_mysql_connection_config(db: Session) -> Generator:
"keyfile_creds"
) or ast.literal_eval(os.environ.get("GOOGLE_CLOUD_SQL_MYSQL_KEYFILE_CREDS"))

if not db_iam_user:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was missing on the main branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep these as separate PRs in the future. I don't want us to think that we had to change Google Cloud functionality as part of this unrelated PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take into another branch.

andres-torres-marroquin
andres-torres-marroquin commented Oct 1, 2024
View reviewed changes
tests/fixtures/google_cloud_sql_postgres_fixtures.py Outdated
@@ -56,6 +56,23 @@ def google_cloud_sql_postgres_connection_config(db: Session) -> Generator:
"keyfile_creds"
) or ast.literal_eval(os.environ.get("GOOGLE_CLOUD_SQL_POSTGRES_KEYFILE_CREDS"))

if not db_iam_user:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was missing on the main branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take into another branch.

galvana
galvana reviewed Oct 1, 2024
View reviewed changes
src/fides/api/service/connectors/rds_connector_mixin.py Outdated Show resolved Hide resolved
src/fides/api/service/connectors/rds_mysql_connector.py Outdated Show resolved Hide resolved
src/fides/api/service/connectors/rds_mysql_connector.py Outdated Show resolved Hide resolved
src/fides/api/service/connectors/rds_mysql_connector.py Show resolved Hide resolved
tests/fixtures/google_cloud_sql_mysql_fixtures.py Outdated
@@ -52,6 +52,20 @@ def google_cloud_sql_mysql_connection_config(db: Session) -> Generator:
"keyfile_creds"
) or ast.literal_eval(os.environ.get("GOOGLE_CLOUD_SQL_MYSQL_KEYFILE_CREDS"))

if not db_iam_user:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep these as separate PRs in the future. I don't want us to think that we had to change Google Cloud functionality as part of this unrelated PR

src/fides/api/service/connectors/rds_mysql_connector.py Outdated Show resolved Hide resolved
src/fides/api/service/connectors/rds_mysql_connector.py Outdated Show resolved Hide resolved
@andres-torres-marroquin andres-torres-marroquin added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Oct 2, 2024
galvana
galvana approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@galvana galvana left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes @andres-torres-marroquin. This is a good intermediate implementation to unblock the monitor work.

@andres-torres-marroquin andres-torres-marroquin merged commit 0ea77a6 into main Oct 3, 2024
40 of 46 checks passed
@andres-torres-marroquin andres-torres-marroquin deleted the PROD-2823-2 branch October 3, 2024 16:30
@cypress Cypress
Copy link

cypress bot commented Oct 3, 2024

fides    Run #10278

Run Properties:  status check passed Passed #10278  •  git commit 0ea77a67b7: PROD-2823 - Add RDSMySQLConnector (#5343)
Project fides
Branch Review main
Run status status check passed Passed #10278
Run duration 00m 39s
Commit git commit 0ea77a67b7: PROD-2823 - Add RDSMySQLConnector (#5343)
Committer Andres Torres
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
View all changes introduced in this branch ↗︎

Kelsey-Ethyca pushed a commit that referenced this pull request Oct 8, 2024
@andres-torres-marroquin @Kelsey-Ethyca
PROD-2823 - Add RDSMySQLConnector (#5343)
b8d6f53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galvana galvana galvana approved these changes

Assignees
No one assigned
Labels
run unsafe ci checks Runs fides-related CI checks that require sensitive credentials
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andres-torres-marroquin @galvana