ScyllaDB Connector #4946

pattisdr · 2024-06-03T22:42:29Z

Closes #PROD-2148

❗ Contains connection type migration. Bump downrev before merge.
👉 Supports https://github.com/ethyca/fidesplus/pull/1447

Description Of Changes

Adds the infrastructure to run a local scylla db instance in Docker, and a simple integration to connect to it. Access and erasure requests are not yet supported for scylla. This first step is just supporting scylla detection and discovery in Fidesplus.

Code Changes

Installs scylla-driver
Adds a new scylla ConnectionType
Adds support for bringing up a test scylla db in Docker: Adds a new docker-compose integration file for scylla db which uses the bitnami image, which allows you to put initialization scripts in the docker entrypoint directory, which doesn't seem to be supported in the regular cassandra/scylla db images.
Adds sample cql file to populate database with fake data - creates two keyspaces, a couple of tables, and populates
Adds the start of a Scylla Connector. Retrieve data and mask data are not currently supported, just testing a connection.

Steps to Confirm

Run nox -s dev -- shell scylladb
In fides/clients, turbo run dev
Go to System > Add system, add name: Scylla, click save
Click on integration tab, search for scylla:
Add creds (these are not secret, these are test credentials to the scylla instance you're running in your own docker container locally) - scylla_user,scylla_pass
Click test connection, verify this is green

Pre-Merge Checklist

vercel · 2024-06-03T22:42:32Z

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	⬜️ Ignored (Inspect)	Visit Preview		Jun 13, 2024 4:12pm

cypress · 2024-06-03T22:54:22Z

Passing run #8300 ↗︎

0	4	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

Merge `d35eed5` into `4fde412`...
Project: fides	Commit: `301034403e ℹ️`
Status: Passed	Duration: 00:34 💡
Started: Jun 13, 2024 4:24 PM	Ended: Jun 13, 2024 4:24 PM

Review all test suite changes for PR #4946 ↗︎

- Still using default username/password. - Update replication_factor to 3 instead of 1 for example keyspaces - Increase number of vendor records in example data - Bump downrev - Make keyspace optional for Integration - Throw some more specific errors when testing the scylla connection around keyspace issues and password issues.

…s in the docker entrypoint directory. - Write tests for scylla connector > test connection

codecov · 2024-06-12T13:47:16Z

Codecov Report

Attention: Patch coverage is 87.50000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 86.59%. Comparing base (0f4ea85) to head (d35eed5).
Report is 1 commits behind head on main.

Files	Patch %	Lines
...c/fides/api/service/connectors/scylla_connector.py	83.33%	7 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4946      +/-   ##
==========================================
- Coverage   86.82%   86.59%   -0.24%     
==========================================
  Files         346      348       +2     
  Lines       21481    21611     +130     
  Branches     2843     2867      +24     
==========================================
+ Hits        18650    18713      +63     
- Misses       2328     2394      +66     
- Partials      503      504       +1

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

docker/docker-compose.integration-scylladb.yml

src/fides/api/alembic/migrations/versions/3304082a6cee_scylla_connection_type.py

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

adamsachs

@pattisdr this side all looks solid to me! very thorough, nice job with everything.

as you said, let's just keep an eye on merge conflicts since we've got similar codepaths being updated from a few spots - thanks for calling that out 👍

noxfiles/run_infrastructure.py

src/fides/api/alembic/migrations/versions/3304082a6cee_scylla_connection_type.py

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

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

adamsachs · 2024-06-12T23:38:59Z

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

+        request_task: RequestTask,
+        input_data: Dict[str, List[Any]],
+    ) -> List[Row]:
+        """Retrieve scylla data - not yet implemented"""


nit: worth raising a NotImplementedError here? that always feels like the most proper thing, just to make it transparent to anyone who somehow hits the codepath at runtime without reading the codebase, but maybe i'm overthinking it :)

now i'm wondering whether raising the error may unexpectedly impact other DSR workflows and i shouldn't be doing that... 🤔

If there are datasets for the connection config this could get picked up in a DSR I believe!

yep confirmed a NotImplementedError will break DSR's if a dataset is also attached to the ConnectionConfig .

tests/ops/api/v1/endpoints/test_connection_template_endpoints.py

