feat(integrations): add Google Cloud SQL to @deepnote/database-integrations#398
feat(integrations): add Google Cloud SQL to @deepnote/database-integrations#398sLightlyDev wants to merge 3 commits into
Conversation
…ations Registers 'cloud-sql' as a new DatabaseIntegrationType. Adds: - metadata schema (service_account: string) - config union member in databaseIntegrationConfigSchema - secret-field-paths entry (service_account is a secret) - env-vars case returning null (Cloud SQL uses the cloud-sql-python-connector, not a plain SQLAlchemy URL) - CLI prompts for add/edit commands - metadata schema unit tests Preceded by vscode-deepnote#406 which includes a local shim for E2E verification; that shim can be removed once this package publishes a new version. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces end-to-end support for the cloud-sql database integration type. It adds cloud-sql to runtime type lists, defines a metadata schema requiring a service account JSON, registers secret field paths, wires a CLI prompt for service_account into add/edit commands, returns null for SQLAlchemy input for cloud-sql, and adds unit and integration tests covering schema and CLI flows. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #398 +/- ##
==========================================
+ Coverage 83.93% 83.95% +0.02%
==========================================
Files 145 146 +1
Lines 8018 8029 +11
Branches 2165 2230 +65
==========================================
+ Hits 6730 6741 +11
Misses 1287 1287
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- env-vars: assert cloud-sql generates no SQLAlchemy var and no errors - CLI add-integration: cloud-sql happy path (name + service account) - CLI edit-integration: cloud-sql keep-defaults and update flows Covers the new lines flagged by codecov/patch on #398. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/cli/src/commands/integrations/tests/add-integration.cloud-sql.test.ts`:
- Line 1: The test file add-integration.cloud-sql.test.ts is misplaced under an
integrations/tests directory; move this .test.ts file to be colocated beside its
source command file (the add-integration.cloud-sql command implementation) so it
follows the repo rule of placing "*.test.ts" next to the source, update any
relative imports inside the test (e.g., the crypto import or local module
imports) so paths resolve after the move, and run the test suite to confirm the
test is discovered and passes.
In
`@packages/cli/src/commands/integrations/tests/edit-integration.cloud-sql.test.ts`:
- Around line 80-114: The test for editIntegration updates the service_account
but never asserts the .env was written; after awaiting promise in the 'updates
the name and service account' test, read envFilePath (the same variable used
earlier) and assert its contents include the updated key
CS_ID_001__SERVICE_ACCOUNT with value new-service-account-data (or the expected
updated secret string) so the test verifies the secret-write to .env; update the
test that calls editIntegration to include this .env read+assert using
EXISTING_YAML/CS_ID_001__SERVICE_ACCOUNT references to locate the correct
integration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb1f0ca2-09a8-4cf2-a322-69cb1f49c0c8
📒 Files selected for processing (12)
packages/cli/src/commands/integrations/add-integration.tspackages/cli/src/commands/integrations/edit-integration.tspackages/cli/src/commands/integrations/integrations-prompts/cloud-sql.tspackages/cli/src/commands/integrations/tests/add-integration.cloud-sql.test.tspackages/cli/src/commands/integrations/tests/edit-integration.cloud-sql.test.tspackages/database-integrations/src/database-integration-config.tspackages/database-integrations/src/database-integration-env-vars.test.tspackages/database-integrations/src/database-integration-env-vars.tspackages/database-integrations/src/database-integration-metadata-schemas.test.tspackages/database-integrations/src/database-integration-metadata-schemas.tspackages/database-integrations/src/database-integration-types.tspackages/database-integrations/src/secret-field-paths.ts
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
|
Re: test file placement — The Re: missing .env assertion in the update test — Good catch. Added a |
|
@coderabbitai approve |
|
✅ Action performedComments resolved and changes approved. |
|
superseded by #403 |
…ations (#403) * feat(integrations): add Google Cloud SQL to @deepnote/database-integrations Registers 'cloud-sql' as a new DatabaseIntegrationType. Adds: - metadata schema (service_account: string) - config union member in databaseIntegrationConfigSchema - secret-field-paths entry (service_account is a secret) - env-vars case returning null (Cloud SQL uses the cloud-sql-python-connector, not a plain SQLAlchemy URL) - CLI prompts for add/edit commands - metadata schema unit tests Preceded by vscode-deepnote#406 which includes a local shim for E2E verification; that shim can be removed once this package publishes a new version. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test(integrations): add Cloud SQL test coverage - env-vars: assert cloud-sql generates no SQLAlchemy var and no errors - CLI add-integration: cloud-sql happy path (name + service account) - CLI edit-integration: cloud-sql keep-defaults and update flows Covers the new lines flagged by codecov/patch on #398. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> * test(integrations): assert .env is updated in cloud-sql edit test Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --------- Co-authored-by: Marko Paleka <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]>
What / Why
Adds
cloud-sqlas a supportedDatabaseIntegrationTypein the@deepnote/database-integrationspackage.This is the upstream schema extraction for the Cloud SQL integration —
vscode-deepnote#406added a local shim with the comment "Pending addition to @deepnote/database-integrations; remove once the package includes 'cloud-sql'." This PR fulfils that.Changes
database-integration-types.ts—'cloud-sql'added tosqlIntegrationTypesdatabase-integration-metadata-schemas.ts—cloudSqlMetadataSchema(service_account: z.string(), consistent with Spanner/BigQuery convention)database-integration-config.ts— config union member addedsecret-field-paths.ts—'cloud-sql': ['service_account'](service account JSON is a secret)database-integration-env-vars.ts—case 'cloud-sql': return null(Cloud SQL connects via the cloud-sql-python-connector, not a plain SQLAlchemy URL — satisfies the exhaustiveness check)integrations-prompts/cloud-sql.ts— new CLI prompt (singleservice_accountfield, modelled on Spanner)add-integration.ts+edit-integration.ts— import + switch case wired indatabase-integration-metadata-schemas.test.ts— 2 new test casesHow to review
Start with
database-integration-types.ts(one-liner), thendatabase-integration-metadata-schemas.ts, thensecret-field-paths.ts, thendatabase-integration-env-vars.ts. The CLI files (add-integration.ts,edit-integration.ts,cloud-sql.ts) follow the exact same pattern as Spanner.Testing
tsc --noEmitacross all packages)@deepnote/database-integrations; all pass in@deepnote/cliwhen run in isolationpromptForFieldsCloudSql): not manually run, but covered by typecheck — follows the Spanner pattern exactlyadd-integrationtests are intermittently flaky under parallel local execution (unrelated to this diff); CI is unaffectedRisks
return nullin env-vars is intentional — Cloud SQL uses the cloud-sql-python-connector rather than a SQLAlchemy URL (same reason Spanner has its own handler). A full SQLAlchemy/connector implementation would be a follow-up.Next step
Once this merges and
@deepnote/database-integrationspublishes a new version,vscode-deepnote#406can be updated to remove the local shim and consume the package directly.Summary by CodeRabbit
New Features
Behavior Changes
Tests