Made exporting notebooks cancellable

package.nls.json

@@ -23,7 +23,7 @@
     "DataScience.checkingIfImportIsSupported": "Checking if import is supported",
     "DataScience.installingMissingDependencies": "Installing missing dependencies",
     "DataScience.exportNotebookToPython": "Exporting Notebook to Python",
-    "DataScience.performingExport": "Performing export",
+    "DataScience.performingExport": "Performing Export",
     "DataScience.convertingToPDF": "Converting to PDF",
     "DataScience.exportHTMLQuickPickLabel": "HTML",
     "DataScience.exportPDFQuickPickLabel": "PDF",

src/client/common/utils/localize.ts

@@ -331,7 +331,7 @@ export namespace DataScience {
         'DataScience.installingMissingDependencies',
         'Installing missing dependencies'
     );
-    export const performingExport = localize('DataScience.performingExport', 'Performing export');
+    export const performingExport = localize('DataScience.performingExport', 'Performing Export');
     export const convertingToPDF = localize('DataScience.convertingToPDF', 'Converting to PDF');
     export const exportNotebookToPython = localize(
         'DataScience.exportNotebookToPython',

src/client/datascience/export/exportBase.ts

@@ -1,11 +1,12 @@
 import { inject, injectable } from 'inversify';
-import { Uri } from 'vscode';
+import * as path from 'path';
+import { CancellationToken, Uri } from 'vscode';
 import { IFileSystem } from '../../common/platform/types';
 import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types';
 import { reportAction } from '../progress/decorator';
 import { ReportableAction } from '../progress/types';
 import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types';
-import { IExport } from './types';
+import { ExportFormat, IExport } from './types';
 
 @injectable()
 export class ExportBase implements IExport {
@@ -18,37 +19,55 @@ export class ExportBase implements IExport {
     ) {}
 
     // tslint:disable-next-line: no-empty
-    public async export(_source: Uri, _target: Uri): Promise<void> {}
+    public async export(_source: Uri, _target: Uri, _token: CancellationToken): Promise<void> {}
 
     @reportAction(ReportableAction.PerformingExport)
-    public async executeCommand(source: Uri, target: Uri, args: string[]): Promise<void> {
+    public async executeCommand(
+        source: Uri,
+        target: Uri,
+        format: ExportFormat,
+        token: CancellationToken
+    ): Promise<void> {
+        if (token.isCancellationRequested) {
+            return;
+        }
+
         const service = await this.getExecutionService(source);
         if (!service) {
             return;
         }
 
-        const oldFileExists = await this.fileSystem.fileExists(target.fsPath);
-        let oldFileTime;
-        if (oldFileExists) {
-            oldFileTime = (await this.fileSystem.stat(target.fsPath)).mtime;
+        if (token.isCancellationRequested) {
+            return;
         }
 
+        const tempTarget = await this.fileSystem.createTemporaryFile(path.extname(target.fsPath));
+        const args = [
+            source.fsPath,
+            '--to',
+            format,
+            '--output',
+            path.basename(tempTarget.filePath),
+            '--output-dir',
+            path.dirname(tempTarget.filePath)
+        ];
         const result = await service.execModule('jupyter', ['nbconvert'].concat(args), {
             throwOnStdErr: false,
-            encoding: 'utf8'
+            encoding: 'utf8',
+            token: token
         });
 
-        // Need to check if export failed, since throwOnStdErr is not an
-        // indicator of a failed export.
-        if (!(await this.fileSystem.fileExists(target.fsPath))) {
+        if (token.isCancellationRequested) {
+            tempTarget.dispose();
+            return;
+        }
+
+        try {
+            await this.fileSystem.copyFile(tempTarget.filePath, target.fsPath);
+        } catch {
             throw new Error(result.stderr);
-        } else if (oldFileExists) {
-            // If we exported to a file that already exists we need to check that
-            // this file was actually overridden during export
-            const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime;
-            if (newFileTime === oldFileTime) {
-                throw new Error(result.stderr);
-            }
+        } finally {
+            tempTarget.dispose();
         }
     }
 

src/client/datascience/export/exportManager.ts

@@ -51,27 +51,31 @@ export class ExportManager implements IExportManager {
             await this.exportUtil.removeSvgs(source);
         }
 
-        const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`);
+        const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`, true);
         try {
             switch (format) {
                 case ExportFormat.python:
-                    await this.exportToPython.export(source, target);
+                    await this.exportToPython.export(source, target, reporter.token);
                     break;
 
                 case ExportFormat.pdf:
-                    await this.exportToPDF.export(source, target);
+                    await this.exportToPDF.export(source, target, reporter.token);
                     break;
 
                 case ExportFormat.html:
-                    await this.exportToHTML.export(source, target);
+                    await this.exportToHTML.export(source, target, reporter.token);
                     break;
 
                 default:
                     break;
             }
         } finally {
-            reporter.dispose();
             tempDir.dispose();
+            reporter.dispose();
+        }
+
+        if (reporter.token.isCancellationRequested) {
+            return;
         }
 
         return target;

src/client/datascience/export/exportToHTML.ts

@@ -1,20 +1,11 @@
 import { injectable } from 'inversify';
-import * as path from 'path';
-import { Uri } from 'vscode';
+import { CancellationToken, Uri } from 'vscode';
 import { ExportBase } from './exportBase';
+import { ExportFormat } from './types';
 
 @injectable()
 export class ExportToHTML extends ExportBase {
-    public async export(source: Uri, target: Uri): Promise<void> {
-        const args = [
-            source.fsPath,
-            '--to',
-            'html',
-            '--output',
-            path.basename(target.fsPath),
-            '--output-dir',
-            path.dirname(target.fsPath)
-        ];
-        await this.executeCommand(source, target, args);
+    public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
+        await this.executeCommand(source, target, ExportFormat.html, token);
     }
 }

src/client/datascience/export/exportToPDF.ts

@@ -1,33 +1,11 @@
-import { inject, injectable } from 'inversify';
-import * as path from 'path';
-import { Uri } from 'vscode';
-import { IFileSystem } from '../../common/platform/types';
-import { IPythonExecutionFactory } from '../../common/process/types';
-import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types';
+import { injectable } from 'inversify';
+import { CancellationToken, Uri } from 'vscode';
 import { ExportBase } from './exportBase';
+import { ExportFormat } from './types';
 
 @injectable()
 export class ExportToPDF extends ExportBase {
-    constructor(
-        @inject(IPythonExecutionFactory) protected readonly pythonExecutionFactory: IPythonExecutionFactory,
-        @inject(IJupyterSubCommandExecutionService)
-        protected jupyterService: IJupyterSubCommandExecutionService,
-        @inject(IFileSystem) protected readonly fileSystem: IFileSystem,
-        @inject(INotebookImporter) protected readonly importer: INotebookImporter
-    ) {
-        super(pythonExecutionFactory, jupyterService, fileSystem, importer);
-    }
-
-    public async export(source: Uri, target: Uri): Promise<void> {
-        const args = [
-            source.fsPath,
-            '--to',
-            'pdf',
-            '--output',
-            path.basename(target.fsPath),
-            '--output-dir',
-            path.dirname(target.fsPath)
-        ];
-        await this.executeCommand(source, target, args);
+    public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
+        await this.executeCommand(source, target, ExportFormat.pdf, token);
     }
 }

src/client/datascience/export/exportToPython.ts

@@ -1,10 +1,13 @@
 import { injectable } from 'inversify';
-import { Uri } from 'vscode';
+import { CancellationToken, Uri } from 'vscode';
 import { ExportBase } from './exportBase';
 
 @injectable()
 export class ExportToPython extends ExportBase {
-    public async export(source: Uri, target: Uri): Promise<void> {
+    public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
+        if (token.isCancellationRequested) {
+            return;
+        }
         const contents = await this.importer.importFromFile(source.fsPath);
         await this.fileSystem.writeFile(target.fsPath, contents);
     }

src/client/datascience/export/types.ts

@@ -1,4 +1,4 @@
-import { Uri } from 'vscode';
+import { CancellationToken, Uri } from 'vscode';
 import { INotebookModel } from '../types';
 
 export enum ExportFormat {
@@ -14,7 +14,7 @@ export interface IExportManager {
 
 export const IExport = Symbol('IExport');
 export interface IExport {
-    export(source: Uri, target: Uri): Promise<void>;
+    export(source: Uri, target: Uri, token: CancellationToken): Promise<void>;
 }
 
 export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker');

src/client/datascience/progress/progressReporter.ts

@@ -33,15 +33,15 @@ export class ProgressReporter implements IProgressReporter {
      * @returns {(IDisposable & { token: CancellationToken })}
      * @memberof ProgressReporter
      */
-    public createProgressIndicator(message: string): IDisposable & { token: CancellationToken } {
+    public createProgressIndicator(message: string, cancellable = false): IDisposable & { token: CancellationToken } {
         const cancellation = new CancellationTokenSource();
         const deferred = createDeferred();
-        const options = { location: ProgressLocation.Notification, cancellable: false, title: message };
+        const options = { location: ProgressLocation.Notification, cancellable: cancellable, title: message };
 
         this.appShell
             .withProgress(options, async (progress, cancelToken) => {
                 cancelToken.onCancellationRequested(() => {
-                    if (!cancelToken.isCancellationRequested) {
+                    if (cancelToken.isCancellationRequested) {
                         cancellation.cancel();
                     }
                     deferred.resolve();

src/test/datascience/export/exportManager.test.ts

@@ -40,7 +40,7 @@ suite('Data Science - Export Manager', () => {
         when(exportUtil.generateTempDir()).thenResolve({ path: 'test', dispose: () => {} });
         when(exportUtil.makeFileInDirectory(anything(), anything(), anything())).thenResolve('foo');
         when(fileSystem.createTemporaryFile(anything())).thenResolve(instance(tempFile));
-        when(exportPdf.export(anything(), anything())).thenResolve();
+        when(exportPdf.export(anything(), anything(), anything())).thenResolve();
         when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo'));
         // tslint:disable-next-line: no-any
         when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock<IDisposable>()) as any);

src/test/datascience/export/exportToHTML.test.ts

@@ -4,7 +4,7 @@
 // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any
 import { assert } from 'chai';
 import * as path from 'path';
-import { Uri } from 'vscode';
+import { CancellationTokenSource, Uri } from 'vscode';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { ExportFormat, IExport } from '../../../client/datascience/export/types';
 import { IExtensionTestApi } from '../../common';
@@ -34,9 +34,11 @@ suite('DataScience - Export HTML', () => {
         const file = await fileSystem.createTemporaryFile('.html');
         const target = Uri.file(file.filePath);
         await file.dispose();
+        const token = new CancellationTokenSource();
         await exportToHTML.export(
             Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')),
-            target
+            target,
+            token.token
         );
 
         assert.equal(await fileSystem.fileExists(target.fsPath), true);

src/test/datascience/export/exportToPython.test.ts

@@ -4,7 +4,7 @@
 // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any
 import { assert } from 'chai';
 import * as path from 'path';
-import { Uri } from 'vscode';
+import { CancellationTokenSource, Uri } from 'vscode';
 import { IDocumentManager } from '../../../client/common/application/types';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { ExportFormat, IExport } from '../../../client/datascience/export/types';
@@ -33,9 +33,11 @@ suite('DataScience - Export Python', () => {
         const fileSystem = api.serviceContainer.get<IFileSystem>(IFileSystem);
         const exportToPython = api.serviceContainer.get<IExport>(IExport, ExportFormat.python);
         const target = Uri.file((await fileSystem.createTemporaryFile('.py')).filePath);
+        const token = new CancellationTokenSource();
         await exportToPython.export(
             Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')),
-            target
+            target,
+            token.token
         );
 
         const documentManager = api.serviceContainer.get<IDocumentManager>(IDocumentManager);