From c04a3ff14606beae66f17aefd741c04986426ec4 Mon Sep 17 00:00:00 2001 From: Tomas Kislan Date: Thu, 30 Oct 2025 12:36:47 +0000 Subject: [PATCH 1/5] fix: Fix SQL dataframe integration Signed-off-by: Tomas Kislan --- ...IntegrationEnvironmentVariablesProvider.ts | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index f42598127..cfd9df1bd 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -199,6 +199,19 @@ export class SqlIntegrationEnvironmentVariablesProvider implements ISqlIntegrati return envVars; } + // Always add the internal DuckDB integration + const dataframeSqlIntegrationEnvVarName = convertToEnvironmentVariableName( + getSqlEnvVarName(DATAFRAME_SQL_INTEGRATION_ID) + ); + const dataframeSqlIntegrationCredentialsJson = JSON.stringify({ + url: 'deepnote+duckdb:///:memory:', + params: {}, + param_style: 'qmark' + }); + + envVars[dataframeSqlIntegrationEnvVarName] = dataframeSqlIntegrationCredentialsJson; + logger.debug(`SqlIntegrationEnvironmentVariablesProvider: Added env var for dataframe SQL integration`); + // Scan all cells for SQL integration IDs const integrationIds = this.scanNotebookForIntegrations(notebook); if (integrationIds.size === 0) { @@ -219,17 +232,7 @@ export class SqlIntegrationEnvironmentVariablesProvider implements ISqlIntegrati try { // Handle internal DuckDB integration specially if (integrationId === DATAFRAME_SQL_INTEGRATION_ID) { - const envVarName = convertToEnvironmentVariableName(getSqlEnvVarName(integrationId)); - const credentialsJson = JSON.stringify({ - url: 'deepnote+duckdb:///:memory:', - params: {}, - param_style: 'qmark' - }); - - envVars[envVarName] = credentialsJson; - logger.debug( - `SqlIntegrationEnvironmentVariablesProvider: Added env var for dataframe SQL integration` - ); + // Internal DuckDB integration is handled above continue; } From 0af50abaa76dff7d65279db26a6627e83ac1578e Mon Sep 17 00:00:00 2001 From: Tomas Kislan Date: Thu, 30 Oct 2025 13:05:32 +0000 Subject: [PATCH 2/5] test: Update test expectations Signed-off-by: Tomas Kislan --- ...nEnvironmentVariablesProvider.unit.test.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index 5a15e4f30..c34542ada 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -14,6 +14,10 @@ import { } from './integrationTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; +const EXPECTED_EMPTY_ENV_VARS = { + SQL_DEEPNOTE_DATAFRAME_SQL: '{"url":"deepnote+duckdb:///:memory:","params":{},"param_style":"qmark"}' +}; + suite('SqlIntegrationEnvironmentVariablesProvider', () => { let provider: SqlIntegrationEnvironmentVariablesProvider; let integrationStorage: IntegrationStorage; @@ -55,7 +59,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); test('Returns empty object when SQL cells have no integration ID', async () => { @@ -67,7 +71,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); test('Returns environment variable for internal DuckDB integration', async () => { @@ -188,7 +192,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Should only have one environment variable assert.property(envVars, 'SQL_MY_POSTGRES_DB'); - assert.strictEqual(Object.keys(envVars).length, 1); + assert.strictEqual(Object.keys(envVars).length, 2); }); test('Handles multiple SQL cells with different integrations', async () => { @@ -233,7 +237,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Should have two environment variables assert.property(envVars, 'SQL_MY_POSTGRES_DB'); assert.property(envVars, 'SQL_MY_BIGQUERY'); - assert.strictEqual(Object.keys(envVars).length, 2); + assert.strictEqual(Object.keys(envVars).length, 3); }); test('Handles missing integration configuration gracefully', async () => { @@ -252,7 +256,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const envVars = await provider.getEnvironmentVariables(uri); // Should return empty object when integration config is missing - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); test('Properly encodes special characters in PostgreSQL credentials', async () => { @@ -623,7 +627,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Should return empty object when unsupported auth method is encountered const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); test('Skips unsupported Snowflake auth method (AZURE_AD)', async () => { @@ -647,7 +651,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); test('Skips unsupported Snowflake auth method (KEY_PAIR)', async () => { @@ -671,7 +675,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, {}); + assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); }); }); From ebfbc3f0cb6e1b42117f07a5240523b29add4caa Mon Sep 17 00:00:00 2001 From: Tomas Kislan Date: Thu, 30 Oct 2025 13:14:16 +0000 Subject: [PATCH 3/5] test: Update comments Signed-off-by: Tomas Kislan --- .../sqlIntegrationEnvironmentVariablesProvider.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index c34542ada..e2e71af6d 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -190,7 +190,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const envVars = await provider.getEnvironmentVariables(uri); - // Should only have one environment variable + // Should only have one environment variable apart from the internal DuckDB integration assert.property(envVars, 'SQL_MY_POSTGRES_DB'); assert.strictEqual(Object.keys(envVars).length, 2); }); @@ -234,7 +234,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const envVars = await provider.getEnvironmentVariables(uri); - // Should have two environment variables + // Should have two environment variables apart from the internal DuckDB integration assert.property(envVars, 'SQL_MY_POSTGRES_DB'); assert.property(envVars, 'SQL_MY_BIGQUERY'); assert.strictEqual(Object.keys(envVars).length, 3); From b10def75d2ac9bbbce789be0c9903a94e57a6d63 Mon Sep 17 00:00:00 2001 From: Tomas Kislan Date: Thu, 30 Oct 2025 13:39:27 +0000 Subject: [PATCH 4/5] test: Update comments Signed-off-by: Tomas Kislan --- .../sqlIntegrationEnvironmentVariablesProvider.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index e2e71af6d..01a03bfca 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -255,7 +255,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const envVars = await provider.getEnvironmentVariables(uri); - // Should return empty object when integration config is missing + // Should return only dataframe integration when integration config is missing assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); @@ -625,7 +625,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); - // Should return empty object when unsupported auth method is encountered + // Should return only dataframe integration when unsupported auth method is encountered const envVars = await provider.getEnvironmentVariables(uri); assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); }); From ca4a1686dc18ce9aded374be249b5c4086baa01f Mon Sep 17 00:00:00 2001 From: Tomas Kislan Date: Thu, 30 Oct 2025 13:49:57 +0000 Subject: [PATCH 5/5] test: Rename empty env vars test variable Signed-off-by: Tomas Kislan --- ...rationEnvironmentVariablesProvider.unit.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index 01a03bfca..171d4a214 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -14,7 +14,7 @@ import { } from './integrationTypes'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; -const EXPECTED_EMPTY_ENV_VARS = { +const EXPECTED_DATAFRAME_ONLY_ENV_VARS = { SQL_DEEPNOTE_DATAFRAME_SQL: '{"url":"deepnote+duckdb:///:memory:","params":{},"param_style":"qmark"}' }; @@ -59,7 +59,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); test('Returns empty object when SQL cells have no integration ID', async () => { @@ -71,7 +71,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); test('Returns environment variable for internal DuckDB integration', async () => { @@ -256,7 +256,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const envVars = await provider.getEnvironmentVariables(uri); // Should return only dataframe integration when integration config is missing - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); test('Properly encodes special characters in PostgreSQL credentials', async () => { @@ -627,7 +627,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Should return only dataframe integration when unsupported auth method is encountered const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); test('Skips unsupported Snowflake auth method (AZURE_AD)', async () => { @@ -651,7 +651,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); test('Skips unsupported Snowflake auth method (KEY_PAIR)', async () => { @@ -675,7 +675,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); - assert.deepStrictEqual(envVars, EXPECTED_EMPTY_ENV_VARS); + assert.deepStrictEqual(envVars, EXPECTED_DATAFRAME_ONLY_ENV_VARS); }); }); });