Skip to content

Commit 5d0b560

Browse files
committed
introduce narrower ConfigurableDatabaseIntegration* types
1 parent 58c86b6 commit 5d0b560

15 files changed

+100
-68
lines changed

src/notebooks/deepnote/deepnoteNotebookManager.unit.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as assert from 'assert';
22

33
import { DeepnoteNotebookManager } from './deepnoteNotebookManager';
44
import type { DeepnoteProject } from '../../platform/deepnote/deepnoteTypes';
5+
import { ProjectIntegration } from '../types';
56

67
suite('DeepnoteNotebookManager', () => {
78
let manager: DeepnoteNotebookManager;
@@ -187,7 +188,7 @@ suite('DeepnoteNotebookManager', () => {
187188
test('should update integrations list for existing project and return true', () => {
188189
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');
189190

190-
const integrations = [
191+
const integrations: ProjectIntegration[] = [
191192
{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' },
192193
{ id: 'int-2', name: 'BigQuery', type: 'big-query' }
193194
];
@@ -211,7 +212,7 @@ suite('DeepnoteNotebookManager', () => {
211212

212213
manager.storeOriginalProject('project-123', projectWithIntegrations, 'notebook-456');
213214

214-
const newIntegrations = [
215+
const newIntegrations: ProjectIntegration[] = [
215216
{ id: 'new-int-1', name: 'New Integration 1', type: 'pgsql' },
216217
{ id: 'new-int-2', name: 'New Integration 2', type: 'big-query' }
217218
];
@@ -257,7 +258,7 @@ suite('DeepnoteNotebookManager', () => {
257258
test('should preserve other project properties and return true', () => {
258259
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');
259260

260-
const integrations = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }];
261+
const integrations: ProjectIntegration[] = [{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' }];
261262

262263
const result = manager.updateProjectIntegrations('project-123', integrations);
263264

@@ -275,7 +276,7 @@ suite('DeepnoteNotebookManager', () => {
275276
manager.storeOriginalProject('project-123', mockProject, 'notebook-456');
276277
manager.updateCurrentNotebookId('project-123', undefined as any);
277278

278-
const integrations = [
279+
const integrations: ProjectIntegration[] = [
279280
{ id: 'int-1', name: 'PostgreSQL', type: 'pgsql' },
280281
{ id: 'int-2', name: 'BigQuery', type: 'big-query' }
281282
];

src/notebooks/deepnote/integrations/integrationDetector.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ import { inject, injectable } from 'inversify';
22

33
import { logger } from '../../../platform/logging';
44
import { IDeepnoteNotebookManager } from '../../types';
5-
import { IntegrationStatus, IntegrationWithStatus } from '../../../platform/notebooks/deepnote/integrationTypes';
5+
import {
6+
ConfigurableDatabaseIntegrationType,
7+
IntegrationStatus,
8+
IntegrationWithStatus
9+
} from '../../../platform/notebooks/deepnote/integrationTypes';
610
import { IIntegrationDetector, IIntegrationStorage } from './types';
7-
import { DatabaseIntegrationType, databaseIntegrationTypes } from '@deepnote/database-integrations';
11+
import { databaseIntegrationTypes } from '@deepnote/database-integrations';
812

913
/**
1014
* Service for detecting integrations used in Deepnote notebooks
@@ -41,7 +45,10 @@ export class IntegrationDetector implements IIntegrationDetector {
4145
for (const projectIntegration of projectIntegrations) {
4246
const integrationId = projectIntegration.id;
4347
const integrationType = projectIntegration.type;
44-
if (!(databaseIntegrationTypes as readonly string[]).includes(integrationType)) {
48+
if (
49+
!(databaseIntegrationTypes as readonly string[]).includes(integrationType) ||
50+
integrationType === 'pandas-dataframe'
51+
) {
4552
logger.debug(`IntegrationDetector: Skipping unsupported integration type: ${integrationType}`);
4653
continue;
4754
}
@@ -53,7 +60,7 @@ export class IntegrationDetector implements IIntegrationDetector {
5360
status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected,
5461
// Include integration metadata from project for prefilling when config is null
5562
integrationName: projectIntegration.name,
56-
integrationType: integrationType as DatabaseIntegrationType
63+
integrationType: integrationType as ConfigurableDatabaseIntegrationType
5764
};
5865

5966
integrations.set(integrationId, status);

src/notebooks/deepnote/integrations/integrationManager.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ export class IntegrationManager implements IIntegrationManager {
161161
integrationType = projectIntegration.type as DatabaseIntegrationType;
162162
}
163163

164+
if (integrationType === 'pandas-dataframe') {
165+
logger.debug(`IntegrationManager: Skipping internal DuckDB integration ${selectedIntegrationId}`);
166+
return;
167+
}
168+
164169
integrations.set(selectedIntegrationId, {
165170
config: config || null,
166171
status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected,

src/notebooks/deepnote/integrations/integrationWebview.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import { logger } from '../../../platform/logging';
77
import { LocalizedMessages, SharedMessages } from '../../../messageTypes';
88
import { IDeepnoteNotebookManager, ProjectIntegration } from '../../types';
99
import { IIntegrationStorage, IIntegrationWebviewProvider } from './types';
10-
import { IntegrationStatus, IntegrationWithStatus } from '../../../platform/notebooks/deepnote/integrationTypes';
11-
import { DatabaseIntegrationConfig } from '@deepnote/database-integrations';
10+
import {
11+
ConfigurableDatabaseIntegrationConfig,
12+
IntegrationStatus,
13+
IntegrationWithStatus
14+
} from '../../../platform/notebooks/deepnote/integrationTypes';
1215

1316
/**
1417
* Manages the webview panel for integration configuration
@@ -217,7 +220,7 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider {
217220
private async handleMessage(message: {
218221
type: string;
219222
integrationId?: string;
220-
config?: DatabaseIntegrationConfig;
223+
config?: ConfigurableDatabaseIntegrationConfig;
221224
}): Promise<void> {
222225
switch (message.type) {
223226
case 'configure':
@@ -259,7 +262,10 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider {
259262
/**
260263
* Save the configuration for an integration
261264
*/
262-
private async saveConfiguration(integrationId: string, config: DatabaseIntegrationConfig): Promise<void> {
265+
private async saveConfiguration(
266+
integrationId: string,
267+
config: ConfigurableDatabaseIntegrationConfig
268+
): Promise<void> {
263269
try {
264270
await this.integrationStorage.save(config);
265271

@@ -345,12 +351,6 @@ export class IntegrationWebviewProvider implements IIntegrationWebviewProvider {
345351
return null;
346352
}
347353

348-
// Skip DuckDB integration (internal, not a real Deepnote integration)
349-
if (type === 'pandas-dataframe') {
350-
logger.trace(`IntegrationWebviewProvider: Skipping internal DuckDB integration ${id}`);
351-
return null;
352-
}
353-
354354
return {
355355
id,
356356
name: integration.config?.name || integration.integrationName || id,

src/notebooks/deepnote/sqlCellStatusBarProvider.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import { IExtensionSyncActivationService } from '../../platform/activation/types
2222
import { IDisposableRegistry } from '../../platform/common/types';
2323
import { IIntegrationStorage } from './integrations/types';
2424
import { Commands } from '../../platform/common/constants';
25-
import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes';
25+
import {
26+
ConfigurableDatabaseIntegrationType,
27+
DATAFRAME_SQL_INTEGRATION_ID
28+
} from '../../platform/notebooks/deepnote/integrationTypes';
2629
import { IDeepnoteNotebookManager } from '../types';
2730
import { DatabaseIntegrationType, databaseIntegrationTypes } from '@deepnote/database-integrations';
2831

@@ -338,15 +341,20 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid
338341

339342
// Add all project integrations
340343
for (const projectIntegration of projectIntegrations) {
344+
const integrationType =
345+
projectIntegration.type &&
346+
(databaseIntegrationTypes as readonly string[]).includes(projectIntegration.type)
347+
? (projectIntegration.type as DatabaseIntegrationType)
348+
: null;
349+
341350
// Skip the internal DuckDB integration
342-
if (projectIntegration.id === DATAFRAME_SQL_INTEGRATION_ID) {
351+
if (projectIntegration.id === DATAFRAME_SQL_INTEGRATION_ID || integrationType === 'pandas-dataframe') {
343352
continue;
344353
}
345354

346-
const integrationType = projectIntegration.type;
347355
const typeLabel =
348356
integrationType && (databaseIntegrationTypes as readonly string[]).includes(integrationType)
349-
? this.getIntegrationTypeLabel(integrationType as DatabaseIntegrationType)
357+
? this.getIntegrationTypeLabel(integrationType)
350358
: projectIntegration.type;
351359

352360
const item: LocalQuickPickItem = {
@@ -430,7 +438,7 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid
430438
this._onDidChangeCellStatusBarItems.fire();
431439
}
432440

433-
private getIntegrationTypeLabel(type: DatabaseIntegrationType): string {
441+
private getIntegrationTypeLabel(type: ConfigurableDatabaseIntegrationType): string {
434442
switch (type) {
435443
case 'pgsql':
436444
return l10n.t('PostgreSQL');

src/notebooks/types.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { NotebookDocument, NotebookEditor, Uri, type Event } from 'vscode';
55
import { Resource } from '../platform/common/types';
66
import type { EnvironmentPath } from '@vscode/python-extension';
77
import { DeepnoteProject } from '../platform/deepnote/deepnoteTypes';
8+
import { ConfigurableDatabaseIntegrationType } from '../platform/notebooks/deepnote/integrationTypes';
89

910
export interface IEmbedNotebookEditorProvider {
1011
findNotebookEditor(resource: Resource): NotebookEditor | undefined;
@@ -31,7 +32,7 @@ export interface INotebookPythonEnvironmentService {
3132
export interface ProjectIntegration {
3233
id: string;
3334
name: string;
34-
type: string;
35+
type: ConfigurableDatabaseIntegrationType;
3536
}
3637

3738
export const IDeepnoteNotebookManager = Symbol('IDeepnoteNotebookManager');

src/platform/notebooks/deepnote/integrationStorage.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,13 @@ import { IAsyncDisposableRegistry } from '../../common/types';
66
import { logger } from '../../logging';
77
import { IIntegrationStorage } from './types';
88
import { upgradeLegacyIntegrationConfig } from './legacyIntegrationConfigUtils';
9-
import {
10-
DatabaseIntegrationConfig,
11-
databaseIntegrationTypes,
12-
databaseMetadataSchemasByType
13-
} from '@deepnote/database-integrations';
14-
import { DATAFRAME_SQL_INTEGRATION_ID } from './integrationTypes';
9+
import { databaseIntegrationTypes, databaseMetadataSchemasByType } from '@deepnote/database-integrations';
10+
import { ConfigurableDatabaseIntegrationConfig, DATAFRAME_SQL_INTEGRATION_ID } from './integrationTypes';
1511

1612
const INTEGRATION_SERVICE_NAME = 'deepnote-integrations';
1713

1814
// NOTE: We need a way to upgrade existing configurations to the new format of deepnote/database-integrations.
19-
type VersionedDatabaseIntegrationConfig = DatabaseIntegrationConfig & { version: 1 };
15+
type VersionedDatabaseIntegrationConfig = ConfigurableDatabaseIntegrationConfig & { version: 1 };
2016

2117
function storeEncryptedIntegrationConfig(
2218
encryptedStorage: IEncryptedStorage,
@@ -33,7 +29,7 @@ function storeEncryptedIntegrationConfig(
3329
*/
3430
@injectable()
3531
export class IntegrationStorage implements IIntegrationStorage {
36-
private readonly cache: Map<string, DatabaseIntegrationConfig> = new Map();
32+
private readonly cache: Map<string, ConfigurableDatabaseIntegrationConfig> = new Map();
3733

3834
private cacheLoaded = false;
3935

@@ -52,15 +48,15 @@ export class IntegrationStorage implements IIntegrationStorage {
5248
/**
5349
* Get all stored integration configurations.
5450
*/
55-
async getAll(): Promise<DatabaseIntegrationConfig[]> {
51+
async getAll(): Promise<ConfigurableDatabaseIntegrationConfig[]> {
5652
await this.ensureCacheLoaded();
5753
return Array.from(this.cache.values());
5854
}
5955

6056
/**
6157
* Get a specific integration configuration by ID
6258
*/
63-
async getIntegrationConfig(integrationId: string): Promise<DatabaseIntegrationConfig | undefined> {
59+
async getIntegrationConfig(integrationId: string): Promise<ConfigurableDatabaseIntegrationConfig | undefined> {
6460
await this.ensureCacheLoaded();
6561
return this.cache.get(integrationId);
6662
}
@@ -73,15 +69,15 @@ export class IntegrationStorage implements IIntegrationStorage {
7369
async getProjectIntegrationConfig(
7470
_projectId: string,
7571
integrationId: string
76-
): Promise<DatabaseIntegrationConfig | undefined> {
72+
): Promise<ConfigurableDatabaseIntegrationConfig | undefined> {
7773
return this.getIntegrationConfig(integrationId);
7874
}
7975

8076
/**
8177
* Save or update an integration configuration
8278
*/
83-
async save(config: DatabaseIntegrationConfig): Promise<void> {
84-
if (config.type === 'pandas-dataframe' || config.id === DATAFRAME_SQL_INTEGRATION_ID) {
79+
async save(config: ConfigurableDatabaseIntegrationConfig): Promise<void> {
80+
if ((config.type as string) === 'pandas-dataframe' || config.id === DATAFRAME_SQL_INTEGRATION_ID) {
8581
logger.warn(`IntegrationStorage: Skipping save for internal DuckDB integration ${config.id}`);
8682
return;
8783
}
@@ -210,7 +206,7 @@ export class IntegrationStorage implements IIntegrationStorage {
210206
const config =
211207
databaseIntegrationTypes.includes(rawConfig.type) &&
212208
rawConfig.type !== 'pandas-dataframe'
213-
? (rawConfig as DatabaseIntegrationConfig)
209+
? (rawConfig as ConfigurableDatabaseIntegrationConfig)
214210
: null;
215211
const validMetadata = config
216212
? databaseMetadataSchemasByType[config.type].safeParse(config.metadata).data
@@ -219,7 +215,7 @@ export class IntegrationStorage implements IIntegrationStorage {
219215
this.cache.set(
220216
id,
221217
// NOTE: We must cast here because there is no union-wide schema parser at the moment.
222-
{ ...config, metadata: validMetadata } as DatabaseIntegrationConfig
218+
{ ...config, metadata: validMetadata } as ConfigurableDatabaseIntegrationConfig
223219
);
224220
} else {
225221
logger.warn(`Invalid integration config for ${id}, marking for deletion`);

src/platform/notebooks/deepnote/integrationStorage.unit.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ suite('IntegrationStorage', () => {
197197
metadata: {}
198198
};
199199

200-
await storage.save(config);
200+
await storage.save(config as any);
201201
const result = await storage.getIntegrationConfig('dataframe-1');
202202

203203
assert.strictEqual(result, undefined);
@@ -211,7 +211,7 @@ suite('IntegrationStorage', () => {
211211
metadata: {}
212212
};
213213

214-
await storage.save(config);
214+
await storage.save(config as any);
215215
const result = await storage.getIntegrationConfig(DATAFRAME_SQL_INTEGRATION_ID);
216216

217217
assert.strictEqual(result, undefined);

src/platform/notebooks/deepnote/integrationTypes.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,13 @@ export type LegacyIntegrationConfig =
136136
| LegacySnowflakeIntegrationConfig
137137
| LegacyDuckDBIntegrationConfig;
138138

139+
export type ConfigurableDatabaseIntegrationConfig = Extract<
140+
DatabaseIntegrationConfig,
141+
{ type: ConfigurableDatabaseIntegrationType }
142+
>;
143+
144+
export type ConfigurableDatabaseIntegrationType = Exclude<DatabaseIntegrationType, 'pandas-dataframe'>;
145+
139146
/**
140147
* Integration connection status
141148
*/
@@ -149,7 +156,7 @@ export enum IntegrationStatus {
149156
* Integration with its current status
150157
*/
151158
export interface IntegrationWithStatus {
152-
config: DatabaseIntegrationConfig | null;
159+
config: ConfigurableDatabaseIntegrationConfig | null;
153160
status: IntegrationStatus;
154161
error?: string;
155162
/**
@@ -159,5 +166,5 @@ export interface IntegrationWithStatus {
159166
/**
160167
* Type from the project's integrations list (used for prefilling when config is null)
161168
*/
162-
integrationType?: DatabaseIntegrationType;
169+
integrationType?: ConfigurableDatabaseIntegrationType;
163170
}

src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
IPlatformDeepnoteNotebookManager
1212
} from './types';
1313
import { DATAFRAME_SQL_INTEGRATION_ID } from './integrationTypes';
14-
import { getEnvironmentVariablesForIntegrations } from '@deepnote/database-integrations';
14+
import { DatabaseIntegrationConfig, getEnvironmentVariablesForIntegrations } from '@deepnote/database-integrations';
1515

1616
/**
1717
* Provides environment variables for SQL integrations.
@@ -88,7 +88,7 @@ export class SqlIntegrationEnvironmentVariablesProvider implements ISqlIntegrati
8888
`SqlIntegrationEnvironmentVariablesProvider: Found ${projectIntegrations.length} integrations in project`
8989
);
9090

91-
const projectIntegrationConfigs = (
91+
const projectIntegrationConfigs: Array<DatabaseIntegrationConfig> = (
9292
await Promise.all(
9393
projectIntegrations.map((integration) => {
9494
return this.integrationStorage.getIntegrationConfig(integration.id);

0 commit comments

Comments
 (0)