feat(integrations): Add Google Cloud SQL database integration#406
Draft
sLightlyDev wants to merge 2 commits into
Draft
feat(integrations): Add Google Cloud SQL database integration#406sLightlyDev wants to merge 2 commits into
sLightlyDev wants to merge 2 commits into
Conversation
Adds Cloud SQL to the integration picker with a name + service account JSON form (validated), modeled on the Spanner integration. Includes a temporary local shim for the cloud-sql config type until it is published upstream in @deepnote/database-integrations.
Contributor
|
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 (3)
📝 WalkthroughWalkthroughAdds Google Cloud SQL as a configurable integration: new type exports and unions, Zod metadata schema for service-account JSON, detection/manager updates to validate configurable types, env-var generation that skips pending integrations, localized UI strings and icons, and a new CloudSQLForm React component that validates service-account JSON and emits a CloudSqlIntegrationConfig on save. Sequence DiagramsequenceDiagram
participant User as User
participant Selector as IntegrationTypeSelector
participant Form as CloudSQLForm
participant Backend as IntegrationManager
participant Storage as IntegrationStorage
User->>Selector: choose 'cloud-sql'
Selector->>Form: render CloudSQLForm
User->>Form: enter name + service account JSON
Form->>Form: validate JSON on keystroke
User->>Form: click Save
Form->>Form: require non-empty valid JSON
Form->>Backend: onSave(CloudSqlIntegrationConfig)
Backend->>Backend: validate type ∈ allDatabaseIntegrationTypes
Backend->>Storage: persist config
Storage->>Storage: validate with cloudSqlMetadataSchema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@src/platform/notebooks/deepnote/integrationStorage.ts`:
- Around line 9-15: The import statements mix third-party and local modules;
reorder them so all third-party imports (e.g., z from 'zod' and
databaseMetadataSchemasByType from '`@deepnote/database-integrations`') appear
first, followed by local/project imports (e.g., allDatabaseIntegrationTypes,
ConfigurableDatabaseIntegrationConfig, DATAFRAME_SQL_INTEGRATION_ID from
'./integrationTypes'); update the import block to group external packages before
the relative './integrationTypes' import while keeping the same imported symbols
and names.
- Around line 18-35: The refine on cloudSqlMetadataSchema.service_account uses
the old Zod 3 `{ message: '...' }` option; update the second argument to use Zod
4's unified `error` option (e.g., replace the `{ message: 'Invalid JSON format'
}` object with an `{ error: { message: 'Invalid JSON format' } }` payload) so
the call to `.refine(...)` on the service_account validator conforms to Zod 4
error customization.
In `@src/platform/notebooks/deepnote/integrationTypes.ts`:
- Around line 85-88: The array allDatabaseIntegrationTypes is declared as
readonly string[] which widens and loses literal union types; change its
declaration to preserve literals by using a const assertion or a typed tuple
spread (e.g. readonly [...typeof databaseIntegrationTypes, ...typeof
PENDING_INTEGRATION_TYPES]) so TypeScript retains the exact string literals,
then export a union type like AllDatabaseIntegrationType = (typeof
allDatabaseIntegrationTypes)[number]; update references that currently cast
(e.g., uses in integrationManager and sqlCellStatusBarProvider) to use the new
union type instead of manual casts.
In
`@src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts`:
- Around line 129-149: Remove the redundant local alias `pendingTypes` and use
the module-level constant `PENDING_INTEGRATION_TYPES` directly: delete the
`const pendingTypes: readonly string[] = PENDING_INTEGRATION_TYPES;` declaration
and change the check `if (pendingTypes.includes(config.type))` to `if
(PENDING_INTEGRATION_TYPES.includes(config.type))` inside the loop in
SqlIntegrationEnvironmentVariablesProvider; leave the other logic (the
`isFederatedAuthMetadata` check and pushing into `projectIntegrationConfigs`)
unchanged.
In `@src/webviews/webview-side/integrations/CloudSQLForm.tsx`:
- Around line 72-95: The handler validates a trimmed service account but calls
onSave with the original untrimmed string; update the pendingConfig so the saved
value matches validation by replacing pendingConfig.metadata.service_account
with trimmedServiceAccount before calling onSave — either mutate a cloned object
(e.g., const next = { ...pendingConfig, metadata: { ...pendingConfig.metadata,
service_account: trimmedServiceAccount } } and pass next to onSave) or call the
appropriate setter (setPendingConfig) to persist the trimmed value, then call
onSave with that trimmed-version object from handleSubmit.
- Line 34: CloudSQLForm currently calls structuredClone(existingConfig)
unguarded; wrap that use with a runtime check like typeof structuredClone !==
'undefined' and fall back to a safe alternative (e.g.,
JSON.parse(JSON.stringify(existingConfig)) or a repo shared helper) so the
webview doesn't crash in environments without structuredClone; update the code
in CloudSQLForm (the place where structuredClone(existingConfig) is called) to
perform the typeof structuredClone check and use the fallback when undefined.
🪄 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: d6e9dfba-2a05-4f1c-ad6a-f5d23e634d5a
⛔ Files ignored due to path filters (1)
src/webviews/webview-side/integrations/icons/cloud-sql.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
src/messageTypes.tssrc/notebooks/deepnote/integrations/integrationDetector.tssrc/notebooks/deepnote/integrations/integrationManager.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/notebooks/deepnote/sqlCellStatusBarProvider.tssrc/platform/common/utils/localize.tssrc/platform/notebooks/deepnote/integrationStorage.tssrc/platform/notebooks/deepnote/integrationTypes.tssrc/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.tssrc/webviews/webview-side/integrations/CloudSQLForm.tsxsrc/webviews/webview-side/integrations/ConfigurationForm.tsxsrc/webviews/webview-side/integrations/IntegrationTypeSelector.tsxsrc/webviews/webview-side/integrations/integrationUtils.tssrc/webviews/webview-side/integrations/types.ts
Comment on lines
+18
to
+35
| const cloudSqlMetadataSchema = z.object({ | ||
| service_account: z | ||
| .string() | ||
| .trim() | ||
| .min(1) | ||
| .refine( | ||
| (value) => { | ||
| try { | ||
| JSON.parse(value); | ||
|
|
||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }, | ||
| { message: 'Invalid JSON format' } | ||
| ) | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider Zod 4 unified error customization.
The .refine() uses { message: 'Invalid JSON format' } (Zod 3 style). Zod 4 introduces unified error customization via the error parameter for consistency across validators.
♻️ Optional Zod 4 refactor
const cloudSqlMetadataSchema = z.object({
service_account: z
.string()
.trim()
.min(1)
.refine(
(value) => {
try {
JSON.parse(value);
-
return true;
} catch {
return false;
}
},
- { message: 'Invalid JSON format' }
+ { error: () => 'Invalid JSON format' }
)
});🤖 Prompt for 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.
In `@src/platform/notebooks/deepnote/integrationStorage.ts` around lines 18 - 35,
The refine on cloudSqlMetadataSchema.service_account uses the old Zod 3 `{
message: '...' }` option; update the second argument to use Zod 4's unified
`error` option (e.g., replace the `{ message: 'Invalid JSON format' }` object
with an `{ error: { message: 'Invalid JSON format' } }` payload) so the call to
`.refine(...)` on the service_account validator conforms to Zod 4 error
customization.
Comment on lines
+85
to
+88
| export const allDatabaseIntegrationTypes: readonly string[] = [ | ||
| ...databaseIntegrationTypes, | ||
| ...PENDING_INTEGRATION_TYPES | ||
| ]; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Type widening loses literal types.
allDatabaseIntegrationTypes is typed as readonly string[], which discards the literal union from the concatenation. TypeScript won't narrow types after .includes() checks, forcing explicit casts downstream (e.g., line 165 in integrationManager.ts, line 375 in sqlCellStatusBarProvider.ts).
Consider typing as readonly [...typeof databaseIntegrationTypes, ...typeof PENDING_INTEGRATION_TYPES] or use a const assertion to preserve literals:
-export const allDatabaseIntegrationTypes: readonly string[] = [
+export const allDatabaseIntegrationTypes = [
...databaseIntegrationTypes,
...PENDING_INTEGRATION_TYPES
-];
+] as const;Then export a union type:
export type AllDatabaseIntegrationType = (typeof allDatabaseIntegrationTypes)[number];This enables type narrowing without casts.
🤖 Prompt for 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.
In `@src/platform/notebooks/deepnote/integrationTypes.ts` around lines 85 - 88,
The array allDatabaseIntegrationTypes is declared as readonly string[] which
widens and loses literal union types; change its declaration to preserve
literals by using a const assertion or a typed tuple spread (e.g. readonly
[...typeof databaseIntegrationTypes, ...typeof PENDING_INTEGRATION_TYPES]) so
TypeScript retains the exact string literals, then export a union type like
AllDatabaseIntegrationType = (typeof allDatabaseIntegrationTypes)[number];
update references that currently cast (e.g., uses in integrationManager and
sqlCellStatusBarProvider) to use the new union type instead of manual casts.
- Move zod and @deepnote/database-integrations imports to third-party group - Use Zod 4 string-param API for .refine() (deprecated message object) - Remove redundant pendingTypes alias; inline cast on PENDING_INTEGRATION_TYPES - Add structuredClone fallback in CloudSQLForm for older webview environments - Save trimmed service_account value on form submit instead of raw input Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #406 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Google Cloud SQL as a database integration in the integration picker.
Implementation note
cloud-sqlis not yet published in@deepnote/database-integrations, so this change includes a small, clearly-marked temporary local shim for the config type (and excludes it from environment-variable generation) so it ships self-contained. Once the type is published upstream, the shim can be removed and the dependency bumped as a follow-up.Testing
No automated/staging test exists for the extension UI, so this was verified locally in the Extension Development Host (
F5):.deepnoteproject → Deepnote: Manage Integrations → Add New Integration.Cmd+R) → integration persists.Existing unit suite passes with no regressions (
0failing).Summary by CodeRabbit