From 068649cba44ab99f82d89c393b082f3aa4b8d7c7 Mon Sep 17 00:00:00 2001 From: Lucy Zhang Date: Wed, 20 Jan 2021 15:45:54 -0800 Subject: [PATCH] Change configure Jupyter server steps from async to sync (#13937) * change config steps to sync * fix tests * use pathexistsSync * remove pathExistsSync call * address PR comments --- extensions/notebook/src/common/utils.ts | 14 ++++----- .../src/jupyter/jupyterSessionManager.ts | 2 +- .../notebook/src/jupyter/serverInstance.ts | 30 +++++++++---------- .../notebook/src/test/common/utils.test.ts | 4 +-- .../src/test/model/serverInstance.test.ts | 12 ++++---- 5 files changed, 29 insertions(+), 33 deletions(-) diff --git a/extensions/notebook/src/common/utils.ts b/extensions/notebook/src/common/utils.ts index 060d6c3330..7cd5e04d32 100644 --- a/extensions/notebook/src/common/utils.ts +++ b/extensions/notebook/src/common/utils.ts @@ -22,13 +22,13 @@ export function getLivyUrl(serverName: string, port: string): string { return this.getKnoxUrl(serverName, port) + '/default/livy/v1/'; } -export async function mkDir(dirPath: string, outputChannel?: vscode.OutputChannel): Promise { - if (!await fs.pathExists(dirPath)) { - if (outputChannel) { - outputChannel.appendLine(localize('mkdirOutputMsg', "... Creating {0}", dirPath)); - } - await fs.ensureDir(dirPath); - } +export async function ensureDir(dirPath: string, outputChannel?: vscode.OutputChannel): Promise { + outputChannel?.appendLine(localize('ensureDirOutputMsg', "... Ensuring {0} exists", dirPath)); + await fs.ensureDir(dirPath); +} +export function ensureDirSync(dirPath: string, outputChannel?: vscode.OutputChannel): void { + outputChannel?.appendLine(localize('ensureDirOutputMsg', "... Ensuring {0} exists", dirPath)); + fs.ensureDirSync(dirPath); } export function getErrorMessage(error: Error | string): string { diff --git a/extensions/notebook/src/jupyter/jupyterSessionManager.ts b/extensions/notebook/src/jupyter/jupyterSessionManager.ts index 70976cff3a..886cd9c4cd 100644 --- a/extensions/notebook/src/jupyter/jupyterSessionManager.ts +++ b/extensions/notebook/src/jupyter/jupyterSessionManager.ts @@ -259,7 +259,7 @@ export class JupyterSession implements nb.ISession { public async configureKernel(): Promise { let sparkmagicConfDir = path.join(utils.getUserHome(), '.sparkmagic'); - await utils.mkDir(sparkmagicConfDir); + await utils.ensureDir(sparkmagicConfDir); // Default to localhost in config file. let creds: ICredentials = { diff --git a/extensions/notebook/src/jupyter/serverInstance.ts b/extensions/notebook/src/jupyter/serverInstance.ts index 6cdd393dda..c8252e9534 100644 --- a/extensions/notebook/src/jupyter/serverInstance.ts +++ b/extensions/notebook/src/jupyter/serverInstance.ts @@ -142,34 +142,34 @@ export class PerFolderServerInstance implements IServerInstance { } private async configureJupyter(): Promise { - await this.createInstanceFolders(); + this.createInstanceFolders(); let resourcesFolder = path.join(this.options.install.extensionPath, 'resources', constants.jupyterConfigRootFolder); - await this.copyInstanceConfig(resourcesFolder); - await this.CopyCustomJs(resourcesFolder); + this.copyInstanceConfig(resourcesFolder); + this.copyCustomJs(resourcesFolder); await this.copyKernelsToSystemJupyterDirs(); } - private async createInstanceFolders(): Promise { + private createInstanceFolders(): void { this.baseDir = path.join(this.getSystemJupyterHomeDir(), 'instances', `${UUID.generateUuid()}`); this.instanceConfigRoot = path.join(this.baseDir, 'config'); this.instanceDataRoot = path.join(this.baseDir, 'data'); - await utils.mkDir(this.baseDir, this.options.install.outputChannel); - await utils.mkDir(this.instanceConfigRoot, this.options.install.outputChannel); - await utils.mkDir(this.instanceDataRoot, this.options.install.outputChannel); + utils.ensureDirSync(this.baseDir, this.options.install.outputChannel); + utils.ensureDirSync(this.instanceConfigRoot, this.options.install.outputChannel); + utils.ensureDirSync(this.instanceDataRoot, this.options.install.outputChannel); } - private async copyInstanceConfig(resourcesFolder: string): Promise { + private copyInstanceConfig(resourcesFolder: string): void { let configSource = path.join(resourcesFolder, NotebookConfigFilename); let configDest = path.join(this.instanceConfigRoot, NotebookConfigFilename); - await fs.copy(configSource, configDest); + fs.copySync(configSource, configDest); } - private async CopyCustomJs(resourcesFolder: string): Promise { + private copyCustomJs(resourcesFolder: string): void { let customPath = path.join(this.instanceConfigRoot, 'custom'); - await utils.mkDir(customPath, this.options.install.outputChannel); + utils.ensureDirSync(customPath, this.options.install.outputChannel); let customSource = path.join(resourcesFolder, CustomJsFilename); let customDest = path.join(customPath, CustomJsFilename); - await fs.copy(customSource, customDest); + fs.copySync(customSource, customDest); } private async copyKernelsToSystemJupyterDirs(): Promise { @@ -180,10 +180,8 @@ export class PerFolderServerInstance implements IServerInstance { kernelsExtensionSource = path.join(this.options.install.extensionPath, 'kernels'); } this._systemJupyterDir = path.join(this.getSystemJupyterHomeDir(), 'kernels'); - if (!(await utils.exists(this._systemJupyterDir))) { - await utils.mkDir(this._systemJupyterDir, this.options.install.outputChannel); - } - await fs.copy(kernelsExtensionSource, this._systemJupyterDir); + utils.ensureDirSync(this._systemJupyterDir, this.options.install.outputChannel); + fs.copySync(kernelsExtensionSource, this._systemJupyterDir); if (this.options.install.runningOnSaw) { await this.options.install.updateKernelSpecPaths(this._systemJupyterDir); } diff --git a/extensions/notebook/src/test/common/utils.test.ts b/extensions/notebook/src/test/common/utils.test.ts index 2d6962f1e4..3623122bf5 100644 --- a/extensions/notebook/src/test/common/utils.test.ts +++ b/extensions/notebook/src/test/common/utils.test.ts @@ -28,10 +28,10 @@ describe('Utils Tests', function () { should(utils.getLivyUrl(host, port)).endWith('/gateway/default/livy/v1/'); }); - it('mkDir', async () => { + it('ensureDir', async () => { const dirPath = path.join(os.tmpdir(), uuid.v4()); await should(fs.stat(dirPath)).be.rejected(); - await utils.mkDir(dirPath, new MockOutputChannel()); + await utils.ensureDir(dirPath, new MockOutputChannel()); should.exist(await fs.stat(dirPath), `Folder ${dirPath} did not exist after creation`); }); diff --git a/extensions/notebook/src/test/model/serverInstance.test.ts b/extensions/notebook/src/test/model/serverInstance.test.ts index b334241d4f..b0b5745fb3 100644 --- a/extensions/notebook/src/test/model/serverInstance.test.ts +++ b/extensions/notebook/src/test/model/serverInstance.test.ts @@ -55,17 +55,15 @@ describe('Jupyter server instance', function (): void { it('Should create config and data directories on configure', async function (): Promise { // Given a server instance - let mkdirStub = sinon.stub(utils,'mkDir').withArgs(sinon.match.any,sinon.match.any).returns(Promise.resolve()); - let copyStub = sinon.stub(fs,'copy').returns(); - let pathStub = sinon.stub(utils,'exists').withArgs(sinon.match.any).returns(Promise.resolve(false)); + let ensureDirSyncStub = sinon.stub(utils,'ensureDirSync').withArgs(sinon.match.any,sinon.match.any).returns(); + let copyStub = sinon.stub(fs,'copySync').returns(); // When I run configure await serverInstance.configure(); // Then I expect a folder to have been created with config and data subdirs - sinon.assert.callCount(mkdirStub,5); + sinon.assert.callCount(ensureDirSyncStub,5); sinon.assert.callCount(copyStub,3); - sinon.assert.callCount(pathStub,1); }); it('Should have URI info after start', async function (): Promise { @@ -152,8 +150,8 @@ describe('Jupyter server instance', function (): void { it('Should remove directory on close', async function (): Promise { // Given configure and startup are done - sinon.stub(utils,'mkDir').withArgs(sinon.match.any,sinon.match.any).returns(Promise.resolve()); - sinon.stub(fs,'copy').returns(); + sinon.stub(utils,'ensureDirSync').withArgs(sinon.match.any,sinon.match.any).returns(); + sinon.stub(fs,'copySync').returns(); let process = setupSpawn({ sdtout: (listener: (msg: string) => void) => { },