Skip to content

Have raw kernel respect disableJupyterServerAutoStart setting #12277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/12246.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Have raw kernel respect the jupyter server disable auto start setting.
9 changes: 3 additions & 6 deletions src/client/datascience/interactive-common/notebookProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
'use strict';

import { inject, injectable } from 'inversify';
import { CancellationToken, EventEmitter, Uri } from 'vscode';
import { EventEmitter, Uri } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { IFileSystem } from '../../common/platform/types';
import { IDisposableRegistry, Resource } from '../../common/types';
Expand Down Expand Up @@ -69,13 +69,10 @@ export class NotebookProvider implements INotebookProvider {
}

// Attempt to connect to our server provider, and if we do, return the connection info
public async connect(
options: ConnectNotebookProviderOptions,
token?: CancellationToken
): Promise<INotebookProviderConnection | undefined> {
public async connect(options: ConnectNotebookProviderOptions): Promise<INotebookProviderConnection | undefined> {
// Connect to either a jupyter server or a stubbed out raw notebook "connection"
if (await this.rawNotebookProvider.supported()) {
return this.rawNotebookProvider.connect(token);
return this.rawNotebookProvider.connect(options);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token was not being used here. Needed the getOnly option instead.

} else {
return this.jupyterNotebookProvider.connect(options);
}
Expand Down
27 changes: 24 additions & 3 deletions src/client/datascience/raw-kernel/rawNotebookProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ import * as localize from '../../common/utils/localize';
import { noop } from '../../common/utils/misc';
import { captureTelemetry } from '../../telemetry';
import { Telemetry } from '../constants';
import { INotebook, IRawConnection, IRawNotebookProvider, IRawNotebookSupportedService } from '../types';
import {
ConnectNotebookProviderOptions,
INotebook,
IRawConnection,
IRawNotebookProvider,
IRawNotebookSupportedService
} from '../types';

export class RawConnection implements IRawConnection {
public readonly type = 'raw';
Expand All @@ -36,7 +42,7 @@ export class RawNotebookProviderBase implements IRawNotebookProvider {
}
// Keep track of the notebooks that we have provided
private notebooks = new Map<string, Promise<INotebook>>();
private rawConnection = new RawConnection();
private rawConnection: IRawConnection | undefined;
private _id = uuid();

constructor(
Expand All @@ -47,7 +53,17 @@ export class RawNotebookProviderBase implements IRawNotebookProvider {
this.asyncRegistry.push(this);
}

public connect(): Promise<IRawConnection> {
public connect(options: ConnectNotebookProviderOptions): Promise<IRawConnection | undefined> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was pretty sure that we were respecting the jupyterServerAutoStart setting (and we were) but the actual difference was here. When the server is not auto started the first calls to it on notebook startup are getOnly connections to the server. For jupyter server it would not be autostarted, so it would return undefined instead of a connection from the getOnly call to connect. It would only start and connect on the first getOnly = false call.

However since raw kernel doesn't really have a server connection it was always returning a stub connection, even if getOnly was true. When this happens it would start up on notebook startup even if your auto start setting was off. This change make it more like a pretend server, in that it will not return a connection until a getOnly = false connection has actually been made or getConnection has been directly called.

// For getOnly, we don't want to create a connection, even though we don't have a server
// here we only want to be "connected" when requested to mimic jupyter server function
if (options.getOnly) {
return Promise.resolve(this.rawConnection);
}

// If not get only, create if needed and return
if (!this.rawConnection) {
this.rawConnection = new RawConnection();
}
return Promise.resolve(this.rawConnection);
}

Expand Down Expand Up @@ -87,6 +103,11 @@ export class RawNotebookProviderBase implements IRawNotebookProvider {
}

protected getConnection(): IRawConnection {
// At the time of getConnection force a connection if not created already
// should always have happened already, but the check here lets us avoid returning undefined option
if (!this.rawConnection) {
this.rawConnection = new RawConnection();
}
return this.rawConnection;
}

Expand Down
13 changes: 10 additions & 3 deletions src/client/datascience/raw-kernel/rawNotebookProviderWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ import { IRoleBasedObject, RoleBasedFactory } from '../jupyter/liveshare/roleBas
import { ILiveShareHasRole } from '../jupyter/liveshare/types';
import { IKernelLauncher } from '../kernel-launcher/types';
import { ProgressReporter } from '../progress/progressReporter';
import { IDataScience, INotebook, IRawConnection, IRawNotebookProvider, IRawNotebookSupportedService } from '../types';
import {
ConnectNotebookProviderOptions,
IDataScience,
INotebook,
IRawConnection,
IRawNotebookProvider,
IRawNotebookSupportedService
} from '../types';
import { GuestRawNotebookProvider } from './liveshare/guestRawNotebookProvider';
import { HostRawNotebookProvider } from './liveshare/hostRawNotebookProvider';

Expand Down Expand Up @@ -104,9 +111,9 @@ export class RawNotebookProviderWrapper implements IRawNotebookProvider, ILiveSh
return notebookProvider.supported();
}

public async connect(): Promise<IRawConnection> {
public async connect(options: ConnectNotebookProviderOptions): Promise<IRawConnection | undefined> {
const notebookProvider = await this.serverFactory.get();
return notebookProvider.connect();
return notebookProvider.connect(options);
}

public async createNotebook(
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export interface IRawNotebookSupportedService {
export const IRawNotebookProvider = Symbol('IRawNotebookProvider');
export interface IRawNotebookProvider extends IAsyncDisposable {
supported(): Promise<boolean>;
connect(token?: CancellationToken): Promise<IRawConnection>;
connect(connect: ConnectNotebookProviderOptions): Promise<IRawConnection | undefined>;
createNotebook(
identity: Uri,
resource: Resource,
Expand Down