HJ-5 - Added new RDS Postgres Connector #5380

andres-torres-marroquin · 2024-10-15T13:57:55Z

Closes #HJ-5

Description Of Changes

Added new RDS Postgres Connector, which allows D&D feature to be implemented.

Code Changes

Added BaseRDSSchema, RDSMySQLSchema now inherits from it.
Added RDSPostgresSchema.
Added RDSPostgresConnector.
Added tests/fixtures/rds_postgres_fixtures.py

Steps to Confirm

Create a Connection with RDS Postgres Credentials.
Click on Test connection.

Pre-Merge Checklist

vercel · 2024-10-15T13:58:01Z

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 21, 2024 2:03pm

CHANGELOG.md

cypress · 2024-10-15T14:12:58Z

fides Run #10521

Run Properties: Passed #10521 • 9c954d8d3d ℹ️: Merge 8e327f413e4d73564c5089c44215543c7c99c09a into ef1b7895f66fa2a82bb65f15e067...

Project	`fides`
Run status	`Passed #10521`
Run duration	`00m 38s`
Commit	`9c954d8d3d ℹ️: Merge 8e327f413e4d73564c5089c44215543c7c99c09a into ef1b7895f66fa2a82bb65f15e067...`
Committer	`Andres Torres`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

src/fides/api/alembic/migrations/versions/5dafbc1818ae_add_rds_postgres_to_connector_type.py

codecov · 2024-10-15T17:05:58Z

Codecov Report

Attention: Patch coverage is 59.78261% with 37 lines in your changes missing coverage. Please review.

Project coverage is 85.48%. Comparing base (7591e3f) to head (7c981a6).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/api/service/connectors/rds_postgres_connector.py	51.47%	33 Missing ⚠️
...ides/api/service/connectors/rds_connector_mixin.py	66.66%	2 Missing ⚠️
...ides/api/service/connectors/rds_mysql_connector.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5380      +/-   ##
==========================================
- Coverage   85.57%   85.48%   -0.10%     
==========================================
  Files         379      382       +3     
  Lines       23985    24067      +82     
  Branches     2623     2624       +1     
==========================================
+ Hits        20526    20573      +47     
- Misses       2907     2942      +35     
  Partials      552      552

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

clients/admin-ui/src/features/integrations/integration-type-info/rdsPostgresInfo.tsx

…fo/rdsPostgresInfo.tsx

CHANGELOG.md

erosselli

nice work! I left some questions, mainly for my own understanding I think.

clients/admin-ui/src/features/integrations/integration-type-info/rdsPostgresInfo.tsx

src/fides/api/schemas/connection_configuration/connection_secrets_rds_mysql.py

erosselli · 2024-10-18T20:04:30Z

src/fides/api/service/connectors/rds_postgres_connector.py

+        input_data: Dict[str, List[Any]],
+    ) -> List[Row]:
+        """DSR execution not yet supported for RDS Postgres"""
+        return []


I think we should probably raise a NotImplemented error here right? we don't w ant our code calling these methods and making it seem like it's actually doing things? or am I missing something?

I think that could be a possibility, Adam did the same as I did here on the S3Connector, should we continue the pattern?

honestly I'm not too sure if raising NotImplemented will make it so that DSRs fail when the connector is configured -- let's keep existing behavior for now and maybe discuss this more thoroughly another time

erosselli · 2024-10-18T20:06:11Z

tests/ops/integration_tests/test_connection_configuration_integration.py

+        rds_postgres_connection_config.secrets["aws_secret_access_key"] = "bad_key"
+        rds_postgres_connection_config.save(db)
+        connector = get_connector(rds_postgres_connection_config)
+        with pytest.raises(ConnectionException):
+            connector.test_connection()


a bit of a nitpick, but maybe we can move this to a separate test ? e.g test_connection_with_bad_key or something, I find it a bit easier to read / maintain when each test only tests for one thing

…fo/rdsPostgresInfo.tsx Co-authored-by: erosselli <[email protected]>

This reverts commit 1467709.

erosselli

Approving! I just left a small suggestion

erosselli · 2024-10-21T14:55:20Z

tests/ops/integration_tests/test_connection_configuration_integration.py

+        connector = get_connector(rds_postgres_connection_config)
+        assert connector.test_connection() == ConnectionTestStatus.succeeded
+
+    def test_test_wrong_connection(


Suggested change

def test_test_wrong_connection(

def test_wrong_connection_secrets(

cypress · 2024-10-21T15:13:42Z

fides Run #10522

Run Properties: Passed #10522 • 7247873f8b: HJ-5 - Added new RDS Postgres Connector (#5380)

Project	`fides`
Run status	`Passed #10522`
Run duration	`00m 38s`
Commit	`7247873f8b: HJ-5 - Added new RDS Postgres Connector (#5380)`
Committer	`Andres Torres`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`4`
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

HJ-5 - Added new RDS Postgres Connector

1811d2a

andres-torres-marroquin commented Oct 15, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

andres-torres-marroquin added 2 commits October 15, 2024 08:25

HJ-5 - Add RDS Postgres to ConnectorType

d67fd2c

Merge branch 'main' into andres/HJ-5

3fddd8a

vercel bot deployed to Preview October 15, 2024 14:54 View deployment

QA

fe3b745

vercel bot had a problem deploying to Preview October 15, 2024 15:22 Failure

QA

06b8bcb

vercel bot deployed to Preview October 15, 2024 16:08 View deployment

QA

456a9fd

andres-torres-marroquin commented Oct 15, 2024

View reviewed changes

src/fides/api/alembic/migrations/versions/5dafbc1818ae_add_rds_postgres_to_connector_type.py Outdated Show resolved Hide resolved

andres-torres-marroquin added 6 commits October 15, 2024 11:13

Making connect_args configurable

5dea41e

QA

183a5ca

Update CHANGELOG.md

0ac54dc

Merge branch 'main' into andres/HJ-5

c4dd561

QA

cd5106a

QA

b9c44ec

vercel bot deployed to Preview October 15, 2024 20:20 View deployment

andres-torres-marroquin commented Oct 16, 2024

View reviewed changes

clients/admin-ui/src/features/integrations/integration-type-info/rdsPostgresInfo.tsx Show resolved Hide resolved

andres-torres-marroquin added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Oct 16, 2024

andres-torres-marroquin added 3 commits October 16, 2024 10:42

QA

362201c

Merge branch 'main' into andres/HJ-5

5e84620

Update clients/admin-ui/src/features/integrations/integration-type-in…

0c1f7c1

…fo/rdsPostgresInfo.tsx

andres-torres-marroquin marked this pull request as ready for review October 16, 2024 17:00

andres-torres-marroquin commented Oct 16, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

andres-torres-marroquin added 2 commits October 16, 2024 11:00

Update CHANGELOG.md

fb96799

QA

7c981a6

andres-torres-marroquin requested a review from erosselli October 18, 2024 17:12

Merge branch 'main' into andres/HJ-5

c549ec1

vercel bot deployed to Preview October 18, 2024 17:15 View deployment

erosselli reviewed Oct 18, 2024

View reviewed changes

andres-torres-marroquin and others added 2 commits October 21, 2024 06:53

Merge branch 'main' into andres/HJ-5

031a89e

Update clients/admin-ui/src/features/integrations/integration-type-in…

a3dd7c2

…fo/rdsPostgresInfo.tsx Co-authored-by: erosselli <[email protected]>

vercel bot deployed to Preview October 21, 2024 13:04 View deployment

andres-torres-marroquin added 4 commits October 21, 2024 07:22

QA

1467709

Revert "QA"

5af3bd7

This reverts commit 1467709.

QA

b9352fe

QA

8e327f4

vercel bot deployed to Preview October 21, 2024 14:03 View deployment

erosselli approved these changes Oct 21, 2024

View reviewed changes

andres-torres-marroquin merged commit 7247873 into main Oct 21, 2024
40 of 46 checks passed

andres-torres-marroquin deleted the andres/HJ-5 branch October 21, 2024 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HJ-5 - Added new RDS Postgres Connector #5380

HJ-5 - Added new RDS Postgres Connector #5380

andres-torres-marroquin commented Oct 15, 2024

vercel bot commented Oct 15, 2024 •

edited

Loading

cypress bot commented Oct 15, 2024 •

edited

Loading

codecov bot commented Oct 15, 2024 •

edited

Loading

erosselli left a comment

erosselli Oct 18, 2024

andres-torres-marroquin Oct 21, 2024

erosselli Oct 21, 2024

erosselli Oct 18, 2024

erosselli left a comment

erosselli Oct 21, 2024

cypress bot commented Oct 21, 2024

	def test_test_wrong_connection(
	def test_wrong_connection_secrets(

HJ-5 - Added new RDS Postgres Connector #5380

HJ-5 - Added new RDS Postgres Connector #5380

Conversation

andres-torres-marroquin commented Oct 15, 2024

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Oct 15, 2024 • edited Loading

cypress bot commented Oct 15, 2024 • edited Loading

fides Run #10521

codecov bot commented Oct 15, 2024 • edited Loading

Codecov Report

erosselli left a comment

Choose a reason for hiding this comment

erosselli Oct 18, 2024

Choose a reason for hiding this comment

andres-torres-marroquin Oct 21, 2024

Choose a reason for hiding this comment

erosselli Oct 21, 2024

Choose a reason for hiding this comment

erosselli Oct 18, 2024

Choose a reason for hiding this comment

erosselli left a comment

Choose a reason for hiding this comment

erosselli Oct 21, 2024

Choose a reason for hiding this comment

cypress bot commented Oct 21, 2024

fides Run #10522

vercel bot commented Oct 15, 2024 •

edited

Loading

cypress bot commented Oct 15, 2024 •

edited

Loading

codecov bot commented Oct 15, 2024 •

edited

Loading