-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Have raw kernel respect the jupyter server disable auto start setting. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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( | ||
|
@@ -47,7 +53,17 @@ export class RawNotebookProviderBase implements IRawNotebookProvider { | |
this.asyncRegistry.push(this); | ||
} | ||
|
||
public connect(): Promise<IRawConnection> { | ||
public connect(options: ConnectNotebookProviderOptions): Promise<IRawConnection | undefined> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
|
||
|
@@ -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; | ||
} | ||
|
||
|
There was a problem hiding this comment.
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.