Implement a no sync rule (#7216)

* implement a no sync rule

* fix linting disable

* fix unused imports

* exclude more testing

* clean up fs usage

* clean up more fs usage

* remove duplicate of code

* fix compile errors

extensions/notebook/src/book/bookTreeItem.ts

@@ -78,6 +78,7 @@ export class BookTreeItem extends vscode.TreeItem {
 			if (this.book.tableOfContents.sections[i].url) {
 				// TODO: Currently only navigating to notebooks. Need to add logic for markdown.
 				let pathToNotebook = path.join(this.book.root, 'content', this.book.tableOfContents.sections[i].url.concat('.ipynb'));
+				// tslint:disable-next-line:no-sync
 				if (fs.existsSync(pathToNotebook)) {
 					this._previousUri = pathToNotebook;
 					return;
@@ -93,6 +94,7 @@ export class BookTreeItem extends vscode.TreeItem {
 			if (this.book.tableOfContents.sections[i].url) {
 				// TODO: Currently only navigating to notebooks. Need to add logic for markdown.
 				let pathToNotebook = path.join(this.book.root, 'content', this.book.tableOfContents.sections[i].url.concat('.ipynb'));
+				// tslint:disable-next-line:no-sync
 				if (fs.existsSync(pathToNotebook)) {
 					this._nextUri = pathToNotebook;
 					return;

extensions/notebook/src/book/bookTreeView.ts

@@ -6,18 +6,16 @@
 import * as azdata from 'azdata';
 import * as vscode from 'vscode';
 import * as path from 'path';
-import * as fs from 'fs';
+import { promises as fs } from 'fs';
 import * as yaml from 'js-yaml';
 import * as glob from 'fast-glob';
 import { BookTreeItem, BookTreeItemType } from './bookTreeItem';
 import { maxBookSearchDepth, notebookConfigKey } from '../common/constants';
-import { isEditorTitleFree } from '../common/utils';
+import { isEditorTitleFree, exists } from '../common/utils';
 import * as nls from 'vscode-nls';
-import { promisify } from 'util';
 import { IJupyterBookToc, IJupyterBookSection } from '../contracts/content';
 
 const localize = nls.loadMessageBundle();
-const existsAsync = promisify(fs.exists);
 
 export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeItem>, azdata.nb.NavigationProvider {
 	readonly providerId: string = 'BookNavigator';
@@ -84,7 +82,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 				const bookViewer = vscode.window.createTreeView('bookTreeView', { showCollapseAll: true, treeDataProvider: this });
 				await vscode.commands.executeCommand('workbench.books.action.focusBooksExplorer');
 				this._openAsUntitled = openAsUntitled;
-				let books = this.getBooks();
+				let books = await this.getBooks();
 				if (books && books.length > 0) {
 					const rootTreeItem = books[0];
 					const sectionToOpen = rootTreeItem.findChildSection(urlToOpen);
@@ -92,8 +90,8 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 					const urlPath = sectionToOpen ? sectionToOpen.url : rootTreeItem.tableOfContents.sections[0].url;
 					const sectionToOpenMarkdown: string = path.join(bookPath, 'content', urlPath.concat('.md'));
 					const sectionToOpenNotebook: string = path.join(bookPath, 'content', urlPath.concat('.ipynb'));
-					const markdownExists = await existsAsync(sectionToOpenMarkdown);
-					const notebookExists = await existsAsync(sectionToOpenNotebook);
+					const markdownExists = await exists(sectionToOpenMarkdown);
+					const notebookExists = await exists(sectionToOpenNotebook);
 					if (markdownExists) {
 						vscode.commands.executeCommand('markdown.showPreview', vscode.Uri.file(sectionToOpenMarkdown));
 					}
@@ -208,13 +206,13 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		return array.reduce((acc, val) => Array.isArray(val.sections) ? acc.concat(val).concat(this.parseJupyterSection(val.sections)) : acc.concat(val), []);
 	}
 
-	public getBooks(): BookTreeItem[] {
+	public async getBooks(): Promise<BookTreeItem[]> {
 		let books: BookTreeItem[] = [];
-		for (let i in this._tableOfContentPaths) {
-			let root = path.dirname(path.dirname(this._tableOfContentPaths[i]));
+		for (const contentPath of this._tableOfContentPaths) {
+			let root = path.dirname(path.dirname(contentPath));
 			try {
-				const config = yaml.safeLoad(fs.readFileSync(path.join(root, '_config.yml'), 'utf-8'));
-				const tableOfContents = yaml.safeLoad(fs.readFileSync(this._tableOfContentPaths[i], 'utf-8'));
+				const config = yaml.safeLoad((await fs.readFile(path.join(root, '_config.yml'), 'utf-8')).toString());
+				const tableOfContents = yaml.safeLoad((await fs.readFile(contentPath, 'utf-8')).toString());
 				try {
 					let book = new BookTreeItem({
 						title: config.title,
@@ -242,7 +240,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		return books;
 	}
 
-	public getSections(tableOfContents: IJupyterBookToc, sections: IJupyterBookSection[], root: string): BookTreeItem[] {
+	public async getSections(tableOfContents: IJupyterBookToc, sections: IJupyterBookSection[], root: string): Promise<BookTreeItem[]> {
 		let notebooks: BookTreeItem[] = [];
 		for (let i = 0; i < sections.length; i++) {
 			if (sections[i].url) {
@@ -267,7 +265,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 					let pathToMarkdown = path.join(root, 'content', sections[i].url.concat('.md'));
 					// Note: Currently, if there is an ipynb and a md file with the same name, Jupyter Books only shows the notebook.
 					// Following Jupyter Books behavior for now
-					if (fs.existsSync(pathToNotebook)) {
+					if (await exists(pathToNotebook)) {
 						let notebook = new BookTreeItem({
 							title: sections[i].title,
 							root: root,
@@ -286,7 +284,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 						if (this._openAsUntitled) {
 							this._allNotebooks.set(path.basename(pathToNotebook), notebook);
 						}
-					} else if (fs.existsSync(pathToMarkdown)) {
+					} else if (await exists(pathToMarkdown)) {
 						let markdown = new BookTreeItem({
 							title: sections[i].title,
 							root: root,

extensions/notebook/src/common/utils.ts

@@ -188,3 +188,12 @@ export function getHostAndPortFromEndpoint(endpoint: string): HostAndIp {
 		port: undefined
 	};
 }
+
+export async function exists(path: string): Promise<boolean> {
+	try {
+		await fs.access(path);
+		return true;
+	} catch (e) {
+		return false;
+	}
+}

extensions/notebook/src/dialog/configurePythonDialog.ts

@@ -6,7 +6,7 @@
 import * as vscode from 'vscode';
 import * as nls from 'vscode-nls';
 import * as azdata from 'azdata';
-import * as fs from 'fs';
+import { promises as fs } from 'fs';
 import * as utils from '../common/utils';
 
 import { JupyterServerInstallation } from '../jupyter/jupyterServerInstallation';
@@ -220,7 +220,7 @@ export class ConfigurePythonDialog {
 
 			if (useExistingPython) {
 				let exePath = JupyterServerInstallation.getPythonExePath(pythonLocation, true);
-				let pythonExists = fs.existsSync(exePath);
+				let pythonExists = await utils.exists(exePath);
 				if (!pythonExists) {
 					this.showErrorMessage(this.PythonNotFoundMsg);
 					return false;
@@ -243,27 +243,23 @@ export class ConfigurePythonDialog {
 		return true;
 	}
 
-	private isFileValid(pythonLocation: string): Promise<boolean> {
+	private async isFileValid(pythonLocation: string): Promise<boolean> {
 		let self = this;
-		return new Promise<boolean>(function (resolve) {
-			fs.stat(pythonLocation, function (err, stats) {
-				if (err) {
-					// Ignore error if folder doesn't exist, since it will be
-					// created during installation
-					if (err.code !== 'ENOENT') {
-						self.showErrorMessage(err.message);
-						resolve(false);
-					}
-				}
-				else {
-					if (stats.isFile()) {
-						self.showErrorMessage(self.InvalidLocationMsg);
-						resolve(false);
-					}
-				}
-				resolve(true);
-			});
-		});
+		try {
+			const stats = await fs.stat(pythonLocation);
+			if (stats.isFile()) {
+				self.showErrorMessage(self.InvalidLocationMsg);
+				return false;
+			}
+		} catch (err) {
+			// Ignore error if folder doesn't exist, since it will be
+			// created during installation
+			if (err.code !== 'ENOENT') {
+				self.showErrorMessage(err.message);
+				return false;
+			}
+		}
+		return true;
 	}
 
 	private async handleBrowse(): Promise<void> {
@@ -304,4 +300,4 @@ export class ConfigurePythonDialog {
 			level: azdata.window.MessageLevel.Error
 		};
 	}
-}
+}

extensions/notebook/src/dialog/pythonPathLookup.ts

@@ -3,7 +3,6 @@
  *  Licensed under the Source EULA. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import * as fs from 'fs';
 import * as glob from 'glob';
 
 import * as utils from '../common/utils';
@@ -92,7 +91,7 @@ export class PythonPathLookup {
 			const cmd = `"${options.command}" ${args.join(' ')}`;
 			let output = await utils.executeBufferedCommand(cmd, {});
 			let value = output ? output.trim() : '';
-			if (value.length > 0 && fs.existsSync(value)) {
+			if (value.length > 0 && await utils.exists(value)) {
 				return value;
 			}
 		} catch (err) {
@@ -163,4 +162,4 @@ export class PythonPathLookup {
 		}
 		return undefined;
 	}
-}
+}

extensions/notebook/src/jupyter/jupyterServerInstallation.ts

@@ -67,7 +67,7 @@ export class JupyterServerInstallation {
 	}
 
 	private async installDependencies(backgroundOperation: azdata.BackgroundOperation): Promise<void> {
-		if (!fs.existsSync(this._pythonExecutable) || this._forceInstall || this._usingExistingPython) {
+		if (!(await utils.exists(this._pythonExecutable)) || this._forceInstall || this._usingExistingPython) {
 			window.showInformationMessage(msgInstallPkgStart);
 
 			this.outputChannel.show(true);
@@ -180,11 +180,11 @@ export class JupyterServerInstallation {
 					});
 
 				downloadRequest.pipe(fs.createWriteStream(pythonPackagePathLocal))
-					.on('close', () => {
+					.on('close', async () => {
 						//unpack python zip/tar file
 						this.outputChannel.appendLine(msgPythonUnpackPending);
 						let pythonSourcePath = path.join(installPath, constants.pythonBundleVersion);
-						if (!this._usingExistingPython && fs.existsSync(pythonSourcePath)) {
+						if (!this._usingExistingPython && await utils.exists(pythonSourcePath)) {
 							try {
 								fs.removeSync(pythonSourcePath);
 							} catch (err) {
@@ -256,7 +256,7 @@ export class JupyterServerInstallation {
 			}
 		}
 
-		if (fs.existsSync(this._pythonExecutable)) {
+		if (await utils.exists(this._pythonExecutable)) {
 			let pythonUserDir = await this.getPythonUserDir(this._pythonExecutable);
 			if (pythonUserDir) {
 				this.pythonEnvVarPath = pythonUserDir + delimiter + this.pythonEnvVarPath;
@@ -330,7 +330,7 @@ export class JupyterServerInstallation {
 			await this.configurePackagePaths();
 		};
 		let installReady = new Deferred<void>();
-		if (!fs.existsSync(this._pythonExecutable) || this._forceInstall || this._usingExistingPython) {
+		if (!(await utils.exists(this._pythonExecutable)) || this._forceInstall || this._usingExistingPython) {
 			this.apiWrapper.startBackgroundOperation({
 				displayName: msgTaskName,
 				description: msgTaskName,
@@ -522,6 +522,7 @@ export class JupyterServerInstallation {
 		}
 
 		let condaExePath = this.getCondaExePath();
+		// tslint:disable-next-line:no-sync
 		return fs.existsSync(condaExePath);
 	}
 
@@ -538,6 +539,7 @@ export class JupyterServerInstallation {
 
 		let useExistingInstall = JupyterServerInstallation.getExistingPythonSetting(apiWrapper);
 		let pythonExe = JupyterServerInstallation.getPythonExePath(pathSetting, useExistingInstall);
+		// tslint:disable-next-line:no-sync
 		return fs.existsSync(pythonExe);
 	}
 
@@ -568,6 +570,7 @@ export class JupyterServerInstallation {
 			let notebookConfig = apiWrapper.getConfiguration(constants.notebookConfigKey);
 			if (notebookConfig) {
 				let configPythonPath = notebookConfig[constants.pythonPathConfigKey];
+				// tslint:disable-next-line:no-sync
 				if (configPythonPath && fs.existsSync(configPythonPath)) {
 					path = configPythonPath;
 				}

extensions/notebook/src/jupyter/serverInstance.ts

@@ -62,8 +62,13 @@ export class ServerInstanceUtils {
 	public copy(src: string, dest: string): Promise<void> {
 		return fs.copy(src, dest);
 	}
-	public existsSync(dirPath: string): boolean {
-		return fs.existsSync(dirPath);
+	public async exists(path: string): Promise<boolean> {
+		try {
+			await fs.access(path);
+			return true;
+		} catch (e) {
+			return false;
+		}
 	}
 	public generateUuid(): string {
 		return UUID.generateUuid();
@@ -204,7 +209,7 @@ export class PerNotebookServerInstance implements IServerInstance {
 	private async copyKernelsToSystemJupyterDirs(): Promise<void> {
 		let kernelsExtensionSource = path.join(this.options.install.extensionPath, 'kernels');
 		this._systemJupyterDir = path.join(this.getSystemJupyterHomeDir(), 'kernels');
-		if (!this.utils.existsSync(this._systemJupyterDir)) {
+		if (!(await this.utils.exists(this._systemJupyterDir))) {
 			await this.utils.mkDir(this._systemJupyterDir, this.options.install.outputChannel);
 		}
 		await this.utils.copy(kernelsExtensionSource, this._systemJupyterDir);

extensions/notebook/src/test/book/book.test.ts

@@ -283,9 +283,9 @@ describe.skip('BookTreeViewProviderTests', function() {
 			await Promise.race([tocRead, errorCase.then(() => { throw new Error('Table of Contents were not ready in time'); })]);
 		});
 
-		it('should show error if notebook or markdown file is missing', function(): void {
+		it('should show error if notebook or markdown file is missing', async function(): Promise<void> {
 			let books = bookTreeViewProvider.getBooks();
-			let children = bookTreeViewProvider.getSections({ sections: [] }, books[0].sections, rootFolderPath);
+			let children = await bookTreeViewProvider.getSections({ sections: [] }, (await books)[0].sections, rootFolderPath);
 			should(bookTreeViewProvider.errorMessage).equal('Missing file : Notebook1');
 			// Rest of book should be detected correctly even with a missing file
 			equalBookItems(children[0], expectedNotebook2);

extensions/notebook/src/test/model/serverInstance.test.ts

@@ -57,7 +57,7 @@ describe('Jupyter server instance', function (): void {
 		// Given a server instance
 		mockUtils.setup(u => u.mkDir(TypeMoq.It.isAnyString(), TypeMoq.It.isAny())).returns(() => Promise.resolve());
 		mockUtils.setup(u => u.copy(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString())).returns(() => Promise.resolve());
-		mockUtils.setup(u => u.existsSync(TypeMoq.It.isAnyString())).returns(() => false);
+		mockUtils.setup(u => u.exists(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(false));
 
 		// When I run configure
 		await serverInstance.configure();
@@ -65,7 +65,7 @@ describe('Jupyter server instance', function (): void {
 		// Then I expect a folder to have been created with config and data subdirs
 		mockUtils.verify(u => u.mkDir(TypeMoq.It.isAnyString(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(5));
 		mockUtils.verify(u => u.copy(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(3));
-		mockUtils.verify(u => u.existsSync(TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(1));
+		mockUtils.verify(u => u.exists(TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(1));
 	});
 
 	it('Should have URI info after start', async function (): Promise<void> {