From 0c31013971420c8408f4983d1ba37ce54c1c79dd Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 12 Dec 2018 14:24:09 -0800 Subject: [PATCH 1/3] Sys Info refactor and add server info --- .gitignore | 1 + build/.nycrc | 20 ------ package.nls.json | 3 +- src/client/common/utils/localize.ts | 1 + src/client/datascience/history.ts | 70 +++++++++++++------ src/client/datascience/jupyterServer.ts | 14 +++- src/client/datascience/types.ts | 8 +++ src/datascience-ui/history-react/cell.tsx | 2 +- .../history-react/mainPanelState.ts | 3 +- src/datascience-ui/history-react/sysInfo.tsx | 4 +- src/test/datascience/execution.unit.test.ts | 5 +- 11 files changed, 82 insertions(+), 49 deletions(-) delete mode 100644 build/.nycrc diff --git a/.gitignore b/.gitignore index 1b56aa33068a..39f9f6961c75 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ node_modules **/testFiles/**/.cache/** *.noseids .nyc_output +.nycrc .vscode-test __pycache__ npm-debug.log diff --git a/build/.nycrc b/build/.nycrc deleted file mode 100644 index 0646aa44fe16..000000000000 --- a/build/.nycrc +++ /dev/null @@ -1,20 +0,0 @@ -{ - "check-coverage": false, - "per-file": true, - "include": [ - "out/client/**/*.js" - ], - "exclude": [ - "out/test/*.js", - "out/**/*.jsx" - ], - "reporter": [ - "lcov", - "html" - ], - "extension": [ - ".js" - ], - "cache": true, - "all": true -} diff --git a/package.nls.json b/package.nls.json index c2f519693712..0f88d48f1ae9 100644 --- a/package.nls.json +++ b/package.nls.json @@ -139,5 +139,6 @@ "DataScience.exportChangeDirectoryComment": "# Change directory to VSCode workspace root so that relative path loads work correctly. Turn this addition off with the DataSciece.changeDirOnImportExport setting", "DataScience.interruptKernelStatus" : "Interrupting iPython Kernel", "DataScience.restartKernelAfterInterruptMessage" : "Interrupting the kernel timed out. Do you want to restart the kernel instead? All variables will be lost.", - "DataScience.pythonInterruptFailedHeader" : "Keyboard interrupt crashed the kernel. Kernel restarted." + "DataScience.pythonInterruptFailedHeader" : "Keyboard interrupt crashed the kernel. Kernel restarted.", + "DataScience.sysInfoURILabel" : "Jupyter Server URI: " } diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ac330d0178f1..706edb837a5b 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -103,6 +103,7 @@ export namespace DataScience { export const exportCancel = localize('DataScience.exportCancel', 'Cancel'); export const restartKernelAfterInterruptMessage = localize('DataScience.restartKernelAfterInterruptMessage', 'Interrupting the kernel timed out. Do you want to restart the kernel instead? All variables will be lost.'); export const pythonInterruptFailedHeader = localize('DataScience.pythonInterruptFailedHeader', 'Keyboard interrupt crashed the kernel. Kernel restarted.'); + export const sysInfoURILabel = localize('DataScience.sysInfoURILabel', 'Jupyter Server URI: '); } // Skip using vscode-nls and instead just compute our strings based on key values. Key values diff --git a/src/client/datascience/history.ts b/src/client/datascience/history.ts index 5d94832a396c..9e665718b710 100644 --- a/src/client/datascience/history.ts +++ b/src/client/datascience/history.ts @@ -34,13 +34,15 @@ import { CellState, ICell, ICodeCssGenerator, + IConnection, IHistory, IHistoryInfo, IJupyterExecution, INotebookExporter, INotebookServer, InterruptResult, - IStatusProvider + IStatusProvider, + SysInfoReason } from './types'; @injectable() @@ -119,7 +121,7 @@ export class History implements IWebPanelMessageListener, IHistory { await this.show(); // Add our sys info if necessary - await this.addInitialSysInfo(); + await this.addSysInfo(SysInfoReason.Start); if (this.jupyterServer) { // Before we try to execute code make sure that we have an initial directory set @@ -308,7 +310,7 @@ export class History implements IWebPanelMessageListener, IHistory { }); } else if (result === InterruptResult.Restarted) { // Uh-oh, keyboard interrupt crashed the kernel. - this.addInterruptFailedInfo().ignoreErrors(); + this.addSysInfo(SysInfoReason.Interrupt).ignoreErrors(); } }) .catch(err => { @@ -339,7 +341,7 @@ export class History implements IWebPanelMessageListener, IHistory { try { if (this.jupyterServer) { await this.jupyterServer.restartKernel(); - await this.addRestartSysInfo(); + await this.addSysInfo(SysInfoReason.Restart); } } finally { status.dispose(); @@ -554,7 +556,7 @@ export class History implements IWebPanelMessageListener, IHistory { // If this is a restart, show our restart info if (restart) { - await this.addRestartSysInfo(); + await this.addSysInfo(SysInfoReason.Restart); } } finally { if (status) { @@ -619,10 +621,11 @@ export class History implements IWebPanelMessageListener, IHistory { return result; } - private generateSysInfoCell = async (message: string) : Promise => { + private generateSysInfoCell = async (reason: SysInfoReason) : Promise => { // Execute the code 'import sys\r\nsys.version' and 'import sys\r\nsys.executable' to get our // version and executable if (this.jupyterServer) { + const message = await this.generateSysInfoMessage(reason); // tslint:disable-next-line:no-multiline-string const versionCells = await this.jupyterServer.execute(`import sys\r\nsys.version`, 'foo.py', 0); // tslint:disable-next-line:no-multiline-string @@ -638,6 +641,12 @@ export class History implements IWebPanelMessageListener, IHistory { // Both should influence our ignore count. We don't want them to count against execution this.ignoreCount = this.ignoreCount + 3; + // Connection string only for our initial start, not restart or interrupt + let connectionString: string = ''; + if (reason === SysInfoReason.Start) { + connectionString = this.generateConnectionInfoString(this.jupyterServer.getConnectionInfo()); + } + // Combine this data together to make our sys info return { data: { @@ -646,6 +655,7 @@ export class History implements IWebPanelMessageListener, IHistory { version: version, notebook_version: localize.DataScience.notebookVersionFormat().format(notebookVersion), path: pythonPath, + connection: connectionString, metadata: {}, source: [] }, @@ -657,33 +667,47 @@ export class History implements IWebPanelMessageListener, IHistory { } } - private addInitialSysInfo = async () : Promise => { - // Message depends upon if ipykernel is supported or not. - if (!(await this.jupyterExecution.isKernelCreateSupported())) { - return this.addSysInfo(localize.DataScience.pythonVersionHeaderNoPyKernel()); + private async generateSysInfoMessage(reason: SysInfoReason): Promise { + switch (reason) { + case SysInfoReason.Start: + // Message depends upon if ipykernel is supported or not. + if (!(await this.jupyterExecution.isKernelCreateSupported())) { + return localize.DataScience.pythonVersionHeaderNoPyKernel(); + } + return localize.DataScience.pythonVersionHeader(); + break; + case SysInfoReason.Restart: + return localize.DataScience.pythonRestartHeader(); + break; + case SysInfoReason.Interrupt: + return localize.DataScience.pythonInterruptFailedHeader(); + break; + default: + this.logger.logError('Invalid SysInfoReason'); + return ''; + break; } - - return this.addSysInfo(localize.DataScience.pythonVersionHeader()); } - private addRestartSysInfo = () : Promise => { - this.addedSysInfo = false; - return this.addSysInfo(localize.DataScience.pythonRestartHeader()); - } + private generateConnectionInfoString(connInfo: IConnection | undefined): string { + if (!connInfo) { + return ''; + } + + const tokenString = connInfo.token.length > 0 ? `?token=${connInfo.token}` : ''; + const urlString = `${connInfo.baseUrl}${tokenString}`; - private addInterruptFailedInfo = () : Promise => { - this.addedSysInfo = false; - return this.addSysInfo(localize.DataScience.pythonInterruptFailedHeader()); + return `${localize.DataScience.sysInfoURILabel()}${urlString}`; } - private addSysInfo = async (message: string) : Promise => { - // Add our sys info if necessary - if (!this.addedSysInfo) { + private addSysInfo = async (reason: SysInfoReason) : Promise => { + if ((reason === SysInfoReason.Start && !this.addedSysInfo) || + (reason === SysInfoReason.Interrupt || reason === SysInfoReason.Restart)) { this.addedSysInfo = true; this.ignoreCount = 0; // Generate a new sys info cell and send it to the web panel. - const sysInfo = await this.generateSysInfoCell(message); + const sysInfo = await this.generateSysInfoCell(reason); if (sysInfo) { this.onAddCodeEvent([sysInfo]); } diff --git a/src/client/datascience/jupyterServer.ts b/src/client/datascience/jupyterServer.ts index 3e1416ae5e70..2d5a2b7113bd 100644 --- a/src/client/datascience/jupyterServer.ts +++ b/src/client/datascience/jupyterServer.ts @@ -405,7 +405,7 @@ export class JupyterServer implements INotebookServer, IDisposable { return InterruptResult.TimedOut; } catch (exc) { // Something failed. See if we restarted or not. - if (interruptBeginTime < this.sessionStartTime) { + if (this.sessionStartTime && (interruptBeginTime < this.sessionStartTime)) { return InterruptResult.Restarted; } @@ -419,6 +419,18 @@ export class JupyterServer implements INotebookServer, IDisposable { throw new Error(localize.DataScience.sessionDisposed()); } + // Return a copy of the connection information that this server used to connect with + public getConnectionInfo(): IConnection | undefined { + if (!this.connInfo) { + return undefined; + } + + // Return a copy with a no-op for dispose + return { + ...this.connInfo, + dispose: noop }; + } + private shutdownSessionAndConnection = async () => { if (this.contentsManager) { this.contentsManager.dispose(); diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 83299eff8974..4a5eb63c31b6 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -47,6 +47,7 @@ export interface INotebookServer extends Disposable { shutdown() : Promise; interruptKernel(timeoutInMs: number) : Promise; setInitialDirectory(directory: string): Promise; + getConnectionInfo(): IConnection | undefined; } export const IJupyterExecution = Symbol('IJupyterExecution'); @@ -150,12 +151,19 @@ export interface IHistoryInfo { redoCount: number; } +export enum SysInfoReason { + Start, + Restart, + Interrupt +} + export interface ISysInfo extends nbformat.IBaseCell { cell_type: 'sys_info'; version: string; notebook_version: string; path: string; message: string; + connection: string; } export const ICodeCssGenerator = Symbol('ICodeCssGenerator'); diff --git a/src/datascience-ui/history-react/cell.tsx b/src/datascience-ui/history-react/cell.tsx index 483cec0ceec5..9b7c71b4d31d 100644 --- a/src/datascience-ui/history-react/cell.tsx +++ b/src/datascience-ui/history-react/cell.tsx @@ -44,7 +44,7 @@ export class Cell extends React.Component { public render() { if (this.props.cellVM.cell.data.cell_type === 'sys_info') { - return ; + return ; } else { return this.renderNormalCell(); } diff --git a/src/datascience-ui/history-react/mainPanelState.ts b/src/datascience-ui/history-react/mainPanelState.ts index 0c807c509af6..12622148ce83 100644 --- a/src/datascience-ui/history-react/mainPanelState.ts +++ b/src/datascience-ui/history-react/mainPanelState.ts @@ -83,7 +83,8 @@ function generateCellData() : (nbformat.ICodeCell | nbformat.IMarkdownCell | nbf notebook_version: '(5, 9, 9)', source: [], metadata: {}, - message: 'You have this python data:' + message: 'You have this python data:', + connection: 'https:\\localhost' }, { cell_type: 'code', diff --git a/src/datascience-ui/history-react/sysInfo.tsx b/src/datascience-ui/history-react/sysInfo.tsx index 0748fa9083ce..148cd0ce6149 100644 --- a/src/datascience-ui/history-react/sysInfo.tsx +++ b/src/datascience-ui/history-react/sysInfo.tsx @@ -13,6 +13,7 @@ interface ISysInfoProps notebook_version: string; version: string; theme: string; + connection: string; } export class SysInfo extends React.Component { @@ -21,7 +22,8 @@ export class SysInfo extends React.Component { } public render() { - const output = `${this.props.message}\r\n${this.props.version}\r\n${this.props.path}\r\n${this.props.notebook_version}`; + const connectionString = this.props.connection.length > 0 ? `${this.props.connection}\r\n` : ''; + const output = `${connectionString}${this.props.message}\r\n${this.props.version}\r\n${this.props.path}\r\n${this.props.notebook_version}`; return (
diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 37ddfe359422..be0bb2a97f6a 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -51,7 +51,7 @@ class MockJupyterServer implements INotebookServer { this.kernelSpec = kernelSpec; // Validate connection info and kernel spec - if (conninfo.baseUrl && /[a-z,A-Z,0-9,-,.,_]+/.test(kernelSpec.name)) { + if (conninfo.baseUrl && kernelSpec.name && /[a-z,A-Z,0-9,-,.,_]+/.test(kernelSpec.name)) { return Promise.resolve(); } return Promise.reject('invalid server startup'); @@ -81,6 +81,9 @@ class MockJupyterServer implements INotebookServer { public setInitialDirectory(directory: string): Promise { throw new Error('Method not implemented'); } + public getConnectionInfo(): IConnection | undefined { + throw new Error('Method not implemented'); + } public async shutdown() { return Promise.resolve(); } From ada4394b25e585a6c5ca90986b907a942b78f89a Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 12 Dec 2018 15:35:16 -0800 Subject: [PATCH 2/3] add news entry --- news/1 Enhancements/3668.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1 Enhancements/3668.md diff --git a/news/1 Enhancements/3668.md b/news/1 Enhancements/3668.md new file mode 100644 index 000000000000..ee19deadb488 --- /dev/null +++ b/news/1 Enhancements/3668.md @@ -0,0 +1 @@ +Add the Jupyter Server URI to the Interactive Window info cell \ No newline at end of file From b145d29fdde3b7d88059b7a592c2de0f74e51cae Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Wed, 12 Dec 2018 16:19:40 -0800 Subject: [PATCH 3/3] review feedback --- .gitignore | 1 - src/client/datascience/history.ts | 12 ++++++++---- src/client/datascience/types.ts | 6 ------ 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 39f9f6961c75..1b56aa33068a 100644 --- a/.gitignore +++ b/.gitignore @@ -7,7 +7,6 @@ node_modules **/testFiles/**/.cache/** *.noseids .nyc_output -.nycrc .vscode-test __pycache__ npm-debug.log diff --git a/src/client/datascience/history.ts b/src/client/datascience/history.ts index 9e665718b710..659f13ba79d9 100644 --- a/src/client/datascience/history.ts +++ b/src/client/datascience/history.ts @@ -41,10 +41,15 @@ import { INotebookExporter, INotebookServer, InterruptResult, - IStatusProvider, - SysInfoReason + IStatusProvider } from './types'; +export enum SysInfoReason { + Start, + Restart, + Interrupt +} + @injectable() export class History implements IWebPanelMessageListener, IHistory { private disposed : boolean = false; @@ -701,8 +706,7 @@ export class History implements IWebPanelMessageListener, IHistory { } private addSysInfo = async (reason: SysInfoReason) : Promise => { - if ((reason === SysInfoReason.Start && !this.addedSysInfo) || - (reason === SysInfoReason.Interrupt || reason === SysInfoReason.Restart)) { + if (!this.addedSysInfo || reason === SysInfoReason.Interrupt || reason === SysInfoReason.Restart) { this.addedSysInfo = true; this.ignoreCount = 0; diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 4a5eb63c31b6..6381e444e9a2 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -151,12 +151,6 @@ export interface IHistoryInfo { redoCount: number; } -export enum SysInfoReason { - Start, - Restart, - Interrupt -} - export interface ISysInfo extends nbformat.IBaseCell { cell_type: 'sys_info'; version: string;