fix: coderabbit feedback

Author: FilipPyrek

src/extension.common.ts

@@ -37,6 +37,7 @@ import {
 import { Common } from './platform/common/utils/localize';
 import { IServiceContainer, IServiceManager } from './platform/ioc/types';
 import { initializeLoggers as init, logger } from './platform/logging';
+import { ILogger } from './platform/logging/types';
 import { getJupyterOutputChannel } from './standalone/devTools/jupyterOutputChannel';
 import { isUsingPylance } from './standalone/intellisense/notebookPythonPathService';
 import { noop } from './platform/common/utils/misc';
@@ -127,6 +128,7 @@ export function initializeGlobals(
         getJupyterOutputChannel(context.subscriptions),
         JUPYTER_OUTPUT_CHANNEL
     );
+    serviceManager.addSingletonInstance<ILogger>(ILogger, logger);
 
     return [serviceManager, serviceContainer];
 }

src/notebooks/deepnote/deepnoteInitNotebookRunner.node.ts

@@ -75,7 +75,6 @@ export class DeepnoteInitNotebookRunner {
         } catch (error) {
             // Log error but don't throw - we want to let user continue anyway
             logger.error(`Error running init notebook for project ${projectId}:`, error);
-            console.error(`Failed to run init notebook:`, error);
             // Still mark as run to avoid retrying on every notebook open
             this.notebookManager.markInitNotebookAsRun(projectId);
         }
@@ -206,7 +205,6 @@ export class DeepnoteInitNotebookRunner {
                 } catch (blockError) {
                     // Log error but continue with next block
                     logger.error(`Error executing init notebook block ${i + 1}:`, blockError);
-                    console.error(`Error in init notebook block ${i + 1}:`, blockError);
                 }
             }
 

src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts

@@ -36,7 +36,7 @@ import { IConfigurationService } from '../../platform/common/types';
 import { disposeAsync } from '../../platform/common/utils';
 import { IDeepnoteInitNotebookRunner } from './deepnoteInitNotebookRunner.node';
 import { IDeepnoteNotebookManager } from '../types';
