Skip to content

Commit e24de8a

Browse files
authored
Provide a default configuration for jupyter startup (microsoft#3558)
* Implement basic custom config and add some tests
1 parent 46650c0 commit e24de8a

21 files changed

+275
-136
lines changed

news/2 Fixes/3475.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Create a default config when starting a local jupyter server to resolve potential conflicts with user's custom configurations.

news/2 Fixes/3533.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix problems with jupyter startup related to custom configurations.

package-lock.json

Lines changed: 5 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,6 +1024,12 @@
10241024
"description": "Select the Jupyter server URI to connect to. Select 'local' to launch a new Juypter server on the local machine.",
10251025
"scope": "resource"
10261026
},
1027+
"python.dataScience.useDefaultConfigForJupyter": {
1028+
"type": "boolean",
1029+
"default": true,
1030+
"description": "When running Jupyter locally, create a default empty Jupyter config for the Python Interactive window",
1031+
"scope": "resource"
1032+
},
10271033
"python.disableInstallationCheck": {
10281034
"type": "boolean",
10291035
"default": false,

src/client/common/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ export interface IDataScienceSettings {
268268
enabled: boolean;
269269
jupyterLaunchTimeout: number;
270270
jupyterServerURI: string;
271+
useDefaultConfigForJupyter: boolean;
271272
}
272273

273274
export const IConfigurationService = Symbol('IConfigurationService');

src/client/datascience/cellFactory.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function generateMarkdownCell(code: string[], file: string, line: number) : ICel
5454

5555
export function generateCells(code: string, file: string, line: number, splitMarkdown?: boolean) : ICell[] {
5656
// Determine if we have a markdown cell/ markdown and code cell combined/ or just a code cell
57-
const split = code.splitLines();
57+
const split = code.splitLines({trim: false});
5858
const firstLine = split[0];
5959
if (RegExpValues.PythonMarkdownCellMarker.test(firstLine)) {
6060
// We have at least one markdown. We might have to split it if there any lines that don't begin

src/client/datascience/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export namespace Telemetry {
7979
export const Interrupt = 'DATASCIENCE.INTERRUPT';
8080
export const ExportPythonFile = 'DATASCIENCE.EXPORT_PYTHON_FILE';
8181
export const ExportPythonFileAndOutput = 'DATASCIENCE.EXPORT_PYTHON_FILE_AND_OUTPUT';
82+
export const StartJupyter = 'DATASCIENCE.JUPYTERSTARTUPCOST';
8283
}
8384

8485
export namespace HelpLinks {

src/client/datascience/editor-integration/codewatcher.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ export class CodeWatcher implements ICodeWatcher {
154154

155155
// tslint:disable-next-line:no-any
156156
private handleError = (err : any) => {
157-
if ((<JupyterInstallError>err).actionTitle !== undefined) {
157+
if (err instanceof JupyterInstallError) {
158158
const jupyterError = err as JupyterInstallError;
159159

160160
// This is a special error that shows a link to open for more help

src/client/datascience/history.ts

Lines changed: 50 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ export class History implements IWebPanelMessageListener, IHistory {
5454
private addedSysInfo: boolean = false;
5555
private ignoreCount: number = 0;
5656
private waitingForExportCells : boolean = false;
57+
private jupyterServer: INotebookServer | undefined;
5758

5859
constructor(
5960
@inject(IApplicationShell) private applicationShell: IApplicationShell,
6061
@inject(IDocumentManager) private documentManager: IDocumentManager,
6162
@inject(IInterpreterService) private interpreterService: IInterpreterService,
62-
@inject(INotebookServer) private jupyterServer: INotebookServer,
6363
@inject(IWebPanelProvider) private provider: IWebPanelProvider,
6464
@inject(IDisposableRegistry) private disposables: IDisposableRegistry,
6565
@inject(ICodeCssGenerator) private cssGenerator : ICodeCssGenerator,
@@ -283,14 +283,16 @@ export class History implements IWebPanelMessageListener, IHistory {
283283
const status = this.statusProvider.set(localize.DataScience.restartingKernelStatus(), this);
284284

285285
// Then restart the kernel. When that finishes, add our sys info again
286-
this.jupyterServer.restartKernel()
287-
.then(() => {
288-
this.addRestartSysInfo().then(status.dispose()).ignoreErrors();
289-
})
290-
.catch(err => {
291-
this.logger.logError(err);
292-
status.dispose();
293-
});
286+
if (this.jupyterServer) {
287+
this.jupyterServer.restartKernel()
288+
.then(() => {
289+
this.addRestartSysInfo().then(status.dispose()).ignoreErrors();
290+
})
291+
.catch(err => {
292+
this.logger.logError(err);
293+
status.dispose();
294+
});
295+
}
294296
this.restartingKernel = false;
295297
} else {
296298
this.restartingKernel = false;
@@ -495,14 +497,15 @@ export class History implements IWebPanelMessageListener, IHistory {
495497
// Startup our jupyter server
496498
const settings = this.configuration.getSettings();
497499
let serverURI: string | undefined = settings.datascience.jupyterServerURI;
500+
const useDefaultConfig : boolean | undefined = settings.datascience.useDefaultConfigForJupyter;
498501

499502
const status = this.setStatus(localize.DataScience.connectingToJupyter());
500503
try {
501504
// For the local case pass in our URI as undefined, that way connect doesn't have to check the setting
502505
if (serverURI === Settings.JupyterServerLocalLaunch) {
503506
serverURI = undefined;
504507
}
505-
this.jupyterServer = await this.jupyterExecution.connectToNotebookServer(serverURI);
508+
this.jupyterServer = await this.jupyterExecution.connectToNotebookServer(serverURI, useDefaultConfig);
506509

507510
// If this is a restart, show our restart info
508511
if (restart) {
@@ -536,40 +539,42 @@ export class History implements IWebPanelMessageListener, IHistory {
536539
return result;
537540
}
538541

539-
private generateSysInfoCell = async (message: string) : Promise<ICell> => {
542+
private generateSysInfoCell = async (message: string) : Promise<ICell | undefined> => {
540543
// Execute the code 'import sys\r\nsys.version' and 'import sys\r\nsys.executable' to get our
541544
// version and executable
542-
// tslint:disable-next-line:no-multiline-string
543-
const versionCells = await this.jupyterServer.execute(`import sys\r\nsys.version`, 'foo.py', 0);
544-
// tslint:disable-next-line:no-multiline-string
545-
const pathCells = await this.jupyterServer.execute(`import sys\r\nsys.executable`, 'foo.py', 0);
546-
// tslint:disable-next-line:no-multiline-string
547-
const notebookVersionCells = await this.jupyterServer.execute(`import notebook\r\nnotebook.version_info`, 'foo.py', 0);
548-
549-
// Both should have streamed output
550-
const version = versionCells.length > 0 ? this.extractStreamOutput(versionCells[0]).trimQuotes() : '';
551-
const notebookVersion = notebookVersionCells.length > 0 ? this.extractStreamOutput(notebookVersionCells[0]).trimQuotes() : '';
552-
const pythonPath = versionCells.length > 0 ? this.extractStreamOutput(pathCells[0]).trimQuotes() : '';
553-
554-
// Both should influence our ignore count. We don't want them to count against execution
555-
this.ignoreCount = this.ignoreCount + 3;
556-
557-
// Combine this data together to make our sys info
558-
return {
559-
data: {
560-
cell_type : 'sys_info',
561-
message: message,
562-
version: version,
563-
notebook_version: localize.DataScience.notebookVersionFormat().format(notebookVersion),
564-
path: pythonPath,
565-
metadata : {},
566-
source : []
567-
},
568-
id: uuid(),
569-
file: '',
570-
line: 0,
571-
state: CellState.finished
572-
};
545+
if (this.jupyterServer) {
546+
// tslint:disable-next-line:no-multiline-string
547+
const versionCells = await this.jupyterServer.execute(`import sys\r\nsys.version`, 'foo.py', 0);
548+
// tslint:disable-next-line:no-multiline-string
549+
const pathCells = await this.jupyterServer.execute(`import sys\r\nsys.executable`, 'foo.py', 0);
550+
// tslint:disable-next-line:no-multiline-string
551+
const notebookVersionCells = await this.jupyterServer.execute(`import notebook\r\nnotebook.version_info`, 'foo.py', 0);
552+
553+
// Both should have streamed output
554+
const version = versionCells.length > 0 ? this.extractStreamOutput(versionCells[0]).trimQuotes() : '';
555+
const notebookVersion = notebookVersionCells.length > 0 ? this.extractStreamOutput(notebookVersionCells[0]).trimQuotes() : '';
556+
const pythonPath = versionCells.length > 0 ? this.extractStreamOutput(pathCells[0]).trimQuotes() : '';
557+
558+
// Both should influence our ignore count. We don't want them to count against execution
559+
this.ignoreCount = this.ignoreCount + 3;
560+
561+
// Combine this data together to make our sys info
562+
return {
563+
data: {
564+
cell_type: 'sys_info',
565+
message: message,
566+
version: version,
567+
notebook_version: localize.DataScience.notebookVersionFormat().format(notebookVersion),
568+
path: pythonPath,
569+
metadata: {},
570+
source: []
571+
},
572+
id: uuid(),
573+
file: '',
574+
line: 0,
575+
state: CellState.finished
576+
};
577+
}
573578
}
574579

575580
private addInitialSysInfo = async () : Promise<void> => {
@@ -594,7 +599,9 @@ export class History implements IWebPanelMessageListener, IHistory {
594599

595600
// Generate a new sys info cell and send it to the web panel.
596601
const sysInfo = await this.generateSysInfoCell(message);
597-
this.onAddCodeEvent([sysInfo]);
602+
if (sysInfo) {
603+
this.onAddCodeEvent([sysInfo]);
604+
}
598605
}
599606
}
600607

src/client/datascience/historycommandlistener.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,11 @@ export class HistoryCommandListener implements IDataScienceCommandListener {
194194
private async exportCellsWithOutput(ranges: {range: Range; title: string}[], document: TextDocument, file: string, cancelToken: CancellationToken) : Promise<void> {
195195
let server: INotebookServer | undefined;
196196
try {
197+
const settings = this.configuration.getSettings();
198+
const useDefaultConfig : boolean | undefined = settings.datascience.useDefaultConfigForJupyter;
199+
197200
// Try starting a server.
198-
server = await this.jupyterExecution.connectToNotebookServer(undefined, cancelToken);
201+
server = await this.jupyterExecution.connectToNotebookServer(undefined, useDefaultConfig, cancelToken);
199202

200203
// If that works, then execute all of the cells.
201204
const cells = Array.prototype.concat(... await Promise.all(ranges.map(r => {

0 commit comments

Comments
 (0)