cypress · 2024-06-13T17:25:19Z

Passing run #8304 ↗︎

0	4	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

ScyllaDB Connector (#4946)
Project: fides	Commit: `958ff68ec0`
Status: Passed	Duration: 00:35 💡
Started: Jun 13, 2024 5:24 PM	Ended: Jun 13, 2024 5:25 PM

Review all test suite changes for PR #4946 ↗︎

First draft scylla connector

d413965

pattisdr changed the title ~~ScyllaDB Connector Draft~~ ScyllaDB Integration Draft Jun 3, 2024

pattisdr added 9 commits June 3, 2024 18:12

Add another keyspace

ffae8fb

Add to ValidTargets

9fcd343

Merge branch 'main' into PROD-2106_scylladb

74482a1

Add a base integration test for syclla db -

2c9ba3b

Merge branch 'main' into PROD-2106_scylladb

64649b9

Bump downrev

53096c0

Add scylla svg logo

359f17e

Use bitnami scylla db image instead which lets you put startup script…

9ac58a5

…s in the docker entrypoint directory. - Write tests for scylla connector > test connection

pattisdr marked this pull request as ready for review June 11, 2024 17:05

pattisdr changed the title ~~ScyllaDB Integration Draft~~ ScyllaDB Integration Starter Jun 11, 2024

pattisdr added 5 commits June 11, 2024 12:06

Merge branch 'main' into PROD-2106_scylladb

a1f65bb

Update changelog

ab1411a

Update test host

c99c485

Merge branch 'main' into PROD-2106_scylladb

6e150f5

Bump downrev

8529093

pattisdr changed the title ~~ScyllaDB Integration Starter~~ ScyllaDB Connector Jun 12, 2024

pattisdr commented Jun 12, 2024

View reviewed changes

docker/docker-compose.integration-scylladb.yml Show resolved Hide resolved

src/fides/api/alembic/migrations/versions/3304082a6cee_scylla_connection_type.py Show resolved Hide resolved

src/fides/api/schemas/connection_configuration/connection_secrets_scylla.py Show resolved Hide resolved

adamsachs approved these changes Jun 12, 2024

View reviewed changes

pattisdr added 2 commits June 13, 2024 10:30

Merge branch 'main' into PROD-2106_scylladb

436faab

Bump downrev

d35eed5

pattisdr merged commit 958ff68 into main Jun 13, 2024
47 of 48 checks passed

pattisdr deleted the PROD-2106_scylladb branch June 13, 2024 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ScyllaDB Connector #4946

ScyllaDB Connector #4946

pattisdr commented Jun 3, 2024 •

edited

Loading

vercel bot commented Jun 3, 2024 •

edited

Loading

cypress bot commented Jun 3, 2024 •

edited

Loading

codecov bot commented Jun 12, 2024 •

edited

Loading

adamsachs left a comment

adamsachs Jun 12, 2024

adamsachs Jun 12, 2024

pattisdr Jun 13, 2024

pattisdr Jun 13, 2024

cypress bot commented Jun 13, 2024

ScyllaDB Connector #4946

ScyllaDB Connector #4946

Conversation

pattisdr commented Jun 3, 2024 • edited Loading

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Jun 3, 2024 • edited Loading

cypress bot commented Jun 3, 2024 • edited Loading

Passing run #8300 ↗︎

Review all test suite changes for PR #4946 ↗︎

codecov bot commented Jun 12, 2024 • edited Loading

Codecov Report

adamsachs left a comment

Choose a reason for hiding this comment

adamsachs Jun 12, 2024

Choose a reason for hiding this comment

adamsachs Jun 12, 2024

Choose a reason for hiding this comment

pattisdr Jun 13, 2024

Choose a reason for hiding this comment

pattisdr Jun 13, 2024

Choose a reason for hiding this comment

cypress bot commented Jun 13, 2024

Passing run #8304 ↗︎

Review all test suite changes for PR #4946 ↗︎

pattisdr commented Jun 3, 2024 •

edited

Loading

vercel bot commented Jun 3, 2024 •

edited

Loading

cypress bot commented Jun 3, 2024 •

edited

Loading

codecov bot commented Jun 12, 2024 •

edited

Loading