-import { DeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node';
+import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node';
 import { DeepnoteProject } from './deepnoteTypes';
 import { IKernelProvider, IKernel } from '../../kernels/types';
 
@@ -74,7 +74,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
         @inject(IConfigurationService) private readonly configService: IConfigurationService,
         @inject(IDeepnoteInitNotebookRunner) private readonly initNotebookRunner: IDeepnoteInitNotebookRunner,
         @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager,
-        @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider
+        @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider,
+        @inject(IDeepnoteRequirementsHelper) private readonly requirementsHelper: IDeepnoteRequirementsHelper
     ) {}
 
     public activate() {
@@ -470,7 +471,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector,
                         if (project) {
                             // Create requirements.txt first (needs to be ready for init notebook)
                             progress.report({ message: 'Creating requirements.txt...' });
-                            await DeepnoteRequirementsHelper.createRequirementsFile(project);
+                            await this.requirementsHelper.createRequirementsFile(project, progressToken);
 
                             // Check if project has an init notebook
                             if (project.project.initNotebookId && !this.notebookManager.hasInitNotebookRun(projectId)) {

src/notebooks/deepnote/deepnoteRequirementsHelper.node.ts

@@ -1,31 +1,47 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { workspace } from 'vscode';
+import { inject, injectable } from 'inversify';
+import { workspace, CancellationToken } from 'vscode';
 import * as fs from 'fs';
-import * as path from 'path';
+import * as path from '../../platform/vscode-path/path';
 import type { DeepnoteProject } from './deepnoteTypes';
+import { ILogger } from '../../platform/logging/types';
 
 /**
  * Helper class for creating requirements.txt files from Deepnote project settings.
  */
+@injectable()
 export class DeepnoteRequirementsHelper {
+    constructor(@inject(ILogger) private readonly logger: ILogger) {}
+
     /**
      * Extracts requirements from project settings and creates a local requirements.txt file.
      * @param project The Deepnote project data containing requirements in settings
+     * @param token Cancellation token to abort the operation if needed
      */
-    static async createRequirementsFile(project: DeepnoteProject): Promise<void> {
+    async createRequirementsFile(project: DeepnoteProject, token: CancellationToken): Promise<void> {
         try {
-            const requirements = project.project.settings?.requirements;
+            // Check if the operation has been cancelled
+            if (token.isCancellationRequested) {
+                return;
+            }
+
+            const requirements = project.project.settings.requirements;
             if (!requirements || !Array.isArray(requirements) || requirements.length === 0) {
-                console.log(`No requirements found in project ${project.project.id}`);
+                this.logger.info(`No requirements found in project ${project.project.id}`);
                 return;
             }
 
             // Get the workspace folder to determine where to create the requirements.txt file
             const workspaceFolders = workspace.workspaceFolders;
             if (!workspaceFolders || workspaceFolders.length === 0) {
-                console.log('No workspace folder found, cannot create requirements.txt');
+                this.logger.info('No workspace folder found, cannot create requirements.txt');
+                return;
+            }
+
+            // Check cancellation before performing I/O
+            if (token.isCancellationRequested) {
                 return;
             }
 
@@ -37,9 +53,23 @@ export class DeepnoteRequirementsHelper {
 
             // Write the requirements.txt file
             await fs.promises.writeFile(requirementsPath, requirementsText, 'utf8');
-            console.log(`Created requirements.txt with ${requirements.length} dependencies at ${requirementsPath}`);
+
+            // Check cancellation after I/O operation
+            if (token.isCancellationRequested) {
+                this.logger.info('Requirements file creation was cancelled after write');
+                return;
+            }
+
+            this.logger.info(
+                `Created requirements.txt with ${requirements.length} dependencies at ${requirementsPath}`
+            );
         } catch (error) {
-            console.error(`Error creating requirements.txt:`, error);
+            this.logger.error(`Error creating requirements.txt:`, error);
         }
     }
 }
+
+export const IDeepnoteRequirementsHelper = Symbol('IDeepnoteRequirementsHelper');
+export interface IDeepnoteRequirementsHelper {
+    createRequirementsFile(project: DeepnoteProject, token: CancellationToken): Promise<void>;
+}

src/notebooks/serviceRegistry.node.ts

@@ -54,6 +54,7 @@ import { DeepnoteServerStarter } from '../kernels/deepnote/deepnoteServerStarter
 import { DeepnoteKernelAutoSelector } from './deepnote/deepnoteKernelAutoSelector.node';
 import { DeepnoteServerProvider } from '../kernels/deepnote/deepnoteServerProvider.node';
 import { DeepnoteInitNotebookRunner, IDeepnoteInitNotebookRunner } from './deepnote/deepnoteInitNotebookRunner.node';
+import { DeepnoteRequirementsHelper, IDeepnoteRequirementsHelper } from './deepnote/deepnoteRequirementsHelper.node';
 
 export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) {
     registerControllerTypes(serviceManager, isDevMode);
@@ -137,6 +138,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea
     serviceManager.addSingleton<IDeepnoteKernelAutoSelector>(IDeepnoteKernelAutoSelector, DeepnoteKernelAutoSelector);
     serviceManager.addBinding(IDeepnoteKernelAutoSelector, IExtensionSyncActivationService);
     serviceManager.addSingleton<IDeepnoteInitNotebookRunner>(IDeepnoteInitNotebookRunner, DeepnoteInitNotebookRunner);
+    serviceManager.addSingleton<IDeepnoteRequirementsHelper>(IDeepnoteRequirementsHelper, DeepnoteRequirementsHelper);
 
     // File export/import
     serviceManager.addSingleton<IFileConverter>(IFileConverter, FileConverter);

src/platform/logging/types.ts

@@ -6,6 +6,7 @@
 
 export type Arguments = unknown[];
 
+export const ILogger = Symbol('ILogger');
 export interface ILogger {
     error(message: string, ...data: Arguments): void;
     warn(message: string, ...data: Arguments): void;