feat: support for all database integrations #160
Conversation
📝 WalkthroughWalkthroughThis PR expands supported database integrations (≈18 types), adds per-type metadata schemas and React configuration forms, renames PostgreSQL/SQL-like field Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Webview as Integration Webview
participant Detector as Integration Detector
participant Router as ConfigurationForm
participant Form as Type-specific Form
participant Storage as IntegrationStorage
Note right of Detector: Detector filters out 'pandas-dataframe' → Configurable types only
User->>Webview: Open integrations panel
Webview->>Detector: Request project integrations
Detector-->>Webview: IntegrationWithStatus[] (ConfigurableDatabaseIntegrationType)
Webview->>Router: Render list + localization (expanded types)
User->>Router: Select integration type (e.g., mysql)
Router->>Form: Render corresponding Form component
alt type == pandas-dataframe
Router-->>User: Skip form / not shown
else
User->>Form: Fill & submit
Form->>Storage: save(ConfigurableDatabaseIntegrationConfig)
Storage-->>Webview: Saved / cached
Webview-->>User: Save confirmation
end
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/!(*.node|*.web).ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/platform/**/*.ts📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #160 +/- ##
======================================
Coverage 73% 73%
======================================
Files 552 552
Lines 44283 44488 +205
Branches 5313 5314 +1
======================================
+ Hits 32517 32708 +191
- Misses 10003 10017 +14
Partials 1763 1763
🚀 New features to boost your workflow:
|
jankuca
force-pushed
the
jk/all-db-integrations
branch
from
c4e789b to
cf5262d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts (1)
1-1: Missing Microsoft copyright header.Add the required copyright header per coding guidelines.
As per coding guidelines.
src/notebooks/deepnote/integrations/integrationManager.ts (1)
164-174: Don't bail out when type is pandas-dataframe.Returning here aborts Manage Integrations entirely, so other integrations never render when this guard trips. Just skip adding the internal entry and carry on.
- if (integrationType === 'pandas-dataframe') { - logger.debug(`IntegrationManager: Skipping internal DuckDB integration ${selectedIntegrationId}`); - return; - } - - integrations.set(selectedIntegrationId, { - config: config || null, - status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected, - integrationName, - integrationType - }); + if (integrationType === 'pandas-dataframe') { + logger.debug(`IntegrationManager: Skipping internal DuckDB integration ${selectedIntegrationId}`); + } else { + integrations.set(selectedIntegrationId, { + config: config || null, + status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected, + integrationName, + integrationType + }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (33)
INTEGRATIONS_CREDENTIALS.md(6 hunks)cspell.json(3 hunks)src/messageTypes.ts(2 hunks)src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts(5 hunks)src/notebooks/deepnote/integrations/integrationDetector.ts(3 hunks)src/notebooks/deepnote/integrations/integrationManager.ts(1 hunks)src/notebooks/deepnote/integrations/integrationWebview.ts(5 hunks)src/notebooks/deepnote/sqlCellStatusBarProvider.ts(3 hunks)src/notebooks/types.ts(2 hunks)src/platform/common/utils/localize.ts(2 hunks)src/platform/notebooks/deepnote/integrationStorage.ts(6 hunks)src/platform/notebooks/deepnote/integrationStorage.unit.test.ts(2 hunks)src/platform/notebooks/deepnote/integrationTypes.ts(3 hunks)src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts(2 hunks)src/platform/notebooks/deepnote/types.ts(3 hunks)src/webviews/webview-side/integrations/AlloyDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/AthenaForm.tsx(1 hunks)src/webviews/webview-side/integrations/ClickHouseForm.tsx(1 hunks)src/webviews/webview-side/integrations/ConfigurationForm.tsx(2 hunks)src/webviews/webview-side/integrations/DatabricksForm.tsx(1 hunks)src/webviews/webview-side/integrations/DremioForm.tsx(1 hunks)src/webviews/webview-side/integrations/IntegrationItem.tsx(1 hunks)src/webviews/webview-side/integrations/IntegrationPanel.tsx(3 hunks)src/webviews/webview-side/integrations/MariaDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MaterializeForm.tsx(1 hunks)src/webviews/webview-side/integrations/MindsDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MongoDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MySQLForm.tsx(1 hunks)src/webviews/webview-side/integrations/RedshiftForm.tsx(1 hunks)src/webviews/webview-side/integrations/SQLServerForm.tsx(1 hunks)src/webviews/webview-side/integrations/SpannerForm.tsx(1 hunks)src/webviews/webview-side/integrations/TrinoForm.tsx(1 hunks)src/webviews/webview-side/integrations/types.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/types.tssrc/platform/notebooks/deepnote/integrationStorage.unit.test.tssrc/platform/notebooks/deepnote/integrationTypes.tssrc/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.tssrc/notebooks/deepnote/integrations/integrationManager.tssrc/webviews/webview-side/integrations/types.tssrc/notebooks/deepnote/sqlCellStatusBarProvider.tssrc/platform/notebooks/deepnote/types.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/platform/notebooks/deepnote/integrationStorage.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/notebooks/deepnote/integrations/integrationDetector.tssrc/messageTypes.tssrc/platform/common/utils/localize.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
Files:
src/notebooks/types.tssrc/platform/notebooks/deepnote/integrationStorage.unit.test.tssrc/platform/notebooks/deepnote/integrationTypes.tssrc/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.tssrc/webviews/webview-side/integrations/RedshiftForm.tsxsrc/webviews/webview-side/integrations/AthenaForm.tsxsrc/webviews/webview-side/integrations/SQLServerForm.tsxsrc/notebooks/deepnote/integrations/integrationManager.tssrc/webviews/webview-side/integrations/types.tssrc/webviews/webview-side/integrations/ClickHouseForm.tsxsrc/webviews/webview-side/integrations/AlloyDBForm.tsxsrc/webviews/webview-side/integrations/IntegrationItem.tsxsrc/notebooks/deepnote/sqlCellStatusBarProvider.tssrc/webviews/webview-side/integrations/SpannerForm.tsxsrc/webviews/webview-side/integrations/DremioForm.tsxsrc/platform/notebooks/deepnote/types.tssrc/webviews/webview-side/integrations/MindsDBForm.tsxsrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.tssrc/webviews/webview-side/integrations/TrinoForm.tsxsrc/webviews/webview-side/integrations/MaterializeForm.tsxsrc/webviews/webview-side/integrations/MySQLForm.tsxsrc/platform/notebooks/deepnote/integrationStorage.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/webviews/webview-side/integrations/MariaDBForm.tsxsrc/webviews/webview-side/integrations/IntegrationPanel.tsxsrc/webviews/webview-side/integrations/MongoDBForm.tsxsrc/notebooks/deepnote/integrations/integrationDetector.tssrc/webviews/webview-side/integrations/ConfigurationForm.tsxsrc/webviews/webview-side/integrations/DatabricksForm.tsxsrc/messageTypes.tssrc/platform/common/utils/localize.ts
**/*.unit.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place unit tests in files matching
*.unit.test.ts
Files:
src/platform/notebooks/deepnote/integrationStorage.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/notebooks/deepnote/integrationStorage.unit.test.tssrc/platform/notebooks/deepnote/integrationTypes.tssrc/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.tssrc/platform/notebooks/deepnote/types.tssrc/platform/notebooks/deepnote/integrationStorage.tssrc/platform/common/utils/localize.ts
**/*.{test,spec}.ts
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined
thenproperty to avoid hanging tests
Files:
src/platform/notebooks/deepnote/integrationStorage.unit.test.tssrc/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts
🧬 Code graph analysis (27)
src/notebooks/types.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
ConfigurableDatabaseIntegrationType(144-144)src/webviews/webview-side/integrations/types.ts (1)
ConfigurableDatabaseIntegrationType(3-3)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
src/webviews/webview-side/integrations/types.ts (2)
ConfigurableDatabaseIntegrationConfig(5-5)ConfigurableDatabaseIntegrationType(3-3)
src/webviews/webview-side/integrations/RedshiftForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/AthenaForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/SQLServerForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/notebooks/deepnote/integrations/integrationManager.ts (1)
src/platform/logging/index.ts (1)
logger(35-48)
src/webviews/webview-side/integrations/types.ts (1)
src/platform/notebooks/deepnote/integrationTypes.ts (3)
ConfigurableDatabaseIntegrationType(144-144)ConfigurableDatabaseIntegrationConfig(139-142)IntegrationWithStatus(158-170)
src/webviews/webview-side/integrations/ClickHouseForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/AlloyDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/IntegrationItem.tsx (4)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
IntegrationWithStatus(158-170)ConfigurableDatabaseIntegrationType(144-144)src/webviews/webview-side/integrations/types.ts (2)
IntegrationWithStatus(9-15)ConfigurableDatabaseIntegrationType(3-3)src/notebooks/deepnote/sqlCellStatusBarProvider.ts (1)
getIntegrationTypeLabel(441-480)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/notebooks/deepnote/sqlCellStatusBarProvider.ts (1)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
DATAFRAME_SQL_INTEGRATION_ID(5-5)ConfigurableDatabaseIntegrationType(144-144)
src/webviews/webview-side/integrations/SpannerForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/DremioForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/platform/notebooks/deepnote/types.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
ConfigurableDatabaseIntegrationConfig(139-142)src/webviews/webview-side/integrations/types.ts (1)
ConfigurableDatabaseIntegrationConfig(5-5)
src/webviews/webview-side/integrations/MindsDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts (1)
src/notebooks/types.ts (1)
ProjectIntegration(32-36)
src/webviews/webview-side/integrations/TrinoForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MaterializeForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MySQLForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/platform/notebooks/deepnote/integrationStorage.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
ConfigurableDatabaseIntegrationConfig(139-142)DATAFRAME_SQL_INTEGRATION_ID(5-5)src/webviews/webview-side/integrations/types.ts (1)
ConfigurableDatabaseIntegrationConfig(5-5)
src/notebooks/deepnote/integrations/integrationWebview.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
ConfigurableDatabaseIntegrationConfig(139-142)src/webviews/webview-side/integrations/types.ts (1)
ConfigurableDatabaseIntegrationConfig(5-5)
src/webviews/webview-side/integrations/MariaDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/IntegrationPanel.tsx (2)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
ConfigurableDatabaseIntegrationConfig(139-142)ConfigurableDatabaseIntegrationType(144-144)src/webviews/webview-side/integrations/types.ts (2)
ConfigurableDatabaseIntegrationConfig(5-5)ConfigurableDatabaseIntegrationType(3-3)
src/webviews/webview-side/integrations/MongoDBForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/notebooks/deepnote/integrations/integrationDetector.ts (2)
src/platform/notebooks/deepnote/integrationTypes.ts (1)
ConfigurableDatabaseIntegrationType(144-144)src/webviews/webview-side/integrations/types.ts (1)
ConfigurableDatabaseIntegrationType(3-3)
src/webviews/webview-side/integrations/ConfigurationForm.tsx (1)
src/webviews/webview-side/integrations/types.ts (2)
ConfigurableDatabaseIntegrationConfig(5-5)ConfigurableDatabaseIntegrationType(3-3)
src/webviews/webview-side/integrations/DatabricksForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
🔇 Additional comments (3)
cspell.json (1)
23-76: Dictionary entries correctly updated for expanded database integrations.All new terms (alloydb, clickhouse, dremio/Dremio, mindsdb, trino/Trino, venv/Venv) are properly added with consistent alphabetical ordering and appropriate case variants. Changes align with the PR's expanded database integration support.
src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts (2)
14-14: LGTM: Import needed for stronger typing.
91-105: No type incompatibility found.The array is typed as
DatabaseIntegrationConfig, not the restrictedConfigurableDatabaseIntegrationConfig. The'pandas-dataframe'type is a valid value forDatabaseIntegrationConfig, andintegrationStorage.save()explicitly guards against it. No changes needed.Likely an incorrect or invalid review comment.
jankuca
force-pushed
the
jk/all-db-integrations
branch
from
337565f to
3aca881
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
INTEGRATIONS_CREDENTIALS.md (1)
18-18: Verify package version consistency.A past review flagged version mismatch between this line (v1.1.0) and another reference to v1.1.1. Ensure all package version references are aligned.
src/webviews/webview-side/integrations/MySQLForm.tsx (1)
221-226: Verify CaCertificateFields prop requirements.Same concern as MindsDBForm:
CaCertificateFieldsreceives only partial props. Ensure the component interface makessslEnabled,caCertificateText,onSslEnabledChange, andonCaCertificateTextChangeoptional when the corresponding show flags are false.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
INTEGRATIONS_CREDENTIALS.md(7 hunks)src/messageTypes.ts(2 hunks)src/notebooks/deepnote/integrations/integrationManager.ts(1 hunks)src/notebooks/deepnote/integrations/integrationWebview.ts(5 hunks)src/platform/common/utils/localize.ts(2 hunks)src/webviews/webview-side/integrations/AlloyDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/CaCertificateFields.tsx(1 hunks)src/webviews/webview-side/integrations/ClickHouseForm.tsx(1 hunks)src/webviews/webview-side/integrations/DatabricksForm.tsx(1 hunks)src/webviews/webview-side/integrations/DremioForm.tsx(1 hunks)src/webviews/webview-side/integrations/MariaDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MaterializeForm.tsx(1 hunks)src/webviews/webview-side/integrations/MindsDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MongoDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MySQLForm.tsx(1 hunks)src/webviews/webview-side/integrations/PostgresForm.tsx(3 hunks)src/webviews/webview-side/integrations/RedshiftForm.tsx(1 hunks)src/webviews/webview-side/integrations/SQLServerForm.tsx(1 hunks)src/webviews/webview-side/integrations/SshOptionsFields.tsx(1 hunks)src/webviews/webview-side/integrations/TrinoForm.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/!(*.node|*.web).ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place shared cross-platform logic in common
.tsfiles (not suffixed with.nodeor.web)
Files:
src/notebooks/deepnote/integrations/integrationManager.tssrc/notebooks/deepnote/integrations/integrationWebview.tssrc/messageTypes.tssrc/platform/common/utils/localize.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
Files:
src/notebooks/deepnote/integrations/integrationManager.tssrc/webviews/webview-side/integrations/MariaDBForm.tsxsrc/webviews/webview-side/integrations/MindsDBForm.tsxsrc/webviews/webview-side/integrations/TrinoForm.tsxsrc/webviews/webview-side/integrations/MongoDBForm.tsxsrc/webviews/webview-side/integrations/DatabricksForm.tsxsrc/webviews/webview-side/integrations/MySQLForm.tsxsrc/webviews/webview-side/integrations/ClickHouseForm.tsxsrc/notebooks/deepnote/integrations/integrationWebview.tssrc/webviews/webview-side/integrations/SshOptionsFields.tsxsrc/webviews/webview-side/integrations/RedshiftForm.tsxsrc/webviews/webview-side/integrations/DremioForm.tsxsrc/webviews/webview-side/integrations/AlloyDBForm.tsxsrc/webviews/webview-side/integrations/PostgresForm.tsxsrc/webviews/webview-side/integrations/CaCertificateFields.tsxsrc/webviews/webview-side/integrations/SQLServerForm.tsxsrc/webviews/webview-side/integrations/MaterializeForm.tsxsrc/messageTypes.tssrc/platform/common/utils/localize.ts
src/platform/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/platform.instructions.md)
src/platform/**/*.ts: Use Inversify decorators for DI: annotate classes with @Injectable() and inject dependencies with @Inject(Token)
Use the centralized logger (import { logger } from '../platform/logging') instead of console.log for application logging
Files:
src/platform/common/utils/localize.ts
🧬 Code graph analysis (14)
src/notebooks/deepnote/integrations/integrationManager.ts (2)
src/platform/logging/index.ts (1)
logger(35-48)src/webviews/webview-side/integrations/types.ts (1)
IntegrationStatus(7-7)
src/webviews/webview-side/integrations/MariaDBForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MindsDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/TrinoForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MongoDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/DatabricksForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MySQLForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/ClickHouseForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/RedshiftForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/DremioForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/AlloyDBForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/PostgresForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/SQLServerForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MaterializeForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
🔇 Additional comments (12)
src/webviews/webview-side/integrations/SQLServerForm.tsx (1)
223-232: Verify CA certificate field omission.Other SQL databases (MariaDB, Trino, Materialize, ClickHouse) include
CaCertificateFields, but SQL Server form omits it. Confirm this is intentional based on SQL Server's SSL configuration model.src/platform/common/utils/localize.ts (1)
840-1137: LGTM! Comprehensive localization strings added.All new integration type labels and form strings follow the established l10n.t() pattern. Consistent naming and well-organized structure across all 15+ integration types.
src/notebooks/deepnote/integrations/integrationWebview.ts (2)
10-14: Type refinement correctly applied.The shift from
DatabaseIntegrationConfigtoConfigurableDatabaseIntegrationConfigis consistent across method signatures and imports.Also applies to: 415-415, 457-460
133-374: LGTM! Localization strings properly wired to webview.All new integration type labels and form strings correctly mapped to the LocalizedMessages interface and sent to the webview.
src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
1-87: LGTM! Clean reusable SSH fields component.Well-structured with proper conditional rendering, localization, and validation hints. Host and User correctly marked as required.
src/webviews/webview-side/integrations/PostgresForm.tsx (2)
114-182: LGTM! SSH and CA certificate handlers properly implemented.Handlers correctly update
metadatawith|| undefinedpattern to clean up empty strings. Consistent with the rest of the form.
286-306: LGTM! Subcomponents correctly integrated.SshOptionsFields and CaCertificateFields properly wired with all necessary props and callbacks.
src/webviews/webview-side/integrations/MindsDBForm.tsx (2)
47-53: LGTM! useEffect dependencies correctly fixed.The effect now properly rebuilds config when
existingConfig,integrationId, ordefaultNamechange, addressing the past review comment.
235-240: Review comment references non-existent code.The MindsDBForm.tsx file is 197 lines and contains no
CaCertificateFieldscomponent or certificate-related fields. Lines 235–240 do not exist. The actual form only includes basic input fields (name, host, port, database, username, password) with no SSL or certificate configuration.Likely an incorrect or invalid review comment.
src/webviews/webview-side/integrations/DatabricksForm.tsx (1)
1-246: LGTM! Databricks form correctly implemented.Follows the established pattern with proper state management, SSH integration, validation, and localization. structuredClone used correctly for deep copying.
src/webviews/webview-side/integrations/MySQLForm.tsx (1)
48-54: LGTM! State management and SSH integration correct.useEffect properly reacts to all dependencies, and SshOptionsFields is correctly wired.
Also applies to: 210-219
src/webviews/webview-side/integrations/DremioForm.tsx (1)
1-202: LGTM! Dremio form correctly implemented.Well-structured form following the established pattern. Token field appropriately uses password input type. SSH integration and state management are correct.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/webviews/webview-side/integrations/MaterializeForm.tsx (1)
1-1: Microsoft copyright header still missing.Per coding guidelines, all source files require the Microsoft copyright header. Add it above imports.
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/copilot-instructions.md(0 hunks)src/webviews/webview-side/integrations/MaterializeForm.tsx(1 hunks)src/webviews/webview-side/integrations/RedshiftForm.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- .github/copilot-instructions.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
Files:
src/webviews/webview-side/integrations/MaterializeForm.tsxsrc/webviews/webview-side/integrations/RedshiftForm.tsx
🧬 Code graph analysis (2)
src/webviews/webview-side/integrations/MaterializeForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/RedshiftForm.tsx (4)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
🔇 Additional comments (14)
src/webviews/webview-side/integrations/RedshiftForm.tsx (8)
27-51: LGTM on interface and state initialization.The interface is well-structured, and the credential preservation via
savedCredentialsstate correctly addresses the previous review feedback.
53-59: LGTM on prop synchronization.The effect correctly resets state when props change, with proper dependencies.
61-97: LGTM on name and auth method handlers.The credential preservation logic correctly saves and restores user/password when toggling authentication methods, addressing the previous review feedback.
99-170: LGTM on field change handlers.All handlers follow consistent patterns, properly guard username/password updates, and use
|| undefinedto clean up empty optional fields.
172-175: LGTM on submit handler.Standard form submission with proper preventDefault and callback.
177-260: LGTM on form fields.Proper localization, validation attributes, and field structure for name, auth method, host, port, and database fields.
262-296: LGTM on conditional credential fields.Proper conditional rendering with semantic HTML and autocomplete attributes.
298-329: LGTM on SSH/SSL fields and form actions.Child components receive correct props and handlers. The explicit
showSslEnabled={true}andshowCertificateText={true}props are redundant if those are defaults, but clarity is fine.src/webviews/webview-side/integrations/MaterializeForm.tsx (6)
1-5: LGTM: Imports are clean.Standard React, localization, and type imports. SSH/CA certificate components properly imported.
7-13: LGTM: Interface properly typed.Props use Extract utility type to narrow DatabaseIntegrationConfig. Callbacks clearly defined.
15-34: LGTM: Factory initializes required fields with sensible defaults.Default port '6875' included (addresses past comment). Optional SSH/SSL fields correctly omitted from initial metadata.
36-57: LGTM: Form state correctly resets on integration/config change.Effect dependencies include existingConfig, integrationId, and defaultName. structuredClone ensures deep copy. Addresses past concern about stale credentials.
59-157: LGTM: Change handlers follow consistent pattern.Empty strings and false values converted to undefined to keep metadata minimal. Nested metadata updates properly spread.
159-295: LGTM: Form structure follows React best practices.All user-facing strings localized. Required fields properly marked and validated. SSH and CA certificate subforms cleanly integrated.
|
I did not manually test all the integrations obviously but I tried to make sure all options are included and since the metadata is being validated against the types from the library, they should be valid. The translation to SQL alchemy config is thoroughly tested in the library and the processing of integration config within the extension has tests from the previous PR. |
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (3)
src/webviews/webview-side/integrations/ClickHouseForm.tsx (1)
1-1: Missing Microsoft copyright header.Per coding guidelines, all source files require the Microsoft copyright header.
As per coding guidelines
src/webviews/webview-side/integrations/MaterializeForm.tsx (1)
1-1: Microsoft copyright header still missing.As per coding guidelines
src/webviews/webview-side/integrations/RedshiftForm.tsx (1)
1-1: Add Microsoft copyright header.All source files must include the Microsoft copyright header per coding guidelines.
As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
src/webviews/webview-side/integrations/AlloyDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/AthenaForm.tsx(1 hunks)src/webviews/webview-side/integrations/BigQueryForm.tsx(1 hunks)src/webviews/webview-side/integrations/ClickHouseForm.tsx(1 hunks)src/webviews/webview-side/integrations/DatabricksForm.tsx(1 hunks)src/webviews/webview-side/integrations/DremioForm.tsx(1 hunks)src/webviews/webview-side/integrations/MariaDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MaterializeForm.tsx(1 hunks)src/webviews/webview-side/integrations/MindsDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MongoDBForm.tsx(1 hunks)src/webviews/webview-side/integrations/MySQLForm.tsx(1 hunks)src/webviews/webview-side/integrations/PostgresForm.tsx(3 hunks)src/webviews/webview-side/integrations/RedshiftForm.tsx(1 hunks)src/webviews/webview-side/integrations/SQLServerForm.tsx(1 hunks)src/webviews/webview-side/integrations/SnowflakeForm.tsx(7 hunks)src/webviews/webview-side/integrations/SpannerForm.tsx(1 hunks)src/webviews/webview-side/integrations/TrinoForm.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Inject interfaces, not concrete classes
Avoid circular dependencies
Usel10n.t()for user-facing strings
Use typed error classes fromsrc/platform/errors/when throwing or handling errors
Use theILoggerservice instead ofconsole.log
Preserve error details while scrubbing PII in messages and telemetry
Include the Microsoft copyright header in source files
Prefer async/await and handle cancellation withCancellationToken
Files:
src/webviews/webview-side/integrations/AthenaForm.tsxsrc/webviews/webview-side/integrations/TrinoForm.tsxsrc/webviews/webview-side/integrations/MongoDBForm.tsxsrc/webviews/webview-side/integrations/SnowflakeForm.tsxsrc/webviews/webview-side/integrations/ClickHouseForm.tsxsrc/webviews/webview-side/integrations/DremioForm.tsxsrc/webviews/webview-side/integrations/MySQLForm.tsxsrc/webviews/webview-side/integrations/SpannerForm.tsxsrc/webviews/webview-side/integrations/MariaDBForm.tsxsrc/webviews/webview-side/integrations/PostgresForm.tsxsrc/webviews/webview-side/integrations/SQLServerForm.tsxsrc/webviews/webview-side/integrations/RedshiftForm.tsxsrc/webviews/webview-side/integrations/MaterializeForm.tsxsrc/webviews/webview-side/integrations/MindsDBForm.tsxsrc/webviews/webview-side/integrations/AlloyDBForm.tsxsrc/webviews/webview-side/integrations/DatabricksForm.tsxsrc/webviews/webview-side/integrations/BigQueryForm.tsx
🧬 Code graph analysis (16)
src/webviews/webview-side/integrations/AthenaForm.tsx (1)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/TrinoForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/MongoDBForm.tsx (4)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/SnowflakeForm.tsx (2)
src/platform/notebooks/deepnote/integrationTypes.ts (2)
SnowflakeAuthMethod(86-86)SnowflakeAuthMethods(87-87)src/platform/notebooks/deepnote/snowflakeAuthConstants.ts (2)
SnowflakeAuthMethod(13-13)SnowflakeAuthMethods(4-11)
src/webviews/webview-side/integrations/ClickHouseForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/DremioForm.tsx (2)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)
src/webviews/webview-side/integrations/MySQLForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/SpannerForm.tsx (2)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)
src/webviews/webview-side/integrations/MariaDBForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/PostgresForm.tsx (2)
src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/SQLServerForm.tsx (3)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)
src/webviews/webview-side/integrations/RedshiftForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/MaterializeForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/MindsDBForm.tsx (4)
src/platform/common/utils/localize.ts (1)
unnamedIntegration(833-833)src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/AlloyDBForm.tsx (3)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)src/webviews/webview-side/integrations/CaCertificateFields.tsx (1)
CaCertificateFields(17-80)
src/webviews/webview-side/integrations/DatabricksForm.tsx (2)
src/webviews/webview-side/react-common/locReactSide.ts (1)
getLocString(11-17)src/webviews/webview-side/integrations/SshOptionsFields.tsx (1)
SshOptionsFields(15-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🔇 Additional comments (24)
src/webviews/webview-side/integrations/SnowflakeForm.tsx (1)
79-231: LGTM—consistent value extraction pattern.Extracting
e.target.valueinto local variables before use standardizes the form handlers. The cast toSnowflakeAuthMethod(line 100) is safe given the controlled select options, andvalue || undefinedcorrectly handles optional fields.src/webviews/webview-side/integrations/BigQueryForm.tsx (1)
59-65: Consistent event handling pattern.Extracting the value to a local variable aligns with
handleCredentialsChange(line 86) and is a React best practice.src/webviews/webview-side/integrations/DremioForm.tsx (1)
33-207: Implementation looks solid.Form logic, state management, and SSH integration are correct. Token field properly uses password type, and required fields are validated.
src/webviews/webview-side/integrations/AthenaForm.tsx (4)
5-23: LGTM.Type narrowing and default initialization are sound.
46-52: Verify form reset behavior on prop changes.useEffect resets all pending edits when any dependency changes. Confirm this is intentional if props can update during editing.
84-191: LGTM.Form structure and submission flow are correct. Sensitive field uses
type="password"andautoComplete="off"appropriately.
1-3: Remove the localization verification request—it's incorrect.
getLocString()is the documented pattern for webview-side React code, not a mistake. The locReactSide module exists specifically because webview code cannot usel10n.t()(which reads from disk). All integration forms in this folder consistently usegetLocString(), confirming this is the intended approach.The copyright header issue remains valid—add it for consistency with guidelines, though it's currently missing across all integration forms in this folder.
Likely an incorrect or invalid review comment.
src/webviews/webview-side/integrations/ClickHouseForm.tsx (9)
1-6: LGTM—Imports are clean.
7-13: LGTM—Interface properly typed.Good use of
Extractutility type for type narrowing.
15-31: LGTM—Helper creates correct empty config.Properly initializes required fields and uses localization.
33-46: LGTM—Initialization handles both new and edit cases.
structuredCloneensures proper deep copy.
48-54: LGTM—Effect properly resets form when switching integrations.Dependency array correctly includes
existingConfig,integrationId, anddefaultName.
56-59: LGTM—Submit handler follows standard pattern.
61-104: LGTM—Field handlers follow consistent pattern.Proper state spreading and optional field handling with
|| undefined.
106-153: LGTM—SSH/SSL handlers properly manage optional fields.
155-272: LGTM—Form properly wired with localization and validation.Required fields marked, subcomponents integrated correctly.
src/webviews/webview-side/integrations/MaterializeForm.tsx (3)
7-34: LGTM: Interface and config initialization are well-structured.The Extract type usage ensures type safety, and the default port is now included in metadata (addressing prior feedback).
43-57: LGTM: State management correctly handles integration switching.The effect now properly depends on all relevant props, preventing stale credentials when switching integrations.
166-303: LGTM: Form structure follows best practices.Proper localization, HTML5 validation, appropriate autocomplete attributes, and consistent use of shared components for SSH/SSL fields.
src/webviews/webview-side/integrations/SQLServerForm.tsx (3)
49-55: Proper state synchronization.The effect correctly handles both existingConfig presence and absence, uses
structuredCloneto prevent mutations, and includes all relevant dependencies.
107-133: Verify|| undefinedcoercion behavior.The pattern
enabled || undefinedconvertsfalsetoundefined(line 110), removing the field from metadata entirely. Similarly, empty strings becomeundefined(lines 117, 124, 131). Confirm this is the intended serialization behavior rather than explicitly storingfalseor empty strings.
135-249: Well-structured form.Proper localization, required-field marking, appropriate autoComplete attributes, and clean integration of
SshOptionsFields. The implementation follows established patterns.src/webviews/webview-side/integrations/PostgresForm.tsx (2)
23-23: VerifysslEnabled: falseinitialization.Handler at line 165 converts
falsetoundefinedviaenabled || undefined. Initial config should match this pattern for consistency—consider removingsslEnabledfrom the initial metadata or confirming the difference is intentional.
292-312: LGTM: Component wiring correct.Both
SshOptionsFieldsandCaCertificateFieldsare properly wired with all required props and handlers.
|
I tested a couple of different postgres integrations and both are timing out with this error (they work directly on Deepnote): 
|
|
This works when I try it from the office; at my home, it also did not work. IP white listing happening likely. |
Summary by CodeRabbit
New Features
Improved UX
Documentation
Other