From 915fa56bcca49b55865b002f70a50c4ad8ec9e36 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 12:58:40 +0200 Subject: [PATCH 01/43] feat: setting env vars for connected integrations --- .../deepnote/deepnoteServerStarter.node.ts | 38 ++- src/kernels/kernel.ts | 14 +- .../raw/launcher/kernelEnvVarsService.node.ts | 61 +++- src/kernels/serviceRegistry.node.ts | 5 + ...IntegrationEnvironmentVariablesProvider.ts | 187 ++++++++++++ ...nEnvironmentVariablesProvider.unit.test.ts | 275 ++++++++++++++++++ .../sqlIntegrationStartupCodeProvider.ts | 118 ++++++++ src/notebooks/serviceRegistry.node.ts | 5 + 8 files changed, 695 insertions(+), 8 deletions(-) create mode 100644 src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts create mode 100644 src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts create mode 100644 src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index ec6b242d3e..07ac0d2fad 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { inject, injectable, named } from 'inversify'; +import { inject, injectable, named, optional } from 'inversify'; import { CancellationToken, Uri } from 'vscode'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { IDeepnoteServerStarter, IDeepnoteToolkitInstaller, DeepnoteServerInfo, DEEPNOTE_DEFAULT_PORT } from './types'; @@ -17,6 +17,7 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from '../../platform/vscode-path/path'; import { generateUuid } from '../../platform/common/uuid'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; /** * Lock file data structure for tracking server ownership @@ -47,7 +48,10 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IHttpClient) private readonly httpClient: IHttpClient, - @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry + @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, + @inject(SqlIntegrationEnvironmentVariablesProvider) + @optional() + private readonly sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider ) { // Register for disposal when the extension deactivates asyncRegistry.push(this); @@ -149,10 +153,38 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension // Detached mode ensures no requests are made to the backend (directly, or via proxy) // as there is no backend running in the extension, therefore: - // 1. integration environment variables won't work / be injected + // 1. integration environment variables are injected here instead // 2. post start hooks won't work / are not executed env.DEEPNOTE_RUNTIME__RUNNING_IN_DETACHED_MODE = 'true'; + // Inject SQL integration environment variables + if (this.sqlIntegrationEnvVars) { + logger.info(`DeepnoteServerStarter: Injecting SQL integration env vars for ${deepnoteFileUri.toString()}`); + try { + const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); + if (sqlEnvVars && Object.keys(sqlEnvVars).length > 0) { + logger.info( + `DeepnoteServerStarter: Injecting ${ + Object.keys(sqlEnvVars).length + } SQL integration env vars: ${Object.keys(sqlEnvVars).join(', ')}` + ); + Object.assign(env, sqlEnvVars); + // Log the first 100 chars of each env var value for debugging + for (const [key, value] of Object.entries(sqlEnvVars)) { + if (value) { + logger.info(`DeepnoteServerStarter: ${key} = ${value.substring(0, 100)}...`); + } + } + } else { + logger.info('DeepnoteServerStarter: No SQL integration env vars to inject'); + } + } catch (error) { + logger.error('DeepnoteServerStarter: Failed to get SQL integration env vars', error); + } + } else { + logger.info('DeepnoteServerStarter: SqlIntegrationEnvironmentVariablesProvider not available'); + } + // Remove PYTHONHOME if it exists (can interfere with venv) delete env.PYTHONHOME; diff --git a/src/kernels/kernel.ts b/src/kernels/kernel.ts index 0d1d1cb4db..86c3faf1c3 100644 --- a/src/kernels/kernel.ts +++ b/src/kernels/kernel.ts @@ -853,10 +853,22 @@ abstract class BaseKernel implements IBaseKernel { // Gather all of the startup code at one time and execute as one cell const startupCode = await this.gatherInternalStartupCode(); - await this.executeSilently(session, startupCode, { + logger.info(`Executing startup code with ${startupCode.length} lines`); + const outputs = await this.executeSilently(session, startupCode, { traceErrors: true, traceErrorsMessage: 'Error executing jupyter extension internal startup code' }); + logger.info(`Startup code execution completed with ${outputs?.length || 0} outputs`); + if (outputs && outputs.length > 0) { + outputs.forEach((output, idx) => { + logger.info( + `Startup code output ${idx}: ${output.output_type} - ${JSON.stringify(output).substring( + 0, + 200 + )}` + ); + }); + } // Run user specified startup commands await this.executeSilently(session, this.getUserStartupCommands(), { traceErrors: false }); } diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index f4629e77e5..265fc9bc94 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { inject, injectable } from 'inversify'; +import { inject, injectable, optional } from 'inversify'; import { logger } from '../../../platform/logging'; import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; import { IConfigurationService, Resource, type ReadWrite } from '../../../platform/common/types'; @@ -18,6 +18,7 @@ import { IJupyterKernelSpec } from '../../types'; import { CancellationToken, Uri } from 'vscode'; import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; import { trackKernelResourceInformation } from '../../telemetry/helper'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; /** * Class used to fetch environment variables for a kernel. @@ -30,8 +31,17 @@ export class KernelEnvironmentVariablesService { @inject(IEnvironmentVariablesService) private readonly envVarsService: IEnvironmentVariablesService, @inject(ICustomEnvironmentVariablesProvider) private readonly customEnvVars: ICustomEnvironmentVariablesProvider, - @inject(IConfigurationService) private readonly configService: IConfigurationService - ) {} + @inject(IConfigurationService) private readonly configService: IConfigurationService, + @inject(SqlIntegrationEnvironmentVariablesProvider) + @optional() + private readonly sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider + ) { + logger.info( + `KernelEnvironmentVariablesService: Constructor called, sqlIntegrationEnvVars is ${ + sqlIntegrationEnvVars ? 'AVAILABLE' : 'UNDEFINED' + }` + ); + } /** * Generates the environment variables for the kernel. * @@ -51,6 +61,11 @@ export class KernelEnvironmentVariablesService { kernelSpec: IJupyterKernelSpec, token?: CancellationToken ) { + logger.info( + `KernelEnvVarsService.getEnvironmentVariables: Called for resource ${ + resource?.toString() || 'undefined' + }, sqlIntegrationEnvVars is ${this.sqlIntegrationEnvVars ? 'AVAILABLE' : 'UNDEFINED'}` + ); let kernelEnv = kernelSpec.env && Object.keys(kernelSpec.env).length > 0 ? (Object.assign({}, kernelSpec.env) as ReadWrite) @@ -68,7 +83,7 @@ export class KernelEnvironmentVariablesService { if (token?.isCancellationRequested) { return; } - let [customEnvVars, interpreterEnv] = await Promise.all([ + let [customEnvVars, interpreterEnv, sqlIntegrationEnvVars] = await Promise.all([ this.customEnvVars .getCustomEnvironmentVariables(resource, isPythonKernel ? 'RunPythonCode' : 'RunNonPythonCode', token) .catch(noop), @@ -82,6 +97,24 @@ export class KernelEnvironmentVariablesService { ); return undefined; }) + : undefined, + this.sqlIntegrationEnvVars + ? this.sqlIntegrationEnvVars + .getEnvironmentVariables(resource, token) + .then((vars) => { + if (vars && Object.keys(vars).length > 0) { + logger.info( + `KernelEnvVarsService: Got ${ + Object.keys(vars).length + } SQL integration env vars: ${Object.keys(vars).join(', ')}` + ); + } + return vars; + }) + .catch((ex) => { + logger.error('Failed to get SQL integration env variables for Kernel', ex); + return undefined; + }) : undefined ]); if (token?.isCancellationRequested) { @@ -123,6 +156,16 @@ export class KernelEnvironmentVariablesService { Object.assign(mergedVars, interpreterEnv, kernelEnv); // kernels vars win over interpreter. + // Merge SQL integration environment variables + if (sqlIntegrationEnvVars) { + logger.info( + `KernelEnvVarsService: Merging ${ + Object.keys(sqlIntegrationEnvVars).length + } SQL integration env vars into kernel env` + ); + Object.assign(mergedVars, sqlIntegrationEnvVars); + } + // If user asks us to, set PYTHONNOUSERSITE // For more details see here https://github.com/microsoft/vscode-jupyter/issues/8553#issuecomment-997144591 // https://docs.python.org/3/library/site.html#site.ENABLE_USER_SITE @@ -138,6 +181,16 @@ export class KernelEnvironmentVariablesService { // We can support this, however since this has not been requested, lets not do it.' this.envVarsService.mergeVariables(kernelEnv, mergedVars); // kernels vars win over interpreter. this.envVarsService.mergeVariables(customEnvVars, mergedVars); // custom vars win over all. + + // Merge SQL integration environment variables + if (sqlIntegrationEnvVars) { + logger.info( + `KernelEnvVarsService: Merging ${ + Object.keys(sqlIntegrationEnvVars).length + } SQL integration env vars into kernel env (non-python)` + ); + this.envVarsService.mergeVariables(sqlIntegrationEnvVars, mergedVars); + } } // env variables in kernelSpecs can contain variables that need to be substituted diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts index 74a8c5594f..80764fbe30 100644 --- a/src/kernels/serviceRegistry.node.ts +++ b/src/kernels/serviceRegistry.node.ts @@ -48,6 +48,7 @@ import { LastCellExecutionTracker } from './execution/lastCellExecutionTracker'; import { ClearJupyterServersCommand } from './jupyter/clearJupyterServersCommand'; import { KernelChatStartupCodeProvider } from './chat/kernelStartupCodeProvider'; import { KernelWorkingDirectory } from './raw/session/kernelWorkingDirectory.node'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { serviceManager.addSingleton(IExtensionSyncActivationService, Activation); @@ -62,6 +63,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea ); serviceManager.addSingleton(IRawKernelSessionFactory, RawKernelSessionFactory); serviceManager.addSingleton(IKernelLauncher, KernelLauncher); + serviceManager.addSingleton( + SqlIntegrationEnvironmentVariablesProvider, + SqlIntegrationEnvironmentVariablesProvider + ); serviceManager.addSingleton( KernelEnvironmentVariablesService, KernelEnvironmentVariablesService diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts new file mode 100644 index 0000000000..795c3e4727 --- /dev/null +++ b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts @@ -0,0 +1,187 @@ +import { inject, injectable } from 'inversify'; +import { CancellationToken, Event, EventEmitter, NotebookDocument, workspace } from 'vscode'; + +import { IDisposableRegistry, Resource } from '../../../platform/common/types'; +import { EnvironmentVariables } from '../../../platform/common/variables/types'; +import { logger } from '../../../platform/logging'; +import { IIntegrationStorage } from './types'; +import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationConfig, IntegrationType } from './integrationTypes'; + +/** + * Converts an integration ID to the environment variable name format expected by SQL blocks. + * Example: 'my-postgres-db' -> 'SQL_MY_POSTGRES_DB' + */ +function convertToEnvironmentVariableName(str: string): string { + return (/^\d/.test(str) ? `_${str}` : str).toUpperCase().replace(/[^\w]/g, '_'); +} + +function getSqlEnvVarName(integrationId: string): string { + return `SQL_${integrationId}`; +} + +/** + * Converts integration configuration to the JSON format expected by the SQL execution code. + */ +function convertIntegrationConfigToJson(config: IntegrationConfig): string { + switch (config.type) { + case IntegrationType.Postgres: + return JSON.stringify({ + type: 'postgres', + host: config.host, + port: config.port, + database: config.database, + username: config.username, + password: config.password, + ssl: config.ssl ?? false + }); + + case IntegrationType.BigQuery: + return JSON.stringify({ + type: 'bigquery', + project_id: config.projectId, + credentials: JSON.parse(config.credentials) // Parse the JSON string to an object + }); + + default: + throw new Error(`Unsupported integration type: ${(config as IntegrationConfig).type}`); + } +} + +/** + * Provides environment variables for SQL integrations. + * This service scans notebooks for SQL blocks and injects the necessary credentials + * as environment variables so they can be used during SQL block execution. + */ +@injectable() +export class SqlIntegrationEnvironmentVariablesProvider { + private readonly _onDidChangeEnvironmentVariables = new EventEmitter(); + + public readonly onDidChangeEnvironmentVariables: Event = this._onDidChangeEnvironmentVariables.event; + + constructor( + @inject(IIntegrationStorage) private readonly integrationStorage: IIntegrationStorage, + @inject(IDisposableRegistry) disposables: IDisposableRegistry + ) { + logger.info('SqlIntegrationEnvironmentVariablesProvider: Constructor called - provider is being instantiated'); + // Listen for changes to integration storage and fire change event + disposables.push( + this.integrationStorage.onDidChangeIntegrations(() => { + // Fire change event for all notebooks + this._onDidChangeEnvironmentVariables.fire(undefined); + }) + ); + } + + /** + * Get environment variables for SQL integrations used in the given notebook. + */ + public async getEnvironmentVariables( + resource: Resource, + _token?: CancellationToken + ): Promise { + const envVars: EnvironmentVariables = {}; + + if (!resource) { + return envVars; + } + + logger.info(`SqlIntegrationEnvironmentVariablesProvider: Getting env vars for resource ${resource.toString()}`); + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: Available notebooks: ${workspace.notebookDocuments + .map((nb) => nb.uri.toString()) + .join(', ')}` + ); + + // Find the notebook document for this resource + const notebook = workspace.notebookDocuments.find((nb) => nb.uri.toString() === resource.toString()); + if (!notebook) { + logger.warn(`SqlIntegrationEnvironmentVariablesProvider: No notebook found for ${resource.toString()}`); + return envVars; + } + + // Scan all cells for SQL integration IDs + const integrationIds = this.scanNotebookForIntegrations(notebook); + if (integrationIds.size === 0) { + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: No SQL integrations found in ${resource.toString()}` + ); + return envVars; + } + + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: Found ${integrationIds.size} SQL integrations: ${Array.from( + integrationIds + ).join(', ')}` + ); + + // Get credentials for each integration and add to environment variables + for (const integrationId of integrationIds) { + try { + const config = await this.integrationStorage.get(integrationId); + if (!config) { + logger.warn( + `SqlIntegrationEnvironmentVariablesProvider: No configuration found for integration ${integrationId}` + ); + continue; + } + + // Convert integration config to JSON and add as environment variable + const envVarName = convertToEnvironmentVariableName(getSqlEnvVarName(integrationId)); + const credentialsJson = convertIntegrationConfigToJson(config); + + envVars[envVarName] = credentialsJson; + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: Added env var ${envVarName} for integration ${integrationId}` + ); + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: Env var value: ${credentialsJson.substring(0, 100)}...` + ); + } catch (error) { + logger.error( + `SqlIntegrationEnvironmentVariablesProvider: Failed to get credentials for integration ${integrationId}`, + error + ); + } + } + + logger.info( + `SqlIntegrationEnvironmentVariablesProvider: Returning ${ + Object.keys(envVars).length + } env vars: ${Object.keys(envVars).join(', ')}` + ); + + return envVars; + } + + /** + * Scan a notebook for SQL integration IDs. + */ + private scanNotebookForIntegrations(notebook: NotebookDocument): Set { + const integrationIds = new Set(); + + for (const cell of notebook.getCells()) { + // Only check SQL cells + if (cell.document.languageId !== 'sql') { + continue; + } + + const metadata = cell.metadata; + if (metadata && typeof metadata === 'object') { + const integrationId = (metadata as Record).sql_integration_id; + if (typeof integrationId === 'string') { + // Skip the internal DuckDB integration + if (integrationId === DATAFRAME_SQL_INTEGRATION_ID) { + continue; + } + + integrationIds.add(integrationId); + logger.trace( + `SqlIntegrationEnvironmentVariablesProvider: Found integration ${integrationId} in cell ${cell.index}` + ); + } + } + } + + return integrationIds; + } +} diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts new file mode 100644 index 0000000000..00cd2daabb --- /dev/null +++ b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -0,0 +1,275 @@ +import { assert } from 'chai'; +import { instance, mock, when } from 'ts-mockito'; +import { EventEmitter, NotebookCell, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; + +import { IDisposableRegistry } from '../../../platform/common/types'; +import { IntegrationStorage } from './integrationStorage'; +import { SqlIntegrationEnvironmentVariablesProvider } from './sqlIntegrationEnvironmentVariablesProvider'; +import { IntegrationType, PostgresIntegrationConfig, BigQueryIntegrationConfig } from './integrationTypes'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../../test/vscode-mock'; + +suite('SqlIntegrationEnvironmentVariablesProvider', () => { + let provider: SqlIntegrationEnvironmentVariablesProvider; + let integrationStorage: IntegrationStorage; + let disposables: IDisposableRegistry; + + setup(() => { + resetVSCodeMocks(); + disposables = []; + integrationStorage = mock(IntegrationStorage); + when(integrationStorage.onDidChangeIntegrations).thenReturn(new EventEmitter().event); + + provider = new SqlIntegrationEnvironmentVariablesProvider(instance(integrationStorage), disposables); + }); + + teardown(() => { + disposables.forEach((d) => d.dispose()); + }); + + test('Returns empty object when resource is undefined', async () => { + const envVars = await provider.getEnvironmentVariables(undefined); + assert.deepStrictEqual(envVars, {}); + }); + + test('Returns empty object when notebook is not found', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + + const envVars = await provider.getEnvironmentVariables(uri); + assert.deepStrictEqual(envVars, {}); + }); + + test('Returns empty object when notebook has no SQL cells', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'python', 'print("hello")'), + createMockCell(1, NotebookCellKind.Markup, 'markdown', '# Title') + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const envVars = await provider.getEnvironmentVariables(uri); + assert.deepStrictEqual(envVars, {}); + }); + + test('Returns empty object when SQL cells have no integration ID', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', {}) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const envVars = await provider.getEnvironmentVariables(uri); + assert.deepStrictEqual(envVars, {}); + }); + + test('Skips internal DuckDB integration', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM df', { + sql_integration_id: 'deepnote-dataframe-sql' + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + + const envVars = await provider.getEnvironmentVariables(uri); + assert.deepStrictEqual(envVars, {}); + }); + + test('Returns environment variable for PostgreSQL integration', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'my-postgres-db'; + const config: PostgresIntegrationConfig = { + id: integrationId, + name: 'My Postgres DB', + type: IntegrationType.Postgres, + host: 'localhost', + port: 5432, + database: 'mydb', + username: 'user', + password: 'pass', + ssl: true + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Check that the environment variable is set + assert.property(envVars, 'SQL_MY_POSTGRES_DB'); + const credentialsJson = JSON.parse(envVars['SQL_MY_POSTGRES_DB']!); + assert.deepStrictEqual(credentialsJson, { + type: 'postgres', + host: 'localhost', + port: 5432, + database: 'mydb', + username: 'user', + password: 'pass', + ssl: true + }); + }); + + test('Returns environment variable for BigQuery integration', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'my-bigquery'; + const serviceAccountJson = JSON.stringify({ type: 'service_account', project_id: 'my-project' }); + const config: BigQueryIntegrationConfig = { + id: integrationId, + name: 'My BigQuery', + type: IntegrationType.BigQuery, + projectId: 'my-project', + credentials: serviceAccountJson + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM dataset.table', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Check that the environment variable is set + assert.property(envVars, 'SQL_MY_BIGQUERY'); + const credentialsJson = JSON.parse(envVars['SQL_MY_BIGQUERY']!); + assert.deepStrictEqual(credentialsJson, { + type: 'bigquery', + project_id: 'my-project', + credentials: { type: 'service_account', project_id: 'my-project' } + }); + }); + + test('Handles multiple SQL cells with same integration', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'my-postgres-db'; + const config: PostgresIntegrationConfig = { + id: integrationId, + name: 'My Postgres DB', + type: IntegrationType.Postgres, + host: 'localhost', + port: 5432, + database: 'mydb', + username: 'user', + password: 'pass' + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', { + sql_integration_id: integrationId + }), + createMockCell(1, NotebookCellKind.Code, 'sql', 'SELECT * FROM orders', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Should only have one environment variable + assert.property(envVars, 'SQL_MY_POSTGRES_DB'); + assert.strictEqual(Object.keys(envVars).length, 1); + }); + + test('Handles multiple SQL cells with different integrations', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const postgresId = 'my-postgres-db'; + const bigqueryId = 'my-bigquery'; + + const postgresConfig: PostgresIntegrationConfig = { + id: postgresId, + name: 'My Postgres DB', + type: IntegrationType.Postgres, + host: 'localhost', + port: 5432, + database: 'mydb', + username: 'user', + password: 'pass' + }; + + const bigqueryConfig: BigQueryIntegrationConfig = { + id: bigqueryId, + name: 'My BigQuery', + type: IntegrationType.BigQuery, + projectId: 'my-project', + credentials: JSON.stringify({ type: 'service_account' }) + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', { + sql_integration_id: postgresId + }), + createMockCell(1, NotebookCellKind.Code, 'sql', 'SELECT * FROM dataset.table', { + sql_integration_id: bigqueryId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(postgresId)).thenResolve(postgresConfig); + when(integrationStorage.get(bigqueryId)).thenResolve(bigqueryConfig); + + const envVars = await provider.getEnvironmentVariables(uri); + + // 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); + }); + + test('Handles missing integration configuration gracefully', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'missing-integration'; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(undefined); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Should return empty object when integration config is missing + assert.deepStrictEqual(envVars, {}); + }); +}); + +function createMockNotebook(uri: Uri, cells: NotebookCell[]): NotebookDocument { + return { + uri, + getCells: () => cells + } as NotebookDocument; +} + +function createMockCell( + index: number, + kind: NotebookCellKind, + languageId: string, + value: string, + metadata?: Record +): NotebookCell { + return { + index, + kind, + document: { + languageId, + getText: () => value + }, + metadata: metadata || {} + } as NotebookCell; +} diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts new file mode 100644 index 0000000000..2591e2d23c --- /dev/null +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { inject, injectable } from 'inversify'; +import { IStartupCodeProvider, IStartupCodeProviders, StartupCodePriority, IKernel } from '../../../kernels/types'; +import { JupyterNotebookView } from '../../../platform/common/constants'; +import { IExtensionSyncActivationService } from '../../../platform/activation/types'; +import { SqlIntegrationEnvironmentVariablesProvider } from './sqlIntegrationEnvironmentVariablesProvider'; +import { logger } from '../../../platform/logging'; +import { isPythonKernelConnection } from '../../../kernels/helpers'; +import { DEEPNOTE_NOTEBOOK_TYPE } from '../../../kernels/deepnote/types'; +import { workspace } from 'vscode'; + +/** + * Provides startup code to inject SQL integration credentials into the kernel environment. + * This is necessary because Jupyter doesn't automatically pass all environment variables + * from the server process to the kernel process. + */ +@injectable() +export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, IExtensionSyncActivationService { + public priority = StartupCodePriority.Base; + + constructor( + @inject(IStartupCodeProviders) private readonly registry: IStartupCodeProviders, + @inject(SqlIntegrationEnvironmentVariablesProvider) + private readonly envVarsProvider: SqlIntegrationEnvironmentVariablesProvider + ) {} + + activate(): void { + logger.info('SqlIntegrationStartupCodeProvider: Activating and registering with JupyterNotebookView'); + this.registry.register(this, JupyterNotebookView); + logger.info('SqlIntegrationStartupCodeProvider: Successfully registered'); + } + + async getCode(kernel: IKernel): Promise { + logger.info( + `SqlIntegrationStartupCodeProvider.getCode called for kernel ${ + kernel.id + }, resourceUri: ${kernel.resourceUri?.toString()}` + ); + + // Only run for Python kernels on Deepnote notebooks + if (!isPythonKernelConnection(kernel.kernelConnectionMetadata)) { + logger.info(`SqlIntegrationStartupCodeProvider: Not a Python kernel, skipping`); + return []; + } + + // Check if this is a Deepnote notebook + if (!kernel.resourceUri) { + logger.info(`SqlIntegrationStartupCodeProvider: No resourceUri, skipping`); + return []; + } + + const notebook = workspace.notebookDocuments.find((nb) => nb.uri.toString() === kernel.resourceUri?.toString()); + if (!notebook) { + logger.info(`SqlIntegrationStartupCodeProvider: Notebook not found for ${kernel.resourceUri.toString()}`); + return []; + } + + logger.info(`SqlIntegrationStartupCodeProvider: Found notebook with type: ${notebook.notebookType}`); + + if (notebook.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) { + logger.info(`SqlIntegrationStartupCodeProvider: Not a Deepnote notebook, skipping`); + return []; + } + + try { + // Get SQL integration environment variables for this notebook + const envVars = await this.envVarsProvider.getEnvironmentVariables(kernel.resourceUri); + + if (!envVars || Object.keys(envVars).length === 0) { + logger.trace( + `SqlIntegrationStartupCodeProvider: No SQL integration env vars for ${kernel.resourceUri.toString()}` + ); + return []; + } + + logger.info( + `SqlIntegrationStartupCodeProvider: Injecting ${ + Object.keys(envVars).length + } SQL integration env vars into kernel: ${Object.keys(envVars).join(', ')}` + ); + + // Generate Python code to set environment variables directly in os.environ + const code: string[] = []; + + code.push('try:'); + code.push(' import os'); + code.push(` # [SQL Integration] Setting ${Object.keys(envVars).length} SQL integration env vars...`); + + // Set each environment variable directly in os.environ + for (const [key, value] of Object.entries(envVars)) { + if (value) { + // Use JSON.stringify to properly escape the value + const jsonEscaped = JSON.stringify(value); + code.push(` os.environ['${key}'] = ${jsonEscaped}`); + } + } + + code.push( + ` # [SQL Integration] Successfully set ${Object.keys(envVars).length} SQL integration env vars` + ); + code.push('except Exception as e:'); + code.push(' import traceback'); + code.push(' print(f"[SQL Integration] ERROR: Failed to set SQL integration env vars: {e}")'); + code.push(' traceback.print_exc()'); + + const fullCode = code.join('\n'); + logger.info(`SqlIntegrationStartupCodeProvider: Generated startup code (${fullCode.length} chars):`); + logger.info(fullCode); + + return code; + } catch (error) { + logger.error('SqlIntegrationStartupCodeProvider: Failed to generate startup code', error); + return []; + } + } +} diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index a42308f4f4..ea981eded5 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -66,6 +66,7 @@ import { DeepnoteKernelAutoSelector } from './deepnote/deepnoteKernelAutoSelecto import { DeepnoteServerProvider } from '../kernels/deepnote/deepnoteServerProvider.node'; import { DeepnoteInitNotebookRunner, IDeepnoteInitNotebookRunner } from './deepnote/deepnoteInitNotebookRunner.node'; import { DeepnoteRequirementsHelper, IDeepnoteRequirementsHelper } from './deepnote/deepnoteRequirementsHelper.node'; +import { SqlIntegrationStartupCodeProvider } from './deepnote/integrations/sqlIntegrationStartupCodeProvider'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { registerControllerTypes(serviceManager, isDevMode); @@ -147,6 +148,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea IExtensionSyncActivationService, SqlCellStatusBarProvider ); + serviceManager.addSingleton( + IExtensionSyncActivationService, + SqlIntegrationStartupCodeProvider + ); // Deepnote kernel services serviceManager.addSingleton(IDeepnoteToolkitInstaller, DeepnoteToolkitInstaller); From c5d47b1b35278b941d398cbd9e979189f5b4a7b4 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 13:06:58 +0200 Subject: [PATCH 02/43] fix: convert SQL integration credentials to normalized format for toolkit --- ...IntegrationEnvironmentVariablesProvider.ts | 38 +++++++++++++------ ...nEnvironmentVariablesProvider.unit.test.ts | 17 +++------ 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts index 795c3e4727..1293102f1f 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts @@ -21,26 +21,40 @@ function getSqlEnvVarName(integrationId: string): string { /** * Converts integration configuration to the JSON format expected by the SQL execution code. + * The format must match what deepnote_toolkit expects: + * { + * "url": "sqlalchemy_connection_url", + * "params": {}, + * "param_style": "qmark" | "format" | etc. + * } */ function convertIntegrationConfigToJson(config: IntegrationConfig): string { switch (config.type) { - case IntegrationType.Postgres: + case IntegrationType.Postgres: { + // Build PostgreSQL connection URL + // Format: postgresql://username:password@host:port/database + const encodedUsername = encodeURIComponent(config.username); + const encodedPassword = encodeURIComponent(config.password); + const url = `postgresql://${encodedUsername}:${encodedPassword}@${config.host}:${config.port}/${config.database}`; + return JSON.stringify({ - type: 'postgres', - host: config.host, - port: config.port, - database: config.database, - username: config.username, - password: config.password, - ssl: config.ssl ?? false + url: url, + params: config.ssl ? { sslmode: 'require' } : {}, + param_style: 'format' }); + } - case IntegrationType.BigQuery: + case IntegrationType.BigQuery: { + // BigQuery uses a special URL format return JSON.stringify({ - type: 'bigquery', - project_id: config.projectId, - credentials: JSON.parse(config.credentials) // Parse the JSON string to an object + url: 'bigquery://?user_supplied_client=true', + params: { + project_id: config.projectId, + credentials: JSON.parse(config.credentials) + }, + param_style: 'format' }); + } default: throw new Error(`Unsupported integration type: ${(config as IntegrationConfig).type}`); diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index 00cd2daabb..d7d444af2b 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -107,15 +107,9 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Check that the environment variable is set assert.property(envVars, 'SQL_MY_POSTGRES_DB'); const credentialsJson = JSON.parse(envVars['SQL_MY_POSTGRES_DB']!); - assert.deepStrictEqual(credentialsJson, { - type: 'postgres', - host: 'localhost', - port: 5432, - database: 'mydb', - username: 'user', - password: 'pass', - ssl: true - }); + assert.strictEqual(credentialsJson.url, 'postgresql://user:pass@localhost:5432/mydb'); + assert.deepStrictEqual(credentialsJson.params, { sslmode: 'require' }); + assert.strictEqual(credentialsJson.param_style, 'format'); }); test('Returns environment variable for BigQuery integration', async () => { @@ -144,11 +138,12 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Check that the environment variable is set assert.property(envVars, 'SQL_MY_BIGQUERY'); const credentialsJson = JSON.parse(envVars['SQL_MY_BIGQUERY']!); - assert.deepStrictEqual(credentialsJson, { - type: 'bigquery', + assert.strictEqual(credentialsJson.url, 'bigquery://?user_supplied_client=true'); + assert.deepStrictEqual(credentialsJson.params, { project_id: 'my-project', credentials: { type: 'service_account', project_id: 'my-project' } }); + assert.strictEqual(credentialsJson.param_style, 'format'); }); test('Handles multiple SQL cells with same integration', async () => { From d6a33201f74de95f04bdf3177bcf3d69534812a7 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:01:53 +0200 Subject: [PATCH 03/43] document integration credential usage --- INTEGRATIONS_CREDENTIALS.md | 381 ++++++++++++++++++++++++++++++++++++ 1 file changed, 381 insertions(+) create mode 100644 INTEGRATIONS_CREDENTIALS.md diff --git a/INTEGRATIONS_CREDENTIALS.md b/INTEGRATIONS_CREDENTIALS.md new file mode 100644 index 0000000000..c2b2784a83 --- /dev/null +++ b/INTEGRATIONS_CREDENTIALS.md @@ -0,0 +1,381 @@ +# Deepnote Integrations & Credentials System + +## Overview + +The integrations system enables Deepnote notebooks to connect to external data sources (PostgreSQL, BigQuery, etc.) by securely managing credentials and exposing them to SQL blocks. The system handles: + +1. **Credential Storage**: Secure storage using VSCode's SecretStorage API +2. **Integration Detection**: Automatic discovery of integrations used in notebooks +3. **UI Management**: Webview-based configuration interface +4. **Kernel Integration**: Injection of credentials into Jupyter kernel environment +5. **Toolkit Exposure**: Making credentials available to `deepnote-toolkit` for SQL execution + +## Architecture + +### Core Components + +#### 1. **Integration Storage** (`integrationStorage.ts`) + +Manages persistent storage of integration configurations using VSCode's encrypted SecretStorage API. + +**Key Features:** +- Uses VSCode's `SecretStorage` API for secure credential storage +- Storage is scoped to the user's machine (shared across all Deepnote projects) +- In-memory caching for performance +- Event-driven updates via `onDidChangeIntegrations` event +- Index-based storage for efficient retrieval + +**Storage Format:** +- Each integration config is stored as JSON under key: `deepnote-integrations.{integrationId}` +- An index is maintained at key: `deepnote-integrations.index` containing all integration IDs + +**Key Methods:** +- `getAll()`: Retrieve all stored integration configurations +- `get(integrationId)`: Get a specific integration by ID +- `save(config)`: Save or update an integration configuration +- `delete(integrationId)`: Remove an integration configuration +- `exists(integrationId)`: Check if an integration is configured + +**Integration Config Types:** + +```typescript +// PostgreSQL +{ + id: string; + name: string; + type: 'postgres'; + host: string; + port: number; + database: string; + username: string; + password: string; + ssl?: boolean; +} + +// BigQuery +{ + id: string; + name: string; + type: 'bigquery'; + projectId: string; + credentials: string; // JSON string of service account credentials +} +``` + +#### 2. **Integration Detector** (`integrationDetector.ts`) + +Scans Deepnote projects to discover which integrations are used in SQL blocks. + +**Detection Process:** +1. Retrieves the Deepnote project from `IDeepnoteNotebookManager` +2. Scans all notebooks in the project +3. Examines each code block for `metadata.sql_integration_id` +4. Checks if each integration is configured (has credentials) +5. Returns a map of integration IDs to their status + +**Integration Status:** +- `Connected`: Integration has valid credentials stored +- `Disconnected`: Integration is used but not configured +- `Error`: Integration configuration is invalid + +**Special Cases:** +- Excludes `deepnote-dataframe-sql` (internal DuckDB integration) +- Only processes code blocks with SQL integration metadata + +#### 3. **Integration Manager** (`integrationManager.ts`) + +Orchestrates the integration management UI and commands. + +**Responsibilities:** +- Registers the `deepnote.manageIntegrations` command +- Updates VSCode context keys for UI visibility: + - `deepnote.hasIntegrations`: True if any integrations are detected + - `deepnote.hasUnconfiguredIntegrations`: True if any integrations lack credentials +- Handles notebook selection changes +- Opens the integration webview with detected integrations + +**Command Flow:** +1. User triggers command (from command palette or SQL cell status bar) +2. Manager detects integrations in the active notebook +3. Manager opens webview with integration list +4. Optionally pre-selects a specific integration for configuration + +#### 4. **Integration Webview** (`integrationWebview.ts`) + +Provides the webview-based UI for managing integration credentials. + +**Features:** +- Persistent webview panel (survives defocus) +- Real-time integration status updates +- Configuration forms for each integration type +- Delete/reset functionality + +**Message Protocol:** + +Extension → Webview: +```typescript +// Update integration list +{ type: 'update', integrations: IntegrationWithStatus[] } + +// Show configuration form +{ type: 'showForm', integrationId: string, config: IntegrationConfig | null } + +// Status messages +{ type: 'success' | 'error', message: string } +``` + +Webview → Extension: +```typescript +// Save configuration +{ type: 'save', integrationId: string, config: IntegrationConfig } + +// Delete configuration +{ type: 'delete', integrationId: string } + +// Request configuration form +{ type: 'configure', integrationId: string } +``` + +### UI Components (React) + +#### 5. **Integration Panel** (`IntegrationPanel.tsx`) + +Main React component that manages the webview UI state. + +**State Management:** +- `integrations`: List of detected integrations with status +- `selectedIntegrationId`: Currently selected integration for configuration +- `selectedConfig`: Existing configuration being edited +- `message`: Success/error messages +- `confirmDelete`: Confirmation state for deletion + +**User Flows:** + +**Configure Integration:** +1. User clicks "Configure" button +2. Panel shows configuration form overlay +3. User enters credentials +4. Panel sends save message to extension +5. Extension stores credentials and updates status +6. Panel shows success message and refreshes list + +**Delete Integration:** +1. User clicks "Reset" button +2. Panel shows confirmation prompt (5 seconds) +3. User clicks again to confirm +4. Panel sends delete message to extension +5. Extension removes credentials +6. Panel updates status to "Disconnected" + +#### 6. **Configuration Forms** (`PostgresForm.tsx`, `BigQueryForm.tsx`) + +Type-specific forms for entering integration credentials. + +**PostgreSQL Form Fields:** +- Name (display name) +- Host +- Port (default: 5432) +- Database +- Username +- Password +- SSL (checkbox) + +**BigQuery Form Fields:** +- Name (display name) +- Project ID +- Service Account Credentials (JSON textarea) + +**Validation:** +- All fields are required +- BigQuery credentials must be valid JSON +- Port must be a valid number + +### Kernel Integration + +#### 7. **SQL Integration Environment Variables Provider** (`sqlIntegrationEnvironmentVariablesProvider.ts`) + +Provides environment variables containing integration credentials for the Jupyter kernel. + +**Process:** +1. Scans the notebook for SQL cells with `sql_integration_id` metadata +2. Retrieves credentials for each detected integration +3. Converts credentials to the format expected by `deepnote-toolkit` +4. Returns environment variables to be injected into the kernel process + +**Environment Variable Format:** + +Variable name: `SQL_{INTEGRATION_ID}` (uppercased, special chars replaced with `_`) + +Example: Integration ID `my-postgres-db` → Environment variable `SQL_MY_POSTGRES_DB` + +**Credential JSON Format:** + +PostgreSQL: +```json +{ + "url": "postgresql://username:password@host:port/database", + "params": { "sslmode": "require" }, + "param_style": "format" +} +``` + +BigQuery: +```json +{ + "url": "bigquery://?user_supplied_client=true", + "params": { + "project_id": "my-project", + "credentials": { /* service account JSON */ } + }, + "param_style": "format" +} +``` + +**Integration Points:** +- Registered as an environment variable provider in the kernel environment service +- Called when starting a Jupyter kernel for a Deepnote notebook +- Environment variables are passed to the kernel process at startup + +#### 8. **SQL Integration Startup Code Provider** (`sqlIntegrationStartupCodeProvider.ts`) + +Injects Python code into the kernel at startup to set environment variables. + +**Why This Is Needed:** +Jupyter doesn't automatically pass all environment variables from the server process to the kernel process. This provider ensures credentials are available in the kernel's `os.environ`. + +**Generated Code:** +```python +try: + import os + # [SQL Integration] Setting N SQL integration env vars... + os.environ['SQL_MY_POSTGRES_DB'] = '{"url":"postgresql://...","params":{},"param_style":"format"}' + os.environ['SQL_MY_BIGQUERY'] = '{"url":"bigquery://...","params":{...},"param_style":"format"}' + # [SQL Integration] Successfully set N SQL integration env vars +except Exception as e: + import traceback + print(f"[SQL Integration] ERROR: Failed to set SQL integration env vars: {e}") + traceback.print_exc() +``` + +**Execution:** +- Registered with `IStartupCodeProviders` for `JupyterNotebookView` +- Runs automatically when a Python kernel starts for a Deepnote notebook +- Priority: `StartupCodePriority.Base` (runs early) +- Only runs for Python kernels on Deepnote notebooks + +### Toolkit Integration + +#### 9. **How Credentials Are Exposed to deepnote-toolkit** + +The `deepnote-toolkit` Python package reads credentials from environment variables to execute SQL blocks. + +**Flow:** +1. Extension detects SQL blocks in notebook +2. Extension retrieves credentials from secure storage +3. Extension converts credentials to JSON format +4. Extension injects credentials as environment variables (two methods): + - **Server Process**: Via `SqlIntegrationEnvironmentVariablesProvider` when starting Jupyter server + - **Kernel Process**: Via `SqlIntegrationStartupCodeProvider` when starting Python kernel +5. `deepnote-toolkit` reads environment variables when executing SQL blocks +6. Toolkit creates database connections using the credentials +7. Toolkit executes SQL queries and returns results + +**Environment Variable Lookup:** +When a SQL block with `sql_integration_id: "my-postgres-db"` is executed: +1. Toolkit looks for environment variable `SQL_MY_POSTGRES_DB` +2. Toolkit parses the JSON value +3. Toolkit creates a SQLAlchemy connection using the `url` and `params` +4. Toolkit executes the SQL query +5. Toolkit returns results as a pandas DataFrame + +## Data Flow + +### Configuration Flow +``` +User → IntegrationPanel (UI) + → vscodeApi.postMessage({ type: 'save', config }) + → IntegrationWebviewProvider.onMessage() + → IntegrationStorage.save(config) + → EncryptedStorage.store() [VSCode SecretStorage API] + → IntegrationStorage fires onDidChangeIntegrations event + → SqlIntegrationEnvironmentVariablesProvider fires onDidChangeEnvironmentVariables event +``` + +### Execution Flow +``` +User executes SQL cell + → Kernel startup triggered + → SqlIntegrationEnvironmentVariablesProvider.getEnvironmentVariables() + → Scans notebook for SQL cells + → Retrieves credentials from IntegrationStorage + → Converts to JSON format + → Returns environment variables + → Environment variables passed to Jupyter server process + → SqlIntegrationStartupCodeProvider.getCode() + → Generates Python code to set os.environ + → Startup code executed in kernel + → deepnote-toolkit reads os.environ['SQL_*'] + → Toolkit executes SQL query + → Results returned to notebook +``` + +## Security Considerations + +1. **Encrypted Storage**: All credentials are stored using VSCode's SecretStorage API, which uses the OS keychain +2. **No Plaintext**: Credentials are never written to disk in plaintext +3. **Scoped Access**: Storage is scoped to the VSCode extension +4. **Environment Isolation**: Each notebook gets only the credentials it needs +5. **No Logging**: Credential values are not logged (only first 100 chars for debugging) + +## Adding New Integration Types + +To add a new integration type (e.g., MySQL, Snowflake): + +1. **Add type to `integrationTypes.ts`**: + ```typescript + export enum IntegrationType { + Postgres = 'postgres', + BigQuery = 'bigquery', + MySQL = 'mysql' // New type + } + + export interface MySQLIntegrationConfig extends BaseIntegrationConfig { + type: IntegrationType.MySQL; + host: string; + port: number; + database: string; + username: string; + password: string; + } + + export type IntegrationConfig = PostgresIntegrationConfig | BigQueryIntegrationConfig | MySQLIntegrationConfig; + ``` + +2. **Add conversion logic in `sqlIntegrationEnvironmentVariablesProvider.ts`**: + ```typescript + case IntegrationType.MySQL: { + const url = `mysql://${config.username}:${config.password}@${config.host}:${config.port}/${config.database}`; + return JSON.stringify({ url, params: {}, param_style: 'format' }); + } + ``` + +3. **Create UI form component** (`MySQLForm.tsx`) + +4. **Update `ConfigurationForm.tsx`** to render the new form + +5. **Update webview types** (`src/webviews/webview-side/integrations/types.ts`) + +6. **Add localization strings** for the new integration type + +## Testing + +Unit tests are located in: +- `sqlIntegrationEnvironmentVariablesProvider.unit.test.ts` + +Tests cover: +- Environment variable generation for each integration type +- Multiple integrations in a single notebook +- Missing credentials handling +- Integration ID to environment variable name conversion +- JSON format validation + From c04c276851176711ca5c7980bdb845120004a11d Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:19:48 +0200 Subject: [PATCH 04/43] refactor: move integration storage logic to platform for proper import limits --- .../deepnote/deepnoteServerStarter.node.ts | 2 +- .../raw/launcher/kernelEnvVarsService.node.ts | 2 +- src/kernels/serviceRegistry.node.ts | 2 +- .../integrations/integrationDetector.ts | 2 +- .../integrations/integrationManager.ts | 2 +- .../deepnote/integrations/integrationUtils.ts | 6 ++++- .../integrations/integrationWebview.ts | 6 ++++- .../sqlIntegrationStartupCodeProvider.ts | 2 +- src/notebooks/deepnote/integrations/types.ts | 26 +++---------------- .../deepnote/sqlCellStatusBarProvider.ts | 2 +- .../sqlCellStatusBarProvider.unit.test.ts | 2 +- src/notebooks/serviceRegistry.node.ts | 2 +- src/notebooks/serviceRegistry.web.ts | 2 +- .../notebooks/deepnote}/integrationStorage.ts | 6 ++--- .../notebooks/deepnote}/integrationTypes.ts | 0 ...IntegrationEnvironmentVariablesProvider.ts | 6 ++--- ...nEnvironmentVariablesProvider.unit.test.ts | 2 +- src/platform/notebooks/deepnote/types.ts | 25 ++++++++++++++++++ 18 files changed, 55 insertions(+), 42 deletions(-) rename src/{notebooks/deepnote/integrations => platform/notebooks/deepnote}/integrationStorage.ts (96%) rename src/{notebooks/deepnote/integrations => platform/notebooks/deepnote}/integrationTypes.ts (100%) rename src/{notebooks/deepnote/integrations => platform/notebooks/deepnote}/sqlIntegrationEnvironmentVariablesProvider.ts (97%) rename src/{notebooks/deepnote/integrations => platform/notebooks/deepnote}/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts (99%) create mode 100644 src/platform/notebooks/deepnote/types.ts diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 07ac0d2fad..c44300bb05 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -17,7 +17,7 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from '../../platform/vscode-path/path'; import { generateUuid } from '../../platform/common/uuid'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; /** * Lock file data structure for tracking server ownership diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 265fc9bc94..1d7740d657 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -18,7 +18,7 @@ import { IJupyterKernelSpec } from '../../types'; import { CancellationToken, Uri } from 'vscode'; import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; import { trackKernelResourceInformation } from '../../telemetry/helper'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; /** * Class used to fetch environment variables for a kernel. diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts index 80764fbe30..293f19403d 100644 --- a/src/kernels/serviceRegistry.node.ts +++ b/src/kernels/serviceRegistry.node.ts @@ -48,7 +48,7 @@ import { LastCellExecutionTracker } from './execution/lastCellExecutionTracker'; import { ClearJupyterServersCommand } from './jupyter/clearJupyterServersCommand'; import { KernelChatStartupCodeProvider } from './chat/kernelStartupCodeProvider'; import { KernelWorkingDirectory } from './raw/session/kernelWorkingDirectory.node'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { serviceManager.addSingleton(IExtensionSyncActivationService, Activation); diff --git a/src/notebooks/deepnote/integrations/integrationDetector.ts b/src/notebooks/deepnote/integrations/integrationDetector.ts index 974b4a24fd..dac3d044d4 100644 --- a/src/notebooks/deepnote/integrations/integrationDetector.ts +++ b/src/notebooks/deepnote/integrations/integrationDetector.ts @@ -2,7 +2,7 @@ import { inject, injectable } from 'inversify'; import { logger } from '../../../platform/logging'; import { IDeepnoteNotebookManager } from '../../types'; -import { IntegrationStatus, IntegrationWithStatus } from './integrationTypes'; +import { IntegrationStatus, IntegrationWithStatus } from '../../../platform/notebooks/deepnote/integrationTypes'; import { IIntegrationDetector, IIntegrationStorage } from './types'; import { BlockWithIntegration, scanBlocksForIntegrations } from './integrationUtils'; diff --git a/src/notebooks/deepnote/integrations/integrationManager.ts b/src/notebooks/deepnote/integrations/integrationManager.ts index 87ab98d905..188bc66318 100644 --- a/src/notebooks/deepnote/integrations/integrationManager.ts +++ b/src/notebooks/deepnote/integrations/integrationManager.ts @@ -5,7 +5,7 @@ import { IExtensionContext } from '../../../platform/common/types'; import { Commands } from '../../../platform/common/constants'; import { logger } from '../../../platform/logging'; import { IIntegrationDetector, IIntegrationManager, IIntegrationStorage, IIntegrationWebviewProvider } from './types'; -import { IntegrationStatus, IntegrationWithStatus } from './integrationTypes'; +import { IntegrationStatus, IntegrationWithStatus } from '../../../platform/notebooks/deepnote/integrationTypes'; import { BlockWithIntegration, scanBlocksForIntegrations } from './integrationUtils'; /** diff --git a/src/notebooks/deepnote/integrations/integrationUtils.ts b/src/notebooks/deepnote/integrations/integrationUtils.ts index 01b62b081e..2a0589dbd0 100644 --- a/src/notebooks/deepnote/integrations/integrationUtils.ts +++ b/src/notebooks/deepnote/integrations/integrationUtils.ts @@ -1,6 +1,10 @@ import { logger } from '../../../platform/logging'; import { IIntegrationStorage } from './types'; -import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationStatus, IntegrationWithStatus } from './integrationTypes'; +import { + DATAFRAME_SQL_INTEGRATION_ID, + IntegrationStatus, + IntegrationWithStatus +} from '../../../platform/notebooks/deepnote/integrationTypes'; /** * Represents a block with SQL integration metadata diff --git a/src/notebooks/deepnote/integrations/integrationWebview.ts b/src/notebooks/deepnote/integrations/integrationWebview.ts index 09c2134369..bbcbd27660 100644 --- a/src/notebooks/deepnote/integrations/integrationWebview.ts +++ b/src/notebooks/deepnote/integrations/integrationWebview.ts @@ -6,7 +6,11 @@ import * as localize from '../../../platform/common/utils/localize'; import { logger } from '../../../platform/logging'; import { LocalizedMessages, SharedMessages } from '../../../messageTypes'; import { IIntegrationStorage, IIntegrationWebviewProvider } from './types'; -import { IntegrationConfig, IntegrationStatus, IntegrationWithStatus } from './integrationTypes'; +import { + IntegrationConfig, + IntegrationStatus, + IntegrationWithStatus +} from '../../../platform/notebooks/deepnote/integrationTypes'; /** * Manages the webview panel for integration configuration diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts index 2591e2d23c..ef866033d9 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { IStartupCodeProvider, IStartupCodeProviders, StartupCodePriority, IKernel } from '../../../kernels/types'; import { JupyterNotebookView } from '../../../platform/common/constants'; import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -import { SqlIntegrationEnvironmentVariablesProvider } from './sqlIntegrationEnvironmentVariablesProvider'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; import { logger } from '../../../platform/logging'; import { isPythonKernelConnection } from '../../../kernels/helpers'; import { DEEPNOTE_NOTEBOOK_TYPE } from '../../../kernels/deepnote/types'; diff --git a/src/notebooks/deepnote/integrations/types.ts b/src/notebooks/deepnote/integrations/types.ts index f414e34583..254f3c8912 100644 --- a/src/notebooks/deepnote/integrations/types.ts +++ b/src/notebooks/deepnote/integrations/types.ts @@ -1,27 +1,7 @@ -import { Event } from 'vscode'; -import { IDisposable } from '../../../platform/common/types'; -import { IntegrationConfig, IntegrationWithStatus } from './integrationTypes'; +import { IntegrationWithStatus } from '../../../platform/notebooks/deepnote/integrationTypes'; -export const IIntegrationStorage = Symbol('IIntegrationStorage'); -export interface IIntegrationStorage extends IDisposable { - /** - * Event fired when integrations change - */ - readonly onDidChangeIntegrations: Event; - - getAll(): Promise; - get(integrationId: string): Promise; - - /** - * Get integration configuration for a specific project and integration - */ - getIntegrationConfig(projectId: string, integrationId: string): Promise; - - save(config: IntegrationConfig): Promise; - delete(integrationId: string): Promise; - exists(integrationId: string): Promise; - clear(): Promise; -} +// Re-export IIntegrationStorage from platform layer +export { IIntegrationStorage } from '../../../platform/notebooks/deepnote/types'; export const IIntegrationDetector = Symbol('IIntegrationDetector'); export interface IIntegrationDetector { diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts index 80740260a9..fddb299b9e 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts @@ -15,7 +15,7 @@ import { IExtensionSyncActivationService } from '../../platform/activation/types import { IDisposableRegistry } from '../../platform/common/types'; import { Commands } from '../../platform/common/constants'; import { IIntegrationStorage } from './integrations/types'; -import { DATAFRAME_SQL_INTEGRATION_ID } from './integrations/integrationTypes'; +import { DATAFRAME_SQL_INTEGRATION_ID } from '../../platform/notebooks/deepnote/integrationTypes'; /** * Provides status bar items for SQL cells showing the integration name diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts index 1949be6712..bae3854aaf 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts @@ -13,7 +13,7 @@ import { import { IDisposableRegistry } from '../../platform/common/types'; import { IIntegrationStorage } from './integrations/types'; import { SqlCellStatusBarProvider } from './sqlCellStatusBarProvider'; -import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationType } from './integrations/integrationTypes'; +import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationType } from '../../platform/notebooks/deepnote/integrationTypes'; suite('SqlCellStatusBarProvider', () => { let provider: SqlCellStatusBarProvider; diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index ea981eded5..3bd4f9d236 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -43,7 +43,7 @@ import { INotebookEditorProvider, INotebookPythonEnvironmentService } from './ty import { DeepnoteActivationService } from './deepnote/deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnote/deepnoteNotebookManager'; import { IDeepnoteNotebookManager } from './types'; -import { IntegrationStorage } from './deepnote/integrations/integrationStorage'; +import { IntegrationStorage } from '../platform/notebooks/deepnote/integrationStorage'; import { IntegrationDetector } from './deepnote/integrations/integrationDetector'; import { IntegrationManager } from './deepnote/integrations/integrationManager'; import { IntegrationWebviewProvider } from './deepnote/integrations/integrationWebview'; diff --git a/src/notebooks/serviceRegistry.web.ts b/src/notebooks/serviceRegistry.web.ts index 1fb50d004a..dc67a169af 100644 --- a/src/notebooks/serviceRegistry.web.ts +++ b/src/notebooks/serviceRegistry.web.ts @@ -38,7 +38,7 @@ import { INotebookEditorProvider, INotebookPythonEnvironmentService } from './ty import { DeepnoteActivationService } from './deepnote/deepnoteActivationService'; import { DeepnoteNotebookManager } from './deepnote/deepnoteNotebookManager'; import { IDeepnoteNotebookManager } from './types'; -import { IntegrationStorage } from './deepnote/integrations/integrationStorage'; +import { IntegrationStorage } from '../platform/notebooks/deepnote/integrationStorage'; import { IntegrationDetector } from './deepnote/integrations/integrationDetector'; import { IntegrationManager } from './deepnote/integrations/integrationManager'; import { IntegrationWebviewProvider } from './deepnote/integrations/integrationWebview'; diff --git a/src/notebooks/deepnote/integrations/integrationStorage.ts b/src/platform/notebooks/deepnote/integrationStorage.ts similarity index 96% rename from src/notebooks/deepnote/integrations/integrationStorage.ts rename to src/platform/notebooks/deepnote/integrationStorage.ts index 17a8a2b15c..4319b395d1 100644 --- a/src/notebooks/deepnote/integrations/integrationStorage.ts +++ b/src/platform/notebooks/deepnote/integrationStorage.ts @@ -1,9 +1,9 @@ import { inject, injectable } from 'inversify'; import { EventEmitter } from 'vscode'; -import { IEncryptedStorage } from '../../../platform/common/application/types'; -import { IAsyncDisposableRegistry } from '../../../platform/common/types'; -import { logger } from '../../../platform/logging'; +import { IEncryptedStorage } from '../../common/application/types'; +import { IAsyncDisposableRegistry } from '../../common/types'; +import { logger } from '../../logging'; import { IntegrationConfig, IntegrationType } from './integrationTypes'; import { IIntegrationStorage } from './types'; diff --git a/src/notebooks/deepnote/integrations/integrationTypes.ts b/src/platform/notebooks/deepnote/integrationTypes.ts similarity index 100% rename from src/notebooks/deepnote/integrations/integrationTypes.ts rename to src/platform/notebooks/deepnote/integrationTypes.ts diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts similarity index 97% rename from src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts rename to src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index 1293102f1f..bd46c75fd4 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -1,9 +1,9 @@ import { inject, injectable } from 'inversify'; import { CancellationToken, Event, EventEmitter, NotebookDocument, workspace } from 'vscode'; -import { IDisposableRegistry, Resource } from '../../../platform/common/types'; -import { EnvironmentVariables } from '../../../platform/common/variables/types'; -import { logger } from '../../../platform/logging'; +import { IDisposableRegistry, Resource } from '../../common/types'; +import { EnvironmentVariables } from '../../common/variables/types'; +import { logger } from '../../logging'; import { IIntegrationStorage } from './types'; import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationConfig, IntegrationType } from './integrationTypes'; diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts similarity index 99% rename from src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts rename to src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index d7d444af2b..0738264a11 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -2,7 +2,7 @@ import { assert } from 'chai'; import { instance, mock, when } from 'ts-mockito'; import { EventEmitter, NotebookCell, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; -import { IDisposableRegistry } from '../../../platform/common/types'; +import { IDisposableRegistry } from '../../common/types'; import { IntegrationStorage } from './integrationStorage'; import { SqlIntegrationEnvironmentVariablesProvider } from './sqlIntegrationEnvironmentVariablesProvider'; import { IntegrationType, PostgresIntegrationConfig, BigQueryIntegrationConfig } from './integrationTypes'; diff --git a/src/platform/notebooks/deepnote/types.ts b/src/platform/notebooks/deepnote/types.ts new file mode 100644 index 0000000000..4a6027a743 --- /dev/null +++ b/src/platform/notebooks/deepnote/types.ts @@ -0,0 +1,25 @@ +import { Event } from 'vscode'; +import { IDisposable } from '../../common/types'; +import { IntegrationConfig } from './integrationTypes'; + +export const IIntegrationStorage = Symbol('IIntegrationStorage'); +export interface IIntegrationStorage extends IDisposable { + /** + * Event fired when integrations change + */ + readonly onDidChangeIntegrations: Event; + + getAll(): Promise; + get(integrationId: string): Promise; + + /** + * Get integration configuration for a specific project and integration + */ + getIntegrationConfig(projectId: string, integrationId: string): Promise; + + save(config: IntegrationConfig): Promise; + delete(integrationId: string): Promise; + exists(integrationId: string): Promise; + clear(): Promise; +} + From a5cc4bcf4c4d61047416e11076d2dba8d9f2ef9b Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:36:09 +0200 Subject: [PATCH 05/43] test: add tests for sql integration env vars --- src/kernels/helpers.unit.test.ts | 215 ++++++++++++++++++ .../kernelEnvVarsService.unit.test.ts | 155 ++++++++++++- 2 files changed, 369 insertions(+), 1 deletion(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index e84f2fc5b1..f2867ea636 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -646,4 +646,219 @@ suite('Kernel Connection Helpers', () => { assert.strictEqual(name, '.env (Python 9.8.7)'); }); }); + + suite('executeSilently', () => { + test('Returns outputs from kernel execution', async () => { + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'ok' as const + } + }), + onIOPub: () => { + // noop + } + }) + }; + + const code = 'print("hello")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + // executeSilently should return outputs array + assert.isArray(result); + }); + + test('Handles empty code', async () => { + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'ok' as const + } + }), + onIOPub: () => { + // noop + } + }) + }; + + const code = ''; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + // Should return empty array for empty code + assert.isArray(result); + }); + + test('Collects stream outputs', async () => { + let iopubCallback: ((msg: any) => void) | undefined; + + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'ok' as const + } + }), + onIOPub: (cb: (msg: any) => void) => { + iopubCallback = cb; + // Simulate stream output + setTimeout(() => { + if (iopubCallback) { + iopubCallback({ + header: { msg_type: 'stream' }, + content: { + name: 'stdout', + text: 'test output' + } + }); + } + }, 0); + } + }) + }; + + const code = 'print("test")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + // Should have collected the stream output + if (result && result.length > 0) { + assert.equal(result[0].output_type, 'stream'); + } + }); + + test('Collects error outputs', async () => { + let iopubCallback: ((msg: any) => void) | undefined; + + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'error' as const, + ename: 'NameError', + evalue: 'name not defined', + traceback: ['Traceback...'] + } + }), + onIOPub: (cb: (msg: any) => void) => { + iopubCallback = cb; + // Simulate error output + setTimeout(() => { + if (iopubCallback) { + iopubCallback({ + header: { msg_type: 'error' }, + content: { + ename: 'NameError', + evalue: 'name not defined', + traceback: ['Traceback...'] + } + }); + } + }, 0); + } + }) + }; + + const code = 'undefined_variable'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + // Should have collected the error output + if (result && result.length > 0) { + assert.equal(result[0].output_type, 'error'); + } + }); + + test('Collects display_data outputs', async () => { + let iopubCallback: ((msg: any) => void) | undefined; + + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'ok' as const + } + }), + onIOPub: (cb: (msg: any) => void) => { + iopubCallback = cb; + // Simulate display_data output + setTimeout(() => { + if (iopubCallback) { + iopubCallback({ + header: { msg_type: 'display_data' }, + content: { + data: { + 'text/plain': 'some data' + }, + metadata: {} + } + }); + } + }, 0); + } + }) + }; + + const code = 'display("data")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + // Should have collected the display_data output + if (result && result.length > 0) { + assert.equal(result[0].output_type, 'display_data'); + } + }); + + test('Handles multiple outputs', async () => { + let iopubCallback: ((msg: any) => void) | undefined; + + const mockKernel = { + requestExecute: () => ({ + done: Promise.resolve({ + content: { + status: 'ok' as const + } + }), + onIOPub: (cb: (msg: any) => void) => { + iopubCallback = cb; + // Simulate multiple outputs + setTimeout(() => { + if (iopubCallback) { + iopubCallback({ + header: { msg_type: 'stream' }, + content: { + name: 'stdout', + text: 'output 1' + } + }); + iopubCallback({ + header: { msg_type: 'stream' }, + content: { + name: 'stdout', + text: 'output 2' + } + }); + } + }, 0); + } + }) + }; + + const code = 'print("1"); print("2")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + // Should have collected multiple outputs + if (result) { + assert.isAtLeast(result.length, 0); + } + }); + }); }); diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts index 927290e91a..ae38bf0cee 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts @@ -17,6 +17,7 @@ import { IJupyterKernelSpec } from '../../types'; import { Uri } from 'vscode'; import { IConfigurationService, IWatchableJupyterSettings, type ReadWrite } from '../../../platform/common/types'; import { JupyterSettings } from '../../../platform/common/configSettings'; +import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; use(chaiAsPromised); @@ -29,6 +30,7 @@ suite('Kernel Environment Variables Service', () => { let interpreterService: IInterpreterService; let configService: IConfigurationService; let settings: IWatchableJupyterSettings; + let sqlIntegrationEnvVars: SqlIntegrationEnvironmentVariablesProvider; const pathFile = Uri.joinPath(Uri.file('foobar'), 'bar'); const interpreter: PythonEnvironment = { uri: pathFile, @@ -53,13 +55,15 @@ suite('Kernel Environment Variables Service', () => { variablesService = new EnvironmentVariablesService(instance(fs)); configService = mock(); settings = mock(JupyterSettings); + sqlIntegrationEnvVars = mock(); when(configService.getSettings(anything())).thenReturn(instance(settings)); kernelVariablesService = new KernelEnvironmentVariablesService( instance(interpreterService), instance(envActivation), variablesService, instance(customVariablesService), - instance(configService) + instance(configService), + instance(sqlIntegrationEnvVars) ); if (process.platform === 'win32') { // Win32 will generate upper case all the time @@ -84,6 +88,7 @@ suite('Kernel Environment Variables Service', () => { }); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -100,6 +105,7 @@ suite('Kernel Environment Variables Service', () => { }); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -119,6 +125,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'new' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -134,6 +141,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'new' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -148,6 +156,7 @@ suite('Kernel Environment Variables Service', () => { test('Returns process.env vars if no interpreter and no kernelspec.env', async () => { delete kernelSpec.interpreterPath; when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -161,6 +170,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); assert.isOk(processPath); @@ -178,6 +188,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); // undefined for interpreter here, interpreterPath from the spec should be used const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -196,6 +207,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); kernelSpec.env = { ONE: '1', TWO: '2' @@ -216,6 +228,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); kernelSpec.env = { ONE: '1', TWO: '2', @@ -241,6 +254,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); when(settings.excludeUserSitePackages).thenReturn(shouldBeSet); // undefined for interpreter here, interpreterPath from the spec should be used @@ -262,4 +276,143 @@ suite('Kernel Environment Variables Service', () => { test('PYTHONNOUSERSITE should be set for Virtual Env', async () => { await testPYTHONNOUSERSITE(EnvironmentType.VirtualEnv, true); }); + + suite('SQL Integration Environment Variables', () => { + test('SQL integration env vars are merged for Python kernels', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({ + SQL_MY_DB: '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}' + }); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); + + assert.strictEqual( + vars!['SQL_MY_DB'], + '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}' + ); + }); + + test('SQL integration env vars are merged for non-Python kernels', async () => { + const resource = Uri.file('test.ipynb'); + delete kernelSpec.interpreterPath; + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({ + SQL_MY_DB: '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}' + }); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, undefined, kernelSpec); + + assert.strictEqual( + vars!['SQL_MY_DB'], + '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}' + ); + }); + + test('SQL integration env vars are not added when provider returns empty object', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); + + assert.isUndefined(vars!['SQL_MY_DB']); + }); + + test('SQL integration env vars are not added when provider returns undefined', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); + + assert.isUndefined(vars!['SQL_MY_DB']); + }); + + test('Multiple SQL integration env vars are merged correctly', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({ + SQL_MY_DB: '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}', + SQL_ANOTHER_DB: '{"url":"postgresql://user2:pass2@host2:5432/db2","params":{},"param_style":"format"}' + }); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); + + assert.strictEqual( + vars!['SQL_MY_DB'], + '{"url":"postgresql://user:pass@host:5432/db","params":{},"param_style":"format"}' + ); + assert.strictEqual( + vars!['SQL_ANOTHER_DB'], + '{"url":"postgresql://user2:pass2@host2:5432/db2","params":{},"param_style":"format"}' + ); + }); + + test('SQL integration env vars work when provider is undefined (optional dependency)', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + + // Create service without SQL integration provider + const serviceWithoutSql = new KernelEnvironmentVariablesService( + instance(interpreterService), + instance(envActivation), + variablesService, + instance(customVariablesService), + instance(configService), + undefined + ); + + const vars = await serviceWithoutSql.getEnvironmentVariables(resource, interpreter, kernelSpec); + + assert.isOk(vars); + assert.isUndefined(vars!['SQL_MY_DB']); + }); + + test('SQL integration env vars handle errors gracefully', async () => { + const resource = Uri.file('test.ipynb'); + when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ + PATH: 'foobar' + }); + when( + customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) + ).thenResolve(); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenReject( + new Error('Failed to get SQL env vars') + ); + + const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); + + // Should still return vars without SQL integration vars + assert.isOk(vars); + assert.isUndefined(vars!['SQL_MY_DB']); + }); + }); }); From 42f191ce250ad3f1c3ef9e2e7084a4efb8369a0b Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:51:36 +0200 Subject: [PATCH 06/43] remove logging of secrets --- .../deepnote/deepnoteServerStarter.node.ts | 7 +------ src/kernels/kernel.ts | 20 +++++++++---------- .../sqlIntegrationStartupCodeProvider.ts | 4 +--- ...IntegrationEnvironmentVariablesProvider.ts | 3 --- 4 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index c44300bb05..856f12a047 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -169,12 +169,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension } SQL integration env vars: ${Object.keys(sqlEnvVars).join(', ')}` ); Object.assign(env, sqlEnvVars); - // Log the first 100 chars of each env var value for debugging - for (const [key, value] of Object.entries(sqlEnvVars)) { - if (value) { - logger.info(`DeepnoteServerStarter: ${key} = ${value.substring(0, 100)}...`); - } - } + logger.info(`DeepnoteServerStarter: Injected SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}`); } else { logger.info('DeepnoteServerStarter: No SQL integration env vars to inject'); } diff --git a/src/kernels/kernel.ts b/src/kernels/kernel.ts index 86c3faf1c3..6b0bed1a32 100644 --- a/src/kernels/kernel.ts +++ b/src/kernels/kernel.ts @@ -853,22 +853,22 @@ abstract class BaseKernel implements IBaseKernel { // Gather all of the startup code at one time and execute as one cell const startupCode = await this.gatherInternalStartupCode(); - logger.info(`Executing startup code with ${startupCode.length} lines`); + logger.trace(`Executing startup code with ${startupCode.length} lines`); + const outputs = await this.executeSilently(session, startupCode, { traceErrors: true, traceErrorsMessage: 'Error executing jupyter extension internal startup code' }); - logger.info(`Startup code execution completed with ${outputs?.length || 0} outputs`); + logger.trace(`Startup code execution completed with ${outputs?.length || 0} outputs`); if (outputs && outputs.length > 0) { - outputs.forEach((output, idx) => { - logger.info( - `Startup code output ${idx}: ${output.output_type} - ${JSON.stringify(output).substring( - 0, - 200 - )}` - ); - }); + // Avoid logging content; output types only. + logger.trace( + `Startup code produced ${outputs.length} output(s): ${outputs + .map((o) => o.output_type) + .join(', ')}` + ); } + // Run user specified startup commands await this.executeSilently(session, this.getUserStartupCommands(), { traceErrors: false }); } diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts index ef866033d9..32cb0bfe85 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -105,9 +105,7 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, code.push(' print(f"[SQL Integration] ERROR: Failed to set SQL integration env vars: {e}")'); code.push(' traceback.print_exc()'); - const fullCode = code.join('\n'); - logger.info(`SqlIntegrationStartupCodeProvider: Generated startup code (${fullCode.length} chars):`); - logger.info(fullCode); + logger.info('SqlIntegrationStartupCodeProvider: Generated startup code'); return code; } catch (error) { diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index bd46c75fd4..770377d8a5 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -147,9 +147,6 @@ export class SqlIntegrationEnvironmentVariablesProvider { logger.info( `SqlIntegrationEnvironmentVariablesProvider: Added env var ${envVarName} for integration ${integrationId}` ); - logger.info( - `SqlIntegrationEnvironmentVariablesProvider: Env var value: ${credentialsJson.substring(0, 100)}...` - ); } catch (error) { logger.error( `SqlIntegrationEnvironmentVariablesProvider: Failed to get credentials for integration ${integrationId}`, From 681a38fab877668f0659725e9c27136b7e678123 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:51:47 +0200 Subject: [PATCH 07/43] fix postgres db url encoding --- .../deepnote/sqlIntegrationEnvironmentVariablesProvider.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index 770377d8a5..5f79c7e966 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -35,7 +35,8 @@ function convertIntegrationConfigToJson(config: IntegrationConfig): string { // Format: postgresql://username:password@host:port/database const encodedUsername = encodeURIComponent(config.username); const encodedPassword = encodeURIComponent(config.password); - const url = `postgresql://${encodedUsername}:${encodedPassword}@${config.host}:${config.port}/${config.database}`; + const encodedDatabase = encodeURIComponent(config.database); + const url = `postgresql://${encodedUsername}:${encodedPassword}@${config.host}:${config.port}/${encodedDatabase}`; return JSON.stringify({ url: url, From 989f6878f7e0320efea90e2012067bfe5eabba88 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:51:53 +0200 Subject: [PATCH 08/43] ensure dispose --- .../deepnote/sqlIntegrationEnvironmentVariablesProvider.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index 5f79c7e966..a20b30cdc3 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -78,6 +78,8 @@ export class SqlIntegrationEnvironmentVariablesProvider { @inject(IDisposableRegistry) disposables: IDisposableRegistry ) { logger.info('SqlIntegrationEnvironmentVariablesProvider: Constructor called - provider is being instantiated'); + // Dispose emitter when extension deactivates + disposables.push(this._onDidChangeEnvironmentVariables); // Listen for changes to integration storage and fire change event disposables.push( this.integrationStorage.onDidChangeIntegrations(() => { From 79d3f458c7cf18dbe7ead3c1ee9aeafb24144a5d Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:52:01 +0200 Subject: [PATCH 09/43] use cancel token --- .../sqlIntegrationEnvironmentVariablesProvider.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index a20b30cdc3..e99f87f825 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -92,16 +92,17 @@ export class SqlIntegrationEnvironmentVariablesProvider { /** * Get environment variables for SQL integrations used in the given notebook. */ - public async getEnvironmentVariables( - resource: Resource, - _token?: CancellationToken - ): Promise { + public async getEnvironmentVariables(resource: Resource, token?: CancellationToken): Promise { const envVars: EnvironmentVariables = {}; if (!resource) { return envVars; } + if (token?.isCancellationRequested) { + return envVars; + } + logger.info(`SqlIntegrationEnvironmentVariablesProvider: Getting env vars for resource ${resource.toString()}`); logger.info( `SqlIntegrationEnvironmentVariablesProvider: Available notebooks: ${workspace.notebookDocuments @@ -133,6 +134,10 @@ export class SqlIntegrationEnvironmentVariablesProvider { // Get credentials for each integration and add to environment variables for (const integrationId of integrationIds) { + if (token?.isCancellationRequested) { + break; + } + try { const config = await this.integrationStorage.get(integrationId); if (!config) { From c5dbd12e6829e21907fc2264fcbf8404aebb4b69 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:52:54 +0200 Subject: [PATCH 10/43] lint md file --- INTEGRATIONS_CREDENTIALS.md | 40 +++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/INTEGRATIONS_CREDENTIALS.md b/INTEGRATIONS_CREDENTIALS.md index c2b2784a83..f79ba9f6e5 100644 --- a/INTEGRATIONS_CREDENTIALS.md +++ b/INTEGRATIONS_CREDENTIALS.md @@ -19,6 +19,7 @@ The integrations system enables Deepnote notebooks to connect to external data s Manages persistent storage of integration configurations using VSCode's encrypted SecretStorage API. **Key Features:** + - Uses VSCode's `SecretStorage` API for secure credential storage - Storage is scoped to the user's machine (shared across all Deepnote projects) - In-memory caching for performance @@ -26,10 +27,12 @@ Manages persistent storage of integration configurations using VSCode's encrypte - Index-based storage for efficient retrieval **Storage Format:** + - Each integration config is stored as JSON under key: `deepnote-integrations.{integrationId}` - An index is maintained at key: `deepnote-integrations.index` containing all integration IDs **Key Methods:** + - `getAll()`: Retrieve all stored integration configurations - `get(integrationId)`: Get a specific integration by ID - `save(config)`: Save or update an integration configuration @@ -67,6 +70,7 @@ Manages persistent storage of integration configurations using VSCode's encrypte Scans Deepnote projects to discover which integrations are used in SQL blocks. **Detection Process:** + 1. Retrieves the Deepnote project from `IDeepnoteNotebookManager` 2. Scans all notebooks in the project 3. Examines each code block for `metadata.sql_integration_id` @@ -74,11 +78,13 @@ Scans Deepnote projects to discover which integrations are used in SQL blocks. 5. Returns a map of integration IDs to their status **Integration Status:** + - `Connected`: Integration has valid credentials stored - `Disconnected`: Integration is used but not configured - `Error`: Integration configuration is invalid **Special Cases:** + - Excludes `deepnote-dataframe-sql` (internal DuckDB integration) - Only processes code blocks with SQL integration metadata @@ -87,6 +93,7 @@ Scans Deepnote projects to discover which integrations are used in SQL blocks. Orchestrates the integration management UI and commands. **Responsibilities:** + - Registers the `deepnote.manageIntegrations` command - Updates VSCode context keys for UI visibility: - `deepnote.hasIntegrations`: True if any integrations are detected @@ -95,6 +102,7 @@ Orchestrates the integration management UI and commands. - Opens the integration webview with detected integrations **Command Flow:** + 1. User triggers command (from command palette or SQL cell status bar) 2. Manager detects integrations in the active notebook 3. Manager opens webview with integration list @@ -105,6 +113,7 @@ Orchestrates the integration management UI and commands. Provides the webview-based UI for managing integration credentials. **Features:** + - Persistent webview panel (survives defocus) - Real-time integration status updates - Configuration forms for each integration type @@ -113,6 +122,7 @@ Provides the webview-based UI for managing integration credentials. **Message Protocol:** Extension → Webview: + ```typescript // Update integration list { type: 'update', integrations: IntegrationWithStatus[] } @@ -125,6 +135,7 @@ Extension → Webview: ``` Webview → Extension: + ```typescript // Save configuration { type: 'save', integrationId: string, config: IntegrationConfig } @@ -143,6 +154,7 @@ Webview → Extension: Main React component that manages the webview UI state. **State Management:** + - `integrations`: List of detected integrations with status - `selectedIntegrationId`: Currently selected integration for configuration - `selectedConfig`: Existing configuration being edited @@ -152,6 +164,7 @@ Main React component that manages the webview UI state. **User Flows:** **Configure Integration:** + 1. User clicks "Configure" button 2. Panel shows configuration form overlay 3. User enters credentials @@ -160,6 +173,7 @@ Main React component that manages the webview UI state. 6. Panel shows success message and refreshes list **Delete Integration:** + 1. User clicks "Reset" button 2. Panel shows confirmation prompt (5 seconds) 3. User clicks again to confirm @@ -172,6 +186,7 @@ Main React component that manages the webview UI state. Type-specific forms for entering integration credentials. **PostgreSQL Form Fields:** + - Name (display name) - Host - Port (default: 5432) @@ -181,11 +196,13 @@ Type-specific forms for entering integration credentials. - SSL (checkbox) **BigQuery Form Fields:** + - Name (display name) - Project ID - Service Account Credentials (JSON textarea) **Validation:** + - All fields are required - BigQuery credentials must be valid JSON - Port must be a valid number @@ -197,6 +214,7 @@ Type-specific forms for entering integration credentials. Provides environment variables containing integration credentials for the Jupyter kernel. **Process:** + 1. Scans the notebook for SQL cells with `sql_integration_id` metadata 2. Retrieves credentials for each detected integration 3. Converts credentials to the format expected by `deepnote-toolkit` @@ -211,6 +229,7 @@ Example: Integration ID `my-postgres-db` → Environment variable `SQL_MY_POSTGR **Credential JSON Format:** PostgreSQL: + ```json { "url": "postgresql://username:password@host:port/database", @@ -220,18 +239,22 @@ PostgreSQL: ``` BigQuery: + ```json { "url": "bigquery://?user_supplied_client=true", "params": { "project_id": "my-project", - "credentials": { /* service account JSON */ } + "credentials": { + /* service account JSON */ + } }, "param_style": "format" } ``` **Integration Points:** + - Registered as an environment variable provider in the kernel environment service - Called when starting a Jupyter kernel for a Deepnote notebook - Environment variables are passed to the kernel process at startup @@ -244,6 +267,7 @@ Injects Python code into the kernel at startup to set environment variables. Jupyter doesn't automatically pass all environment variables from the server process to the kernel process. This provider ensures credentials are available in the kernel's `os.environ`. **Generated Code:** + ```python try: import os @@ -258,6 +282,7 @@ except Exception as e: ``` **Execution:** + - Registered with `IStartupCodeProviders` for `JupyterNotebookView` - Runs automatically when a Python kernel starts for a Deepnote notebook - Priority: `StartupCodePriority.Base` (runs early) @@ -270,6 +295,7 @@ except Exception as e: The `deepnote-toolkit` Python package reads credentials from environment variables to execute SQL blocks. **Flow:** + 1. Extension detects SQL blocks in notebook 2. Extension retrieves credentials from secure storage 3. Extension converts credentials to JSON format @@ -282,6 +308,7 @@ The `deepnote-toolkit` Python package reads credentials from environment variabl **Environment Variable Lookup:** When a SQL block with `sql_integration_id: "my-postgres-db"` is executed: + 1. Toolkit looks for environment variable `SQL_MY_POSTGRES_DB` 2. Toolkit parses the JSON value 3. Toolkit creates a SQLAlchemy connection using the `url` and `params` @@ -291,6 +318,7 @@ When a SQL block with `sql_integration_id: "my-postgres-db"` is executed: ## Data Flow ### Configuration Flow + ``` User → IntegrationPanel (UI) → vscodeApi.postMessage({ type: 'save', config }) @@ -302,6 +330,7 @@ User → IntegrationPanel (UI) ``` ### Execution Flow + ``` User executes SQL cell → Kernel startup triggered @@ -332,13 +361,14 @@ User executes SQL cell To add a new integration type (e.g., MySQL, Snowflake): 1. **Add type to `integrationTypes.ts`**: + ```typescript export enum IntegrationType { Postgres = 'postgres', BigQuery = 'bigquery', MySQL = 'mysql' // New type } - + export interface MySQLIntegrationConfig extends BaseIntegrationConfig { type: IntegrationType.MySQL; host: string; @@ -347,11 +377,12 @@ To add a new integration type (e.g., MySQL, Snowflake): username: string; password: string; } - + export type IntegrationConfig = PostgresIntegrationConfig | BigQueryIntegrationConfig | MySQLIntegrationConfig; ``` 2. **Add conversion logic in `sqlIntegrationEnvironmentVariablesProvider.ts`**: + ```typescript case IntegrationType.MySQL: { const url = `mysql://${config.username}:${config.password}@${config.host}:${config.port}/${config.database}`; @@ -370,12 +401,13 @@ To add a new integration type (e.g., MySQL, Snowflake): ## Testing Unit tests are located in: + - `sqlIntegrationEnvironmentVariablesProvider.unit.test.ts` Tests cover: + - Environment variable generation for each integration type - Multiple integrations in a single notebook - Missing credentials handling - Integration ID to environment variable name conversion - JSON format validation - From 02729da57f6faba8d977b3ef5e8160f894540348 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 16:58:50 +0200 Subject: [PATCH 11/43] add interface for env var provider --- .../deepnote/deepnoteServerStarter.node.ts | 2 +- src/kernels/serviceRegistry.node.ts | 5 +++-- src/platform/notebooks/deepnote/types.ts | 17 +++++++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 856f12a047..f91f957177 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -17,7 +17,7 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from '../../platform/vscode-path/path'; import { generateUuid } from '../../platform/common/uuid'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; +import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; /** * Lock file data structure for tracking server ownership diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts index 293f19403d..0d55ce9e91 100644 --- a/src/kernels/serviceRegistry.node.ts +++ b/src/kernels/serviceRegistry.node.ts @@ -49,6 +49,7 @@ import { ClearJupyterServersCommand } from './jupyter/clearJupyterServersCommand import { KernelChatStartupCodeProvider } from './chat/kernelStartupCodeProvider'; import { KernelWorkingDirectory } from './raw/session/kernelWorkingDirectory.node'; import { SqlIntegrationEnvironmentVariablesProvider } from '../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; +import { ISqlIntegrationEnvVarsProvider } from '../platform/notebooks/deepnote/types'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { serviceManager.addSingleton(IExtensionSyncActivationService, Activation); @@ -63,8 +64,8 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea ); serviceManager.addSingleton(IRawKernelSessionFactory, RawKernelSessionFactory); serviceManager.addSingleton(IKernelLauncher, KernelLauncher); - serviceManager.addSingleton( - SqlIntegrationEnvironmentVariablesProvider, + serviceManager.addSingleton( + ISqlIntegrationEnvVarsProvider, SqlIntegrationEnvironmentVariablesProvider ); serviceManager.addSingleton( diff --git a/src/platform/notebooks/deepnote/types.ts b/src/platform/notebooks/deepnote/types.ts index 4a6027a743..e308f6d108 100644 --- a/src/platform/notebooks/deepnote/types.ts +++ b/src/platform/notebooks/deepnote/types.ts @@ -1,5 +1,6 @@ -import { Event } from 'vscode'; -import { IDisposable } from '../../common/types'; +import { CancellationToken, Event } from 'vscode'; +import { IDisposable, Resource } from '../../common/types'; +import { EnvironmentVariables } from '../../common/variables/types'; import { IntegrationConfig } from './integrationTypes'; export const IIntegrationStorage = Symbol('IIntegrationStorage'); @@ -23,3 +24,15 @@ export interface IIntegrationStorage extends IDisposable { clear(): Promise; } +export const ISqlIntegrationEnvVarsProvider = Symbol('ISqlIntegrationEnvVarsProvider'); +export interface ISqlIntegrationEnvVarsProvider { + /** + * Event fired when environment variables change + */ + readonly onDidChangeEnvironmentVariables: Event; + + /** + * Get environment variables for SQL integrations used in the given notebook. + */ + getEnvironmentVariables(resource: Resource, token?: CancellationToken): Promise; +} From 7f94f93cae838b79bc881bf0c5eb1866b769e660 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:00:02 +0200 Subject: [PATCH 12/43] decrease log level --- src/kernels/deepnote/deepnoteServerStarter.node.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index f91f957177..10314924af 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -163,15 +163,15 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension try { const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); if (sqlEnvVars && Object.keys(sqlEnvVars).length > 0) { - logger.info( + logger.debug( `DeepnoteServerStarter: Injecting ${ Object.keys(sqlEnvVars).length } SQL integration env vars: ${Object.keys(sqlEnvVars).join(', ')}` ); Object.assign(env, sqlEnvVars); - logger.info(`DeepnoteServerStarter: Injected SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}`); + logger.debug(`DeepnoteServerStarter: Injected SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}`); } else { - logger.info('DeepnoteServerStarter: No SQL integration env vars to inject'); + logger.debug('DeepnoteServerStarter: No SQL integration env vars to inject'); } } catch (error) { logger.error('DeepnoteServerStarter: Failed to get SQL integration env vars', error); From e974e810859d590510bf197d192dba108a3a3e88 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:07:22 +0200 Subject: [PATCH 13/43] refactor tests --- src/kernels/helpers.unit.test.ts | 217 ++++++++++++++----------------- 1 file changed, 96 insertions(+), 121 deletions(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index f2867ea636..cd389bc280 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -648,19 +648,57 @@ suite('Kernel Connection Helpers', () => { }); suite('executeSilently', () => { - test('Returns outputs from kernel execution', async () => { - const mockKernel = { + interface MockKernelOptions { + status: 'ok' | 'error'; + messages?: Array<{ + msg_type: 'stream' | 'error' | 'display_data'; + content: any; + }>; + errorContent?: { + ename: string; + evalue: string; + traceback: string[]; + }; + } + + function createMockKernel(options: MockKernelOptions) { + let iopubCallback: ((msg: any) => void) | undefined; + + return { requestExecute: () => ({ done: Promise.resolve({ - content: { - status: 'ok' as const - } + content: + options.status === 'ok' + ? { status: 'ok' as const } + : { + status: 'error' as const, + ...options.errorContent + } }), - onIOPub: () => { - // noop + onIOPub: (cb: (msg: any) => void) => { + iopubCallback = cb; + // Dispatch messages asynchronously to preserve async behavior + if (options.messages && options.messages.length > 0) { + setTimeout(() => { + if (iopubCallback) { + options.messages!.forEach((msg) => { + iopubCallback!({ + header: { msg_type: msg.msg_type }, + content: msg.content + }); + }); + } + }, 0); + } } }) }; + } + + test('Returns outputs from kernel execution', async () => { + const mockKernel = createMockKernel({ + status: 'ok' + }); const code = 'print("hello")'; const { executeSilently } = await import('./helpers'); @@ -671,18 +709,9 @@ suite('Kernel Connection Helpers', () => { }); test('Handles empty code', async () => { - const mockKernel = { - requestExecute: () => ({ - done: Promise.resolve({ - content: { - status: 'ok' as const - } - }), - onIOPub: () => { - // noop - } - }) - }; + const mockKernel = createMockKernel({ + status: 'ok' + }); const code = ''; const { executeSilently } = await import('./helpers'); @@ -693,32 +722,18 @@ suite('Kernel Connection Helpers', () => { }); test('Collects stream outputs', async () => { - let iopubCallback: ((msg: any) => void) | undefined; - - const mockKernel = { - requestExecute: () => ({ - done: Promise.resolve({ + const mockKernel = createMockKernel({ + status: 'ok', + messages: [ + { + msg_type: 'stream', content: { - status: 'ok' as const + name: 'stdout', + text: 'test output' } - }), - onIOPub: (cb: (msg: any) => void) => { - iopubCallback = cb; - // Simulate stream output - setTimeout(() => { - if (iopubCallback) { - iopubCallback({ - header: { msg_type: 'stream' }, - content: { - name: 'stdout', - text: 'test output' - } - }); - } - }, 0); } - }) - }; + ] + }); const code = 'print("test")'; const { executeSilently } = await import('./helpers'); @@ -732,36 +747,24 @@ suite('Kernel Connection Helpers', () => { }); test('Collects error outputs', async () => { - let iopubCallback: ((msg: any) => void) | undefined; - - const mockKernel = { - requestExecute: () => ({ - done: Promise.resolve({ + const mockKernel = createMockKernel({ + status: 'error', + errorContent: { + ename: 'NameError', + evalue: 'name not defined', + traceback: ['Traceback...'] + }, + messages: [ + { + msg_type: 'error', content: { - status: 'error' as const, ename: 'NameError', evalue: 'name not defined', traceback: ['Traceback...'] } - }), - onIOPub: (cb: (msg: any) => void) => { - iopubCallback = cb; - // Simulate error output - setTimeout(() => { - if (iopubCallback) { - iopubCallback({ - header: { msg_type: 'error' }, - content: { - ename: 'NameError', - evalue: 'name not defined', - traceback: ['Traceback...'] - } - }); - } - }, 0); } - }) - }; + ] + }); const code = 'undefined_variable'; const { executeSilently } = await import('./helpers'); @@ -775,34 +778,20 @@ suite('Kernel Connection Helpers', () => { }); test('Collects display_data outputs', async () => { - let iopubCallback: ((msg: any) => void) | undefined; - - const mockKernel = { - requestExecute: () => ({ - done: Promise.resolve({ + const mockKernel = createMockKernel({ + status: 'ok', + messages: [ + { + msg_type: 'display_data', content: { - status: 'ok' as const + data: { + 'text/plain': 'some data' + }, + metadata: {} } - }), - onIOPub: (cb: (msg: any) => void) => { - iopubCallback = cb; - // Simulate display_data output - setTimeout(() => { - if (iopubCallback) { - iopubCallback({ - header: { msg_type: 'display_data' }, - content: { - data: { - 'text/plain': 'some data' - }, - metadata: {} - } - }); - } - }, 0); } - }) - }; + ] + }); const code = 'display("data")'; const { executeSilently } = await import('./helpers'); @@ -816,39 +805,25 @@ suite('Kernel Connection Helpers', () => { }); test('Handles multiple outputs', async () => { - let iopubCallback: ((msg: any) => void) | undefined; - - const mockKernel = { - requestExecute: () => ({ - done: Promise.resolve({ + const mockKernel = createMockKernel({ + status: 'ok', + messages: [ + { + msg_type: 'stream', content: { - status: 'ok' as const + name: 'stdout', + text: 'output 1' + } + }, + { + msg_type: 'stream', + content: { + name: 'stdout', + text: 'output 2' } - }), - onIOPub: (cb: (msg: any) => void) => { - iopubCallback = cb; - // Simulate multiple outputs - setTimeout(() => { - if (iopubCallback) { - iopubCallback({ - header: { msg_type: 'stream' }, - content: { - name: 'stdout', - text: 'output 1' - } - }); - iopubCallback({ - header: { msg_type: 'stream' }, - content: { - name: 'stdout', - text: 'output 2' - } - }); - } - }, 0); } - }) - }; + ] + }); const code = 'print("1"); print("2")'; const { executeSilently } = await import('./helpers'); From 3947c69906bd999a83e1523c43ded5fb68957214 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:18:21 +0200 Subject: [PATCH 14/43] refactor tests --- src/kernels/helpers.unit.test.ts | 122 ++++++++++++++++--------------- 1 file changed, 65 insertions(+), 57 deletions(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index cd389bc280..9be43a9d1b 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -662,63 +662,69 @@ suite('Kernel Connection Helpers', () => { } function createMockKernel(options: MockKernelOptions) { - let iopubCallback: ((msg: any) => void) | undefined; - return { - requestExecute: () => ({ - done: Promise.resolve({ - content: - options.status === 'ok' - ? { status: 'ok' as const } - : { - status: 'error' as const, - ...options.errorContent - } - }), - onIOPub: (cb: (msg: any) => void) => { - iopubCallback = cb; + requestExecute: () => { + let iopubCallback: ((msg: any) => void) | undefined; + + // Create a promise that resolves after IOPub messages are dispatched + const donePromise = new Promise((resolve) => { // Dispatch messages asynchronously to preserve async behavior - if (options.messages && options.messages.length > 0) { - setTimeout(() => { - if (iopubCallback) { - options.messages!.forEach((msg) => { - iopubCallback!({ - header: { msg_type: msg.msg_type }, - content: msg.content - }); + setTimeout(() => { + if (iopubCallback && options.messages && options.messages.length > 0) { + options.messages.forEach((msg) => { + iopubCallback!({ + header: { msg_type: msg.msg_type }, + content: msg.content }); - } - }, 0); + }); + } + // Resolve the done promise after messages are dispatched + resolve({ + content: + options.status === 'ok' + ? { status: 'ok' as const } + : { + status: 'error' as const, + ...options.errorContent + } + }); + }, 0); + }); + + return { + done: donePromise, + set onIOPub(cb: (msg: any) => void) { + iopubCallback = cb; } - } - }) + }; + } }; } test('Returns outputs from kernel execution', async () => { const mockKernel = createMockKernel({ - status: 'ok' + status: 'ok', + messages: [ + { + msg_type: 'stream', + content: { + name: 'stdout', + text: 'hello\n' + } + } + ] }); const code = 'print("hello")'; const { executeSilently } = await import('./helpers'); const result = await executeSilently(mockKernel as any, code); - // executeSilently should return outputs array - assert.isArray(result); - }); - - test('Handles empty code', async () => { - const mockKernel = createMockKernel({ - status: 'ok' - }); - - const code = ''; - const { executeSilently } = await import('./helpers'); - const result = await executeSilently(mockKernel as any, code); - - // Should return empty array for empty code + // executeSilently should return outputs array with collected stream output assert.isArray(result); + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'stream'); + assert.equal((result[0] as any).name, 'stdout'); + assert.equal((result[0] as any).text, 'hello\n'); }); test('Collects stream outputs', async () => { @@ -740,10 +746,10 @@ suite('Kernel Connection Helpers', () => { const result = await executeSilently(mockKernel as any, code); assert.isArray(result); - // Should have collected the stream output - if (result && result.length > 0) { - assert.equal(result[0].output_type, 'stream'); - } + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'stream'); + assert.equal((result[0] as any).name, 'stdout'); + assert.equal((result[0] as any).text, 'test output'); }); test('Collects error outputs', async () => { @@ -771,10 +777,11 @@ suite('Kernel Connection Helpers', () => { const result = await executeSilently(mockKernel as any, code); assert.isArray(result); - // Should have collected the error output - if (result && result.length > 0) { - assert.equal(result[0].output_type, 'error'); - } + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'error'); + assert.equal((result[0] as any).ename, 'NameError'); + assert.equal((result[0] as any).evalue, 'name not defined'); + assert.deepStrictEqual((result[0] as any).traceback, ['Traceback...']); }); test('Collects display_data outputs', async () => { @@ -798,10 +805,10 @@ suite('Kernel Connection Helpers', () => { const result = await executeSilently(mockKernel as any, code); assert.isArray(result); - // Should have collected the display_data output - if (result && result.length > 0) { - assert.equal(result[0].output_type, 'display_data'); - } + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'display_data'); + assert.deepStrictEqual((result[0] as any).data, { 'text/plain': 'some data' }); + assert.deepStrictEqual((result[0] as any).metadata, {}); }); test('Handles multiple outputs', async () => { @@ -830,10 +837,11 @@ suite('Kernel Connection Helpers', () => { const result = await executeSilently(mockKernel as any, code); assert.isArray(result); - // Should have collected multiple outputs - if (result) { - assert.isAtLeast(result.length, 0); - } + // Consecutive stream messages with the same name are concatenated + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'stream'); + assert.equal((result[0] as any).name, 'stdout'); + assert.equal((result[0] as any).text, 'output 1output 2'); }); }); }); From 4bbfb516b16c674cb1c5b1d36848f3f351de32a1 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:18:29 +0200 Subject: [PATCH 15/43] remove setTimeout race from tests --- src/kernels/helpers.unit.test.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index 9be43a9d1b..48859458a8 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -664,22 +664,27 @@ suite('Kernel Connection Helpers', () => { function createMockKernel(options: MockKernelOptions) { return { requestExecute: () => { - let iopubCallback: ((msg: any) => void) | undefined; + let resolvePromise: (value: any) => void; - // Create a promise that resolves after IOPub messages are dispatched + // Create a promise that will be resolved after IOPub messages are dispatched const donePromise = new Promise((resolve) => { - // Dispatch messages asynchronously to preserve async behavior - setTimeout(() => { - if (iopubCallback && options.messages && options.messages.length > 0) { + resolvePromise = resolve; + }); + + return { + done: donePromise, + set onIOPub(cb: (msg: any) => void) { + // Invoke IOPub callback synchronously with all messages + if (options.messages && options.messages.length > 0) { options.messages.forEach((msg) => { - iopubCallback!({ + cb({ header: { msg_type: msg.msg_type }, content: msg.content }); }); } // Resolve the done promise after messages are dispatched - resolve({ + resolvePromise({ content: options.status === 'ok' ? { status: 'ok' as const } @@ -688,13 +693,6 @@ suite('Kernel Connection Helpers', () => { ...options.errorContent } }); - }, 0); - }); - - return { - done: donePromise, - set onIOPub(cb: (msg: any) => void) { - iopubCallback = cb; } }; } From 114b751066ae655ba525f47bdcec2e3bab692689 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:20:13 +0200 Subject: [PATCH 16/43] decease log level --- .../raw/launcher/kernelEnvVarsService.node.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 1d7740d657..42ab8cbd35 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -36,10 +36,8 @@ export class KernelEnvironmentVariablesService { @optional() private readonly sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider ) { - logger.info( - `KernelEnvironmentVariablesService: Constructor called, sqlIntegrationEnvVars is ${ - sqlIntegrationEnvVars ? 'AVAILABLE' : 'UNDEFINED' - }` + logger.debug( + `KernelEnvironmentVariablesService: Constructor; SQL env provider present=${!!sqlIntegrationEnvVars}` ); } /** @@ -61,7 +59,7 @@ export class KernelEnvironmentVariablesService { kernelSpec: IJupyterKernelSpec, token?: CancellationToken ) { - logger.info( + logger.debug( `KernelEnvVarsService.getEnvironmentVariables: Called for resource ${ resource?.toString() || 'undefined' }, sqlIntegrationEnvVars is ${this.sqlIntegrationEnvVars ? 'AVAILABLE' : 'UNDEFINED'}` @@ -103,7 +101,7 @@ export class KernelEnvironmentVariablesService { .getEnvironmentVariables(resource, token) .then((vars) => { if (vars && Object.keys(vars).length > 0) { - logger.info( + logger.debug( `KernelEnvVarsService: Got ${ Object.keys(vars).length } SQL integration env vars: ${Object.keys(vars).join(', ')}` @@ -158,7 +156,7 @@ export class KernelEnvironmentVariablesService { // Merge SQL integration environment variables if (sqlIntegrationEnvVars) { - logger.info( + logger.debug( `KernelEnvVarsService: Merging ${ Object.keys(sqlIntegrationEnvVars).length } SQL integration env vars into kernel env` @@ -184,7 +182,7 @@ export class KernelEnvironmentVariablesService { // Merge SQL integration environment variables if (sqlIntegrationEnvVars) { - logger.info( + logger.debug( `KernelEnvVarsService: Merging ${ Object.keys(sqlIntegrationEnvVars).length } SQL integration env vars into kernel env (non-python)` From b4651e0d68937bec6c8f586ad75debcf9efab594 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:24:59 +0200 Subject: [PATCH 17/43] DRY tests --- .../kernelEnvVarsService.unit.test.ts | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts index ae38bf0cee..e24fc11785 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts @@ -57,14 +57,6 @@ suite('Kernel Environment Variables Service', () => { settings = mock(JupyterSettings); sqlIntegrationEnvVars = mock(); when(configService.getSettings(anything())).thenReturn(instance(settings)); - kernelVariablesService = new KernelEnvironmentVariablesService( - instance(interpreterService), - instance(envActivation), - variablesService, - instance(customVariablesService), - instance(configService), - instance(sqlIntegrationEnvVars) - ); if (process.platform === 'win32') { // Win32 will generate upper case all the time const entries = Object.entries(process.env); @@ -76,9 +68,34 @@ suite('Kernel Environment Variables Service', () => { processEnv = process.env; } processPath = Object.keys(processEnv).find((k) => k.toLowerCase() == 'path'); + kernelVariablesService = buildKernelEnvVarsService(); }); + teardown(() => Object.assign(process.env, originalEnvVars)); + /** + * Helper factory function to build KernelEnvironmentVariablesService with optional overrides. + * @param overrides Optional overrides for the service dependencies + * @returns A new instance of KernelEnvironmentVariablesService + */ + function buildKernelEnvVarsService(overrides?: { + sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider | undefined; + }): KernelEnvironmentVariablesService { + const sqlProvider = + overrides && 'sqlIntegrationEnvVars' in overrides + ? overrides.sqlIntegrationEnvVars + : instance(sqlIntegrationEnvVars); + + return new KernelEnvironmentVariablesService( + instance(interpreterService), + instance(envActivation), + variablesService, + instance(customVariablesService), + instance(configService), + sqlProvider + ); + } + test('Python Interpreter path trumps process', async () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobar' @@ -381,14 +398,7 @@ suite('Kernel Environment Variables Service', () => { ).thenResolve(); // Create service without SQL integration provider - const serviceWithoutSql = new KernelEnvironmentVariablesService( - instance(interpreterService), - instance(envActivation), - variablesService, - instance(customVariablesService), - instance(configService), - undefined - ); + const serviceWithoutSql = buildKernelEnvVarsService({ sqlIntegrationEnvVars: undefined }); const vars = await serviceWithoutSql.getEnvironmentVariables(resource, interpreter, kernelSpec); From 1bb15db062dcae5a357b05bdd1af6bab863a3d62 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:31:08 +0200 Subject: [PATCH 18/43] remove integration info from logs --- .../sqlIntegrationStartupCodeProvider.ts | 2 +- ...IntegrationEnvironmentVariablesProvider.ts | 20 +++++-------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts index 32cb0bfe85..deb846abf5 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -78,7 +78,7 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, logger.info( `SqlIntegrationStartupCodeProvider: Injecting ${ Object.keys(envVars).length - } SQL integration env vars into kernel: ${Object.keys(envVars).join(', ')}` + } SQL integration env vars into kernel` ); // Generate Python code to set environment variables directly in os.environ diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index e99f87f825..bb72e9b17a 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -103,11 +103,9 @@ export class SqlIntegrationEnvironmentVariablesProvider { return envVars; } - logger.info(`SqlIntegrationEnvironmentVariablesProvider: Getting env vars for resource ${resource.toString()}`); - logger.info( - `SqlIntegrationEnvironmentVariablesProvider: Available notebooks: ${workspace.notebookDocuments - .map((nb) => nb.uri.toString()) - .join(', ')}` + logger.trace(`SqlIntegrationEnvironmentVariablesProvider: Getting env vars for resource`); + logger.trace( + `SqlIntegrationEnvironmentVariablesProvider: Available notebooks count: ${workspace.notebookDocuments.length}` ); // Find the notebook document for this resource @@ -126,11 +124,7 @@ export class SqlIntegrationEnvironmentVariablesProvider { return envVars; } - logger.info( - `SqlIntegrationEnvironmentVariablesProvider: Found ${integrationIds.size} SQL integrations: ${Array.from( - integrationIds - ).join(', ')}` - ); + logger.trace(`SqlIntegrationEnvironmentVariablesProvider: Found ${integrationIds.size} SQL integrations`); // Get credentials for each integration and add to environment variables for (const integrationId of integrationIds) { @@ -163,11 +157,7 @@ export class SqlIntegrationEnvironmentVariablesProvider { } } - logger.info( - `SqlIntegrationEnvironmentVariablesProvider: Returning ${ - Object.keys(envVars).length - } env vars: ${Object.keys(envVars).join(', ')}` - ); + logger.trace(`SqlIntegrationEnvironmentVariablesProvider: Returning ${Object.keys(envVars).length} env vars`); return envVars; } From 8cf39c84480ff565e65d7fec3d65eaba544ece46 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:38:51 +0200 Subject: [PATCH 19/43] add encoding tests --- ...nEnvironmentVariablesProvider.unit.test.ts | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index 0738264a11..7faeb4058d 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -242,6 +242,113 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { // Should return empty object when integration config is missing assert.deepStrictEqual(envVars, {}); }); + + test('Properly encodes special characters in PostgreSQL credentials', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'special-chars-db'; + const config: PostgresIntegrationConfig = { + id: integrationId, + name: 'Special Chars DB', + type: IntegrationType.Postgres, + host: 'db.example.com', + port: 5432, + database: 'my@db:name', + username: 'user@domain', + password: 'pa:ss@word!#$%', + ssl: false + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM users', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Check that the environment variable is set + assert.property(envVars, 'SQL_SPECIAL_CHARS_DB'); + const credentialsJson = JSON.parse(envVars['SQL_SPECIAL_CHARS_DB']!); + + // Verify that special characters are properly URL-encoded + assert.strictEqual( + credentialsJson.url, + 'postgresql://user%40domain:pa%3Ass%40word!%23%24%25@db.example.com:5432/my%40db%3Aname' + ); + assert.deepStrictEqual(credentialsJson.params, {}); + assert.strictEqual(credentialsJson.param_style, 'format'); + }); + + test('Normalizes integration ID with spaces and mixed case for env var name', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'My Production DB'; + const config: PostgresIntegrationConfig = { + id: integrationId, + name: 'Production Database', + type: IntegrationType.Postgres, + host: 'prod.example.com', + port: 5432, + database: 'proddb', + username: 'admin', + password: 'secret', + ssl: true + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT * FROM products', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Check that the environment variable name is properly normalized + // Spaces should be converted to underscores and uppercased + assert.property(envVars, 'SQL_MY_PRODUCTION_DB'); + const credentialsJson = JSON.parse(envVars['SQL_MY_PRODUCTION_DB']!); + assert.strictEqual(credentialsJson.url, 'postgresql://admin:secret@prod.example.com:5432/proddb'); + assert.deepStrictEqual(credentialsJson.params, { sslmode: 'require' }); + assert.strictEqual(credentialsJson.param_style, 'format'); + }); + + test('Normalizes integration ID with special characters for env var name', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'my-db@2024!'; + const config: PostgresIntegrationConfig = { + id: integrationId, + name: 'Test DB', + type: IntegrationType.Postgres, + host: 'localhost', + port: 5432, + database: 'testdb', + username: 'user', + password: 'pass', + ssl: false + }; + + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT 1', { + sql_integration_id: integrationId + }) + ]); + + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + when(integrationStorage.get(integrationId)).thenResolve(config); + + const envVars = await provider.getEnvironmentVariables(uri); + + // Check that special characters in integration ID are normalized for env var name + // Non-alphanumeric characters should be converted to underscores + assert.property(envVars, 'SQL_MY_DB_2024_'); + const credentialsJson = JSON.parse(envVars['SQL_MY_DB_2024_']!); + assert.strictEqual(credentialsJson.url, 'postgresql://user:pass@localhost:5432/testdb'); + }); }); function createMockNotebook(uri: Uri, cells: NotebookCell[]): NotebookDocument { From 6173b1602bd51fbdd3ea404d89bdf115af8691c6 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:55:44 +0200 Subject: [PATCH 20/43] update md file about logging from integration logic --- INTEGRATIONS_CREDENTIALS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/INTEGRATIONS_CREDENTIALS.md b/INTEGRATIONS_CREDENTIALS.md index f79ba9f6e5..f9b96522d5 100644 --- a/INTEGRATIONS_CREDENTIALS.md +++ b/INTEGRATIONS_CREDENTIALS.md @@ -354,7 +354,7 @@ User executes SQL cell 2. **No Plaintext**: Credentials are never written to disk in plaintext 3. **Scoped Access**: Storage is scoped to the VSCode extension 4. **Environment Isolation**: Each notebook gets only the credentials it needs -5. **No Logging**: Credential values are not logged (only first 100 chars for debugging) +5. **No Logging**: Credential values are never logged; only non-sensitive metadata (key names, counts) is logged ## Adding New Integration Types From dd7b995e679b2c87e5bd04b5949f1938526ef109 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:55:53 +0200 Subject: [PATCH 21/43] remove unused import --- src/kernels/deepnote/deepnoteServerStarter.node.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 10314924af..593d6c5c1f 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -17,7 +17,6 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from '../../platform/vscode-path/path'; import { generateUuid } from '../../platform/common/uuid'; -import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; /** * Lock file data structure for tracking server ownership From 94b34a8d6d56a380f28f558301532b9c54a69839 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:56:14 +0200 Subject: [PATCH 22/43] improve method naming in integration stoarge --- .../integrations/integrationManager.ts | 2 +- .../deepnote/integrations/integrationUtils.ts | 2 +- .../deepnote/sqlCellStatusBarProvider.ts | 2 +- .../sqlCellStatusBarProvider.unit.test.ts | 4 ++-- .../notebooks/deepnote/integrationStorage.ts | 9 ++++++--- ...lIntegrationEnvironmentVariablesProvider.ts | 2 +- ...onEnvironmentVariablesProvider.unit.test.ts | 18 +++++++++--------- src/platform/notebooks/deepnote/types.ts | 4 ++-- 8 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/notebooks/deepnote/integrations/integrationManager.ts b/src/notebooks/deepnote/integrations/integrationManager.ts index 188bc66318..c138cc42b6 100644 --- a/src/notebooks/deepnote/integrations/integrationManager.ts +++ b/src/notebooks/deepnote/integrations/integrationManager.ts @@ -149,7 +149,7 @@ export class IntegrationManager implements IIntegrationManager { // ensure it's in the map even if not detected from the project if (selectedIntegrationId && !integrations.has(selectedIntegrationId)) { logger.debug(`IntegrationManager: Adding requested integration ${selectedIntegrationId} to the map`); - const config = await this.integrationStorage.get(selectedIntegrationId); + const config = await this.integrationStorage.getIntegrationConfig(selectedIntegrationId); integrations.set(selectedIntegrationId, { config: config || null, status: config ? IntegrationStatus.Connected : IntegrationStatus.Disconnected diff --git a/src/notebooks/deepnote/integrations/integrationUtils.ts b/src/notebooks/deepnote/integrations/integrationUtils.ts index 2a0589dbd0..9e9015ca9e 100644 --- a/src/notebooks/deepnote/integrations/integrationUtils.ts +++ b/src/notebooks/deepnote/integrations/integrationUtils.ts @@ -52,7 +52,7 @@ export async function scanBlocksForIntegrations( logger.debug(`${logContext}: Found integration: ${integrationId} in block ${block.id}`); // Check if the integration is configured - const config = await integrationStorage.get(integrationId); + const config = await integrationStorage.getIntegrationConfig(integrationId); const status: IntegrationWithStatus = { config: config || null, diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts index fddb299b9e..c6439f2d8e 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.ts @@ -96,7 +96,7 @@ export class SqlCellStatusBarProvider implements NotebookCellStatusBarItemProvid } // Get integration configuration to display the name - const config = await this.integrationStorage.getIntegrationConfig(projectId, integrationId); + const config = await this.integrationStorage.getProjectIntegrationConfig(projectId, integrationId); const displayName = config?.name || l10n.t('Unknown integration (configure)'); // Create a status bar item that opens the integration management UI diff --git a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts index bae3854aaf..ac7cf16386 100644 --- a/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts +++ b/src/notebooks/deepnote/sqlCellStatusBarProvider.unit.test.ts @@ -68,7 +68,7 @@ suite('SqlCellStatusBarProvider', () => { } ); - when(integrationStorage.getIntegrationConfig(anything(), anything())).thenResolve({ + when(integrationStorage.getProjectIntegrationConfig(anything(), anything())).thenResolve({ id: integrationId, name: 'My Postgres DB', type: IntegrationType.Postgres, @@ -101,7 +101,7 @@ suite('SqlCellStatusBarProvider', () => { } ); - when(integrationStorage.getIntegrationConfig(anything(), anything())).thenResolve(undefined); + when(integrationStorage.getProjectIntegrationConfig(anything(), anything())).thenResolve(undefined); const result = await provider.provideCellStatusBarItems(cell, cancellationToken); diff --git a/src/platform/notebooks/deepnote/integrationStorage.ts b/src/platform/notebooks/deepnote/integrationStorage.ts index 4319b395d1..6b86d52f1b 100644 --- a/src/platform/notebooks/deepnote/integrationStorage.ts +++ b/src/platform/notebooks/deepnote/integrationStorage.ts @@ -43,7 +43,7 @@ export class IntegrationStorage implements IIntegrationStorage { /** * Get a specific integration configuration by ID */ - async get(integrationId: string): Promise { + async getIntegrationConfig(integrationId: string): Promise { await this.ensureCacheLoaded(); return this.cache.get(integrationId); } @@ -53,8 +53,11 @@ export class IntegrationStorage implements IIntegrationStorage { * Note: Currently integrations are stored globally, not per-project, * so this method ignores the projectId parameter */ - async getIntegrationConfig(_projectId: string, integrationId: string): Promise { - return this.get(integrationId); + async getProjectIntegrationConfig( + _projectId: string, + integrationId: string + ): Promise { + return this.getIntegrationConfig(integrationId); } /** diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index bb72e9b17a..6985189a49 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -133,7 +133,7 @@ export class SqlIntegrationEnvironmentVariablesProvider { } try { - const config = await this.integrationStorage.get(integrationId); + const config = await this.integrationStorage.getIntegrationConfig(integrationId); if (!config) { logger.warn( `SqlIntegrationEnvironmentVariablesProvider: No configuration found for integration ${integrationId}` diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index 7faeb4058d..d6f53f7be8 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -100,7 +100,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); @@ -131,7 +131,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); @@ -170,7 +170,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); @@ -213,8 +213,8 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(postgresId)).thenResolve(postgresConfig); - when(integrationStorage.get(bigqueryId)).thenResolve(bigqueryConfig); + when(integrationStorage.getIntegrationConfig(postgresId)).thenResolve(postgresConfig); + when(integrationStorage.getIntegrationConfig(bigqueryId)).thenResolve(bigqueryConfig); const envVars = await provider.getEnvironmentVariables(uri); @@ -235,7 +235,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(undefined); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(undefined); const envVars = await provider.getEnvironmentVariables(uri); @@ -265,7 +265,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); @@ -304,7 +304,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); @@ -339,7 +339,7 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { ]); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); - when(integrationStorage.get(integrationId)).thenResolve(config); + when(integrationStorage.getIntegrationConfig(integrationId)).thenResolve(config); const envVars = await provider.getEnvironmentVariables(uri); diff --git a/src/platform/notebooks/deepnote/types.ts b/src/platform/notebooks/deepnote/types.ts index e308f6d108..c9840166a8 100644 --- a/src/platform/notebooks/deepnote/types.ts +++ b/src/platform/notebooks/deepnote/types.ts @@ -11,12 +11,12 @@ export interface IIntegrationStorage extends IDisposable { readonly onDidChangeIntegrations: Event; getAll(): Promise; - get(integrationId: string): Promise; + getIntegrationConfig(integrationId: string): Promise; /** * Get integration configuration for a specific project and integration */ - getIntegrationConfig(projectId: string, integrationId: string): Promise; + getProjectIntegrationConfig(projectId: string, integrationId: string): Promise; save(config: IntegrationConfig): Promise; delete(integrationId: string): Promise; From f5377ee777e4e10d365f045e718678385543e65b Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:57:06 +0200 Subject: [PATCH 23/43] add md code fence lang --- INTEGRATIONS_CREDENTIALS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/INTEGRATIONS_CREDENTIALS.md b/INTEGRATIONS_CREDENTIALS.md index f9b96522d5..a7d9e41986 100644 --- a/INTEGRATIONS_CREDENTIALS.md +++ b/INTEGRATIONS_CREDENTIALS.md @@ -319,7 +319,7 @@ When a SQL block with `sql_integration_id: "my-postgres-db"` is executed: ### Configuration Flow -``` +```text User → IntegrationPanel (UI) → vscodeApi.postMessage({ type: 'save', config }) → IntegrationWebviewProvider.onMessage() @@ -331,7 +331,7 @@ User → IntegrationPanel (UI) ### Execution Flow -``` +```text User executes SQL cell → Kernel startup triggered → SqlIntegrationEnvironmentVariablesProvider.getEnvironmentVariables() From 1622f92bcda09e6c6cf209333793badee4e41f36 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:57:45 +0200 Subject: [PATCH 24/43] dedupe log --- src/kernels/deepnote/deepnoteServerStarter.node.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 593d6c5c1f..450b859b9e 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -163,12 +163,9 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); if (sqlEnvVars && Object.keys(sqlEnvVars).length > 0) { logger.debug( - `DeepnoteServerStarter: Injecting ${ - Object.keys(sqlEnvVars).length - } SQL integration env vars: ${Object.keys(sqlEnvVars).join(', ')}` + `DeepnoteServerStarter: Injecting SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}` ); Object.assign(env, sqlEnvVars); - logger.debug(`DeepnoteServerStarter: Injected SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}`); } else { logger.debug('DeepnoteServerStarter: No SQL integration env vars to inject'); } From 4783e5976bc52e50ea7eb7416cec23505dd23d94 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 17:59:33 +0200 Subject: [PATCH 25/43] dedupe tests --- src/kernels/helpers.unit.test.ts | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index 48859458a8..c3d48585c2 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -725,31 +725,6 @@ suite('Kernel Connection Helpers', () => { assert.equal((result[0] as any).text, 'hello\n'); }); - test('Collects stream outputs', async () => { - const mockKernel = createMockKernel({ - status: 'ok', - messages: [ - { - msg_type: 'stream', - content: { - name: 'stdout', - text: 'test output' - } - } - ] - }); - - const code = 'print("test")'; - const { executeSilently } = await import('./helpers'); - const result = await executeSilently(mockKernel as any, code); - - assert.isArray(result); - assert.equal(result.length, 1); - assert.equal(result[0].output_type, 'stream'); - assert.equal((result[0] as any).name, 'stdout'); - assert.equal((result[0] as any).text, 'test output'); - }); - test('Collects error outputs', async () => { const mockKernel = createMockKernel({ status: 'error', From 0b3514cb96ae820e5506ca891c43ff09b066f408 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:00:06 +0200 Subject: [PATCH 26/43] fix interface injection --- src/kernels/deepnote/deepnoteServerStarter.node.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 450b859b9e..46facbdbd6 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -12,6 +12,7 @@ import { STANDARD_OUTPUT_CHANNEL } from '../../platform/common/constants'; import { sleep } from '../../platform/common/utils/async'; import { Cancellation, raceCancellationError } from '../../platform/common/cancellation'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; +import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; import getPort from 'get-port'; import * as fs from 'fs-extra'; import * as os from 'os'; @@ -48,9 +49,9 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IHttpClient) private readonly httpClient: IHttpClient, @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, - @inject(SqlIntegrationEnvironmentVariablesProvider) + @inject(ISqlIntegrationEnvVarsProvider) @optional() - private readonly sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider + private readonly sqlIntegrationEnvVars?: ISqlIntegrationEnvVarsProvider ) { // Register for disposal when the extension deactivates asyncRegistry.push(this); From 91fd3cc3394f6ba52d9ad97a4f424e921b2491b2 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:04:20 +0200 Subject: [PATCH 27/43] add edge case tests --- src/kernels/helpers.unit.test.ts | 142 ++++++++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 1 deletion(-) diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index c3d48585c2..0d6d163881 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -651,7 +651,7 @@ suite('Kernel Connection Helpers', () => { interface MockKernelOptions { status: 'ok' | 'error'; messages?: Array<{ - msg_type: 'stream' | 'error' | 'display_data'; + msg_type: 'stream' | 'error' | 'display_data' | 'execute_result'; content: any; }>; errorContent?: { @@ -816,5 +816,145 @@ suite('Kernel Connection Helpers', () => { assert.equal((result[0] as any).name, 'stdout'); assert.equal((result[0] as any).text, 'output 1output 2'); }); + + test('Collects execute_result outputs', async () => { + const mockKernel = createMockKernel({ + status: 'ok', + messages: [ + { + msg_type: 'execute_result', + content: { + data: { + 'text/plain': '42' + }, + metadata: {}, + execution_count: 1 + } + } + ] + }); + + const code = '42'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'execute_result'); + assert.deepStrictEqual((result[0] as any).data, { 'text/plain': '42' }); + assert.deepStrictEqual((result[0] as any).metadata, {}); + assert.equal((result[0] as any).execution_count, 1); + }); + + test('Stream messages with different names produce separate outputs', async () => { + const mockKernel = createMockKernel({ + status: 'ok', + messages: [ + { + msg_type: 'stream', + content: { + name: 'stdout', + text: 'standard output' + } + }, + { + msg_type: 'stream', + content: { + name: 'stderr', + text: 'error output' + } + }, + { + msg_type: 'stream', + content: { + name: 'stdout', + text: ' more stdout' + } + } + ] + }); + + const code = 'print("test")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code); + + assert.isArray(result); + // Should have 3 outputs: stdout, stderr, stdout (not concatenated because stderr is in between) + assert.equal(result.length, 3); + assert.equal(result[0].output_type, 'stream'); + assert.equal((result[0] as any).name, 'stdout'); + assert.equal((result[0] as any).text, 'standard output'); + assert.equal(result[1].output_type, 'stream'); + assert.equal((result[1] as any).name, 'stderr'); + assert.equal((result[1] as any).text, 'error output'); + assert.equal(result[2].output_type, 'stream'); + assert.equal((result[2] as any).name, 'stdout'); + assert.equal((result[2] as any).text, ' more stdout'); + }); + + test('errorOptions with traceErrors logs errors', async () => { + const mockKernel = createMockKernel({ + status: 'error', + errorContent: { + ename: 'ValueError', + evalue: 'invalid value', + traceback: ['Traceback (most recent call last):', ' File "", line 1'] + }, + messages: [ + { + msg_type: 'error', + content: { + ename: 'ValueError', + evalue: 'invalid value', + traceback: ['Traceback (most recent call last):', ' File "", line 1'] + } + } + ] + }); + + const code = 'raise ValueError("invalid value")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code, { + traceErrors: true, + traceErrorsMessage: 'Custom error message' + }); + + assert.isArray(result); + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'error'); + assert.equal((result[0] as any).ename, 'ValueError'); + }); + + test('errorOptions without traceErrors still collects errors', async () => { + const mockKernel = createMockKernel({ + status: 'error', + errorContent: { + ename: 'RuntimeError', + evalue: 'runtime issue', + traceback: ['Traceback...'] + }, + messages: [ + { + msg_type: 'error', + content: { + ename: 'RuntimeError', + evalue: 'runtime issue', + traceback: ['Traceback...'] + } + } + ] + }); + + const code = 'raise RuntimeError("runtime issue")'; + const { executeSilently } = await import('./helpers'); + const result = await executeSilently(mockKernel as any, code, { + traceErrors: false + }); + + assert.isArray(result); + assert.equal(result.length, 1); + assert.equal(result[0].output_type, 'error'); + assert.equal((result[0] as any).ename, 'RuntimeError'); + }); }); }); From 1c0ab8ed1f309a8cc462273128921c4c2834cdb2 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:06:42 +0200 Subject: [PATCH 28/43] use interface --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 6 +++--- .../integrations/sqlIntegrationStartupCodeProvider.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 42ab8cbd35..b1f8669231 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -18,7 +18,7 @@ import { IJupyterKernelSpec } from '../../types'; import { CancellationToken, Uri } from 'vscode'; import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; import { trackKernelResourceInformation } from '../../telemetry/helper'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; +import { ISqlIntegrationEnvVarsProvider } from '../../../platform/notebooks/deepnote/types'; /** * Class used to fetch environment variables for a kernel. @@ -32,9 +32,9 @@ export class KernelEnvironmentVariablesService { @inject(ICustomEnvironmentVariablesProvider) private readonly customEnvVars: ICustomEnvironmentVariablesProvider, @inject(IConfigurationService) private readonly configService: IConfigurationService, - @inject(SqlIntegrationEnvironmentVariablesProvider) + @inject(ISqlIntegrationEnvVarsProvider) @optional() - private readonly sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider + private readonly sqlIntegrationEnvVars?: ISqlIntegrationEnvVarsProvider ) { logger.debug( `KernelEnvironmentVariablesService: Constructor; SQL env provider present=${!!sqlIntegrationEnvVars}` diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts index deb846abf5..bb55994c71 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { IStartupCodeProvider, IStartupCodeProviders, StartupCodePriority, IKernel } from '../../../kernels/types'; import { JupyterNotebookView } from '../../../platform/common/constants'; import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; +import { ISqlIntegrationEnvVarsProvider } from '../../../platform/notebooks/deepnote/types'; import { logger } from '../../../platform/logging'; import { isPythonKernelConnection } from '../../../kernels/helpers'; import { DEEPNOTE_NOTEBOOK_TYPE } from '../../../kernels/deepnote/types'; @@ -22,8 +22,8 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, constructor( @inject(IStartupCodeProviders) private readonly registry: IStartupCodeProviders, - @inject(SqlIntegrationEnvironmentVariablesProvider) - private readonly envVarsProvider: SqlIntegrationEnvironmentVariablesProvider + @inject(ISqlIntegrationEnvVarsProvider) + private readonly envVarsProvider: ISqlIntegrationEnvVarsProvider ) {} activate(): void { From 47647cab503da4909dcf247406d4a1e38cd34c6c Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:07:38 +0200 Subject: [PATCH 29/43] lower logs --- .../sqlIntegrationStartupCodeProvider.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts index bb55994c71..693f1d8f11 100644 --- a/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts +++ b/src/notebooks/deepnote/integrations/sqlIntegrationStartupCodeProvider.ts @@ -27,13 +27,13 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, ) {} activate(): void { - logger.info('SqlIntegrationStartupCodeProvider: Activating and registering with JupyterNotebookView'); + logger.debug('SqlIntegrationStartupCodeProvider: Activating and registering with JupyterNotebookView'); this.registry.register(this, JupyterNotebookView); - logger.info('SqlIntegrationStartupCodeProvider: Successfully registered'); + logger.debug('SqlIntegrationStartupCodeProvider: Successfully registered'); } async getCode(kernel: IKernel): Promise { - logger.info( + logger.debug( `SqlIntegrationStartupCodeProvider.getCode called for kernel ${ kernel.id }, resourceUri: ${kernel.resourceUri?.toString()}` @@ -41,26 +41,26 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, // Only run for Python kernels on Deepnote notebooks if (!isPythonKernelConnection(kernel.kernelConnectionMetadata)) { - logger.info(`SqlIntegrationStartupCodeProvider: Not a Python kernel, skipping`); + logger.debug(`SqlIntegrationStartupCodeProvider: Not a Python kernel, skipping`); return []; } // Check if this is a Deepnote notebook if (!kernel.resourceUri) { - logger.info(`SqlIntegrationStartupCodeProvider: No resourceUri, skipping`); + logger.debug(`SqlIntegrationStartupCodeProvider: No resourceUri, skipping`); return []; } const notebook = workspace.notebookDocuments.find((nb) => nb.uri.toString() === kernel.resourceUri?.toString()); if (!notebook) { - logger.info(`SqlIntegrationStartupCodeProvider: Notebook not found for ${kernel.resourceUri.toString()}`); + logger.debug(`SqlIntegrationStartupCodeProvider: Notebook not found for ${kernel.resourceUri.toString()}`); return []; } - logger.info(`SqlIntegrationStartupCodeProvider: Found notebook with type: ${notebook.notebookType}`); + logger.debug(`SqlIntegrationStartupCodeProvider: Found notebook with type: ${notebook.notebookType}`); if (notebook.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) { - logger.info(`SqlIntegrationStartupCodeProvider: Not a Deepnote notebook, skipping`); + logger.debug(`SqlIntegrationStartupCodeProvider: Not a Deepnote notebook, skipping`); return []; } @@ -75,7 +75,7 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, return []; } - logger.info( + logger.debug( `SqlIntegrationStartupCodeProvider: Injecting ${ Object.keys(envVars).length } SQL integration env vars into kernel` @@ -105,7 +105,7 @@ export class SqlIntegrationStartupCodeProvider implements IStartupCodeProvider, code.push(' print(f"[SQL Integration] ERROR: Failed to set SQL integration env vars: {e}")'); code.push(' traceback.print_exc()'); - logger.info('SqlIntegrationStartupCodeProvider: Generated startup code'); + logger.debug('SqlIntegrationStartupCodeProvider: Generated startup code'); return code; } catch (error) { From 6674550298850aedc95f3caf10cbbb377cc50c62 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:08:51 +0200 Subject: [PATCH 30/43] add cancel test --- ...nEnvironmentVariablesProvider.unit.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts index d6f53f7be8..f7348d124a 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.unit.test.ts @@ -1,6 +1,6 @@ import { assert } from 'chai'; import { instance, mock, when } from 'ts-mockito'; -import { EventEmitter, NotebookCell, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; +import { CancellationTokenSource, EventEmitter, NotebookCell, NotebookCellKind, NotebookDocument, Uri } from 'vscode'; import { IDisposableRegistry } from '../../common/types'; import { IntegrationStorage } from './integrationStorage'; @@ -349,6 +349,23 @@ suite('SqlIntegrationEnvironmentVariablesProvider', () => { const credentialsJson = JSON.parse(envVars['SQL_MY_DB_2024_']!); assert.strictEqual(credentialsJson.url, 'postgresql://user:pass@localhost:5432/testdb'); }); + + test('Honors CancellationToken (returns empty when cancelled early)', async () => { + const uri = Uri.file('/test/notebook.deepnote'); + const integrationId = 'cancel-me'; + const notebook = createMockNotebook(uri, [ + createMockCell(0, NotebookCellKind.Code, 'sql', 'SELECT 1', { sql_integration_id: integrationId }) + ]); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook]); + // Return a slow promise to ensure cancellation path is hit + when(integrationStorage.getIntegrationConfig(integrationId)).thenCall( + () => new Promise((resolve) => setTimeout(() => resolve(undefined), 50)) + ); + const cts = new CancellationTokenSource(); + cts.cancel(); + const envVars = await provider.getEnvironmentVariables(uri, cts.token); + assert.deepStrictEqual(envVars, {}); + }); }); function createMockNotebook(uri: Uri, cells: NotebookCell[]): NotebookDocument { From 3cad6034647f87d6bccac260290f2aaf7585f805 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:08:59 +0200 Subject: [PATCH 31/43] implement iface --- .../deepnote/sqlIntegrationEnvironmentVariablesProvider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index 6985189a49..029650c3ec 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -4,7 +4,7 @@ import { CancellationToken, Event, EventEmitter, NotebookDocument, workspace } f import { IDisposableRegistry, Resource } from '../../common/types'; import { EnvironmentVariables } from '../../common/variables/types'; import { logger } from '../../logging'; -import { IIntegrationStorage } from './types'; +import { IIntegrationStorage, ISqlIntegrationEnvVarsProvider } from './types'; import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationConfig, IntegrationType } from './integrationTypes'; /** @@ -68,7 +68,7 @@ function convertIntegrationConfigToJson(config: IntegrationConfig): string { * as environment variables so they can be used during SQL block execution. */ @injectable() -export class SqlIntegrationEnvironmentVariablesProvider { +export class SqlIntegrationEnvironmentVariablesProvider implements ISqlIntegrationEnvVarsProvider { private readonly _onDidChangeEnvironmentVariables = new EventEmitter(); public readonly onDidChangeEnvironmentVariables: Event = this._onDidChangeEnvironmentVariables.event; From 2a987de37c822fd48cc59ffdcadd47fae7f042a3 Mon Sep 17 00:00:00 2001 From: jankuca Date: Fri, 17 Oct 2025 18:15:27 +0200 Subject: [PATCH 32/43] fix test safety --- .../launcher/kernelEnvVarsService.unit.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts index e24fc11785..7548580f7f 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts @@ -105,7 +105,7 @@ suite('Kernel Environment Variables Service', () => { }); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -122,7 +122,7 @@ suite('Kernel Environment Variables Service', () => { }); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -142,7 +142,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'new' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -158,7 +158,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'new' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -173,7 +173,7 @@ suite('Kernel Environment Variables Service', () => { test('Returns process.env vars if no interpreter and no kernelspec.env', async () => { delete kernelSpec.interpreterPath; when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve(); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -187,7 +187,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); assert.isOk(processPath); @@ -205,7 +205,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); // undefined for interpreter here, interpreterPath from the spec should be used const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -271,7 +271,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); when(settings.excludeUserSitePackages).thenReturn(shouldBeSet); // undefined for interpreter here, interpreterPath from the spec should be used @@ -356,7 +356,7 @@ suite('Kernel Environment Variables Service', () => { when( customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything()) ).thenResolve(); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); const vars = await kernelVariablesService.getEnvironmentVariables(resource, interpreter, kernelSpec); From ad89f88c56f0009a164e64be17ba4650e501fa9a Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 14:44:58 +0200 Subject: [PATCH 33/43] update docs --- INTEGRATIONS_CREDENTIALS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/INTEGRATIONS_CREDENTIALS.md b/INTEGRATIONS_CREDENTIALS.md index a7d9e41986..108e33a5c0 100644 --- a/INTEGRATIONS_CREDENTIALS.md +++ b/INTEGRATIONS_CREDENTIALS.md @@ -34,7 +34,8 @@ Manages persistent storage of integration configurations using VSCode's encrypte **Key Methods:** - `getAll()`: Retrieve all stored integration configurations -- `get(integrationId)`: Get a specific integration by ID +- `getIntegrationConfig(integrationId)`: Get a specific integration by ID +- `getProjectIntegrationConfig(projectId, integrationId)`: Get the effective project-scoped config - `save(config)`: Save or update an integration configuration - `delete(integrationId)`: Remove an integration configuration - `exists(integrationId)`: Check if an integration is configured From 07cdb1f34cf87ed25226b472225d56743272be14 Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 14:45:04 +0200 Subject: [PATCH 34/43] hide details from logs --- src/kernels/deepnote/deepnoteServerStarter.node.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 46facbdbd6..bee7c4f131 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -159,22 +159,20 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension // Inject SQL integration environment variables if (this.sqlIntegrationEnvVars) { - logger.info(`DeepnoteServerStarter: Injecting SQL integration env vars for ${deepnoteFileUri.toString()}`); + logger.debug(`DeepnoteServerStarter: Injecting SQL integration env vars for ${deepnoteFileUri.toString()}`); try { const sqlEnvVars = await this.sqlIntegrationEnvVars.getEnvironmentVariables(deepnoteFileUri, token); if (sqlEnvVars && Object.keys(sqlEnvVars).length > 0) { - logger.debug( - `DeepnoteServerStarter: Injecting SQL env vars: ${Object.keys(sqlEnvVars).join(', ')}` - ); + logger.debug(`DeepnoteServerStarter: Injecting ${Object.keys(sqlEnvVars).length} SQL env vars`); Object.assign(env, sqlEnvVars); } else { logger.debug('DeepnoteServerStarter: No SQL integration env vars to inject'); } } catch (error) { - logger.error('DeepnoteServerStarter: Failed to get SQL integration env vars', error); + logger.error('DeepnoteServerStarter: Failed to get SQL integration env vars', error.message); } } else { - logger.info('DeepnoteServerStarter: SqlIntegrationEnvironmentVariablesProvider not available'); + logger.debug('DeepnoteServerStarter: SqlIntegrationEnvironmentVariablesProvider not available'); } // Remove PYTHONHOME if it exists (can interfere with venv) From 842302bda0e79b7f2b5586d3007c277829b0cbac Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 14:47:30 +0200 Subject: [PATCH 35/43] use iface --- .../raw/launcher/kernelEnvVarsService.unit.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts index 7548580f7f..ff13e0d948 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts @@ -17,7 +17,7 @@ import { IJupyterKernelSpec } from '../../types'; import { Uri } from 'vscode'; import { IConfigurationService, IWatchableJupyterSettings, type ReadWrite } from '../../../platform/common/types'; import { JupyterSettings } from '../../../platform/common/configSettings'; -import { SqlIntegrationEnvironmentVariablesProvider } from '../../../platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider'; +import { ISqlIntegrationEnvVarsProvider } from '../../../platform/notebooks/deepnote/types'; use(chaiAsPromised); @@ -30,7 +30,7 @@ suite('Kernel Environment Variables Service', () => { let interpreterService: IInterpreterService; let configService: IConfigurationService; let settings: IWatchableJupyterSettings; - let sqlIntegrationEnvVars: SqlIntegrationEnvironmentVariablesProvider; + let sqlIntegrationEnvVars: ISqlIntegrationEnvVarsProvider; const pathFile = Uri.joinPath(Uri.file('foobar'), 'bar'); const interpreter: PythonEnvironment = { uri: pathFile, @@ -55,7 +55,7 @@ suite('Kernel Environment Variables Service', () => { variablesService = new EnvironmentVariablesService(instance(fs)); configService = mock(); settings = mock(JupyterSettings); - sqlIntegrationEnvVars = mock(); + sqlIntegrationEnvVars = mock(); when(configService.getSettings(anything())).thenReturn(instance(settings)); if (process.platform === 'win32') { // Win32 will generate upper case all the time @@ -79,7 +79,7 @@ suite('Kernel Environment Variables Service', () => { * @returns A new instance of KernelEnvironmentVariablesService */ function buildKernelEnvVarsService(overrides?: { - sqlIntegrationEnvVars?: SqlIntegrationEnvironmentVariablesProvider | undefined; + sqlIntegrationEnvVars?: ISqlIntegrationEnvVarsProvider | undefined; }): KernelEnvironmentVariablesService { const sqlProvider = overrides && 'sqlIntegrationEnvVars' in overrides From 0bad9cd32c2a98be14b69c92778dfdfa7a49c9a2 Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:00:21 +0200 Subject: [PATCH 36/43] remove any from test --- src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts index ff13e0d948..b4dd91bdc5 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.unit.test.ts @@ -224,7 +224,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); kernelSpec.env = { ONE: '1', TWO: '2' @@ -245,7 +245,7 @@ suite('Kernel Environment Variables Service', () => { when(customVariablesService.getCustomEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobaz' }); - when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve(undefined as any); + when(sqlIntegrationEnvVars.getEnvironmentVariables(anything(), anything())).thenResolve({}); kernelSpec.env = { ONE: '1', TWO: '2', From 1b212bdd92bf5a5686eefeee93158ef3342fd247 Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:00:34 +0200 Subject: [PATCH 37/43] convert base error to custom --- ...lIntegrationEnvironmentVariablesProvider.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts index 029650c3ec..13da2ab4a3 100644 --- a/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts +++ b/src/platform/notebooks/deepnote/sqlIntegrationEnvironmentVariablesProvider.ts @@ -3,10 +3,26 @@ import { CancellationToken, Event, EventEmitter, NotebookDocument, workspace } f import { IDisposableRegistry, Resource } from '../../common/types'; import { EnvironmentVariables } from '../../common/variables/types'; +import { BaseError } from '../../errors/types'; import { logger } from '../../logging'; import { IIntegrationStorage, ISqlIntegrationEnvVarsProvider } from './types'; import { DATAFRAME_SQL_INTEGRATION_ID, IntegrationConfig, IntegrationType } from './integrationTypes'; +/** + * Error thrown when an unsupported integration type is encountered. + * + * Cause: + * An integration configuration has a type that is not supported by the SQL integration system. + * + * Handled by: + * Callers should handle this error and inform the user that the integration type is not supported. + */ +class UnsupportedIntegrationError extends BaseError { + constructor(public readonly integrationType: string) { + super('unknown', `Unsupported integration type: ${integrationType}`); + } +} + /** * Converts an integration ID to the environment variable name format expected by SQL blocks. * Example: 'my-postgres-db' -> 'SQL_MY_POSTGRES_DB' @@ -58,7 +74,7 @@ function convertIntegrationConfigToJson(config: IntegrationConfig): string { } default: - throw new Error(`Unsupported integration type: ${(config as IntegrationConfig).type}`); + throw new UnsupportedIntegrationError((config as IntegrationConfig).type); } } From 881009d2de042acec3354d4fb956a1bda0ac661c Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:05:51 +0200 Subject: [PATCH 38/43] add jsdoc --- src/platform/notebooks/deepnote/types.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/platform/notebooks/deepnote/types.ts b/src/platform/notebooks/deepnote/types.ts index c9840166a8..6036737e2c 100644 --- a/src/platform/notebooks/deepnote/types.ts +++ b/src/platform/notebooks/deepnote/types.ts @@ -11,6 +11,23 @@ export interface IIntegrationStorage extends IDisposable { readonly onDidChangeIntegrations: Event; getAll(): Promise; + + /** + * Retrieves the global (non-project-scoped) integration configuration by integration ID. + * + * This method returns integration configurations that are stored globally and shared + * across all projects. These configurations are stored in VSCode's SecretStorage and + * are scoped to the user's machine. + * + * This differs from `getProjectIntegrationConfig()` which returns project-scoped + * configurations that are specific to a particular Deepnote project and stored + * within the project's YAML file. + * + * @param integrationId - The unique identifier of the integration to retrieve + * @returns A Promise that resolves to: + * - The `IntegrationConfig` object if a global configuration exists for the given ID + * - `undefined` if no global configuration exists for the given integration ID + */ getIntegrationConfig(integrationId: string): Promise; /** From b62680199cc08c7fbe1c9ebd1620af7d2cef0a4a Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:08:50 +0200 Subject: [PATCH 39/43] remove env var log --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index b1f8669231..3208fd2bc5 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -142,7 +142,7 @@ export class KernelEnvironmentVariablesService { // Keep a list of the kernelSpec variables that need to be substituted. const kernelSpecVariablesRequiringSubstitution: Record = {}; for (const [key, value] of Object.entries(kernelEnv || {})) { - if (typeof value === 'string' && substituteEnvVars(key, value, process.env) !== value) { + if (typeof value === 'string' && substituteEnvVars(value, process.env) !== value) { kernelSpecVariablesRequiringSubstitution[key] = value; delete kernelEnv[key]; } @@ -193,7 +193,7 @@ export class KernelEnvironmentVariablesService { // env variables in kernelSpecs can contain variables that need to be substituted for (const [key, value] of Object.entries(kernelSpecVariablesRequiringSubstitution)) { - mergedVars[key] = substituteEnvVars(key, value, mergedVars); + mergedVars[key] = substituteEnvVars(value, mergedVars); } return mergedVars; @@ -202,7 +202,7 @@ export class KernelEnvironmentVariablesService { const SUBST_REGEX = /\${([a-zA-Z]\w*)?([^}\w].*)?}/g; -function substituteEnvVars(key: string, value: string, globalVars: EnvironmentVariables): string { +function substituteEnvVars(value: string, globalVars: EnvironmentVariables): string { if (!value.includes('$')) { return value; } @@ -222,7 +222,6 @@ function substituteEnvVars(key: string, value: string, globalVars: EnvironmentVa return globalVars[substName] || ''; }); if (!invalid && replacement !== value) { - logger.debug(`${key} value in kernelSpec updated from ${value} to ${replacement}`); value = replacement; } From fd84cff59a790e72378dd274a998bb54885148c4 Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:13:58 +0200 Subject: [PATCH 40/43] redact log --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 3208fd2bc5..698e0c9388 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -102,9 +102,7 @@ export class KernelEnvironmentVariablesService { .then((vars) => { if (vars && Object.keys(vars).length > 0) { logger.debug( - `KernelEnvVarsService: Got ${ - Object.keys(vars).length - } SQL integration env vars: ${Object.keys(vars).join(', ')}` + `KernelEnvVarsService: Got ${Object.keys(vars).length} SQL integration env vars` ); } return vars; From e020716d142f3dfaa2a32774a413d813315d006c Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:29:57 +0200 Subject: [PATCH 41/43] fix var list merge --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 698e0c9388..085926ccc0 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -159,7 +159,7 @@ export class KernelEnvironmentVariablesService { Object.keys(sqlIntegrationEnvVars).length } SQL integration env vars into kernel env` ); - Object.assign(mergedVars, sqlIntegrationEnvVars); + this.envVarsService.mergeVariables(sqlIntegrationEnvVars, mergedVars); } // If user asks us to, set PYTHONNOUSERSITE From 2d309ff350bc81a3f01a8b45641a83a1c80ce1c6 Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:30:34 +0200 Subject: [PATCH 42/43] fix var value test --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 085926ccc0..011448744e 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -140,7 +140,8 @@ export class KernelEnvironmentVariablesService { // Keep a list of the kernelSpec variables that need to be substituted. const kernelSpecVariablesRequiringSubstitution: Record = {}; for (const [key, value] of Object.entries(kernelEnv || {})) { - if (typeof value === 'string' && substituteEnvVars(value, process.env) !== value) { + // Detect placeholders regardless of current process.env; we'll resolve after merges. + if (typeof value === 'string' && /\${[A-Za-z]\w*(?:[^}\w].*)?}/.test(value)) { kernelSpecVariablesRequiringSubstitution[key] = value; delete kernelEnv[key]; } From e63ef6e3544181083a37c7408e33f324ad0d098c Mon Sep 17 00:00:00 2001 From: jankuca Date: Mon, 20 Oct 2025 15:33:00 +0200 Subject: [PATCH 43/43] redact log --- src/kernels/raw/launcher/kernelEnvVarsService.node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index 011448744e..51e88d9483 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -61,7 +61,7 @@ export class KernelEnvironmentVariablesService { ) { logger.debug( `KernelEnvVarsService.getEnvironmentVariables: Called for resource ${ - resource?.toString() || 'undefined' + resource ? getDisplayPath(resource) : 'undefined' }, sqlIntegrationEnvVars is ${this.sqlIntegrationEnvVars ? 'AVAILABLE' : 'UNDEFINED'}` ); let kernelEnv =