Skip to content

Commit

Permalink
Make the Jupyter supervisor the default in non-desktop Positron confi…
Browse files Browse the repository at this point in the history
…gurations (#5215)

This change makes Kallichore the default in non-desktop configurations
of Positron, such as on Posit Workbench. In addition to the
configuration change, it includes a number of other changes that improve
the stability of the supervisor integration in order to bring it up to
the quality needed.

In particular:
- The supervisor's behavior around session starting has changed; the RPC
that starts sessions now doesn't return until the session has fully
started and returned kernel information. We pick up this kernel info in
Positron rather than making our own RPC for kernel info (when starting
for the first time).
- We now hold any attempts to make UI Comm requests until the UI Comm
has opened. This eliminates a lot of `setConsoleWidth` errors in the
logs.
- The Kallichore terminal is now hidden by default.
- The R and Python extensions no longer have a hard dependency on the
Jupyter Adapter extension. This allows them to work even when the
Jupyter Adapter is unbootable (such as on REHL 9 or other systems where
there are glibc issues with ZeroMQ)

Addresses #4941. 

### QA Notes

If logging issues, it's helpful to include the Kallichore logs, so run
with the "Show Terminal" setting (added in this PR) turned on.

---------

Signed-off-by: Jonathan <[email protected]>
Co-authored-by: sharon <[email protected]>
  • Loading branch information
jmcphers and sharon-wang authored Oct 30, 2024
1 parent 578e92c commit d6e9e79
Show file tree
Hide file tree
Showing 12 changed files with 328 additions and 59 deletions.
7 changes: 6 additions & 1 deletion extensions/kallichore-adapter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
"default": false,
"description": "%configuration.enable.description%"
},
"kallichoreSupervisor.showTerminal": {
"type": "boolean",
"default": false,
"description": "%configuration.showTerminal.description%"
},
"kallichoreSupervisor.logLevel": {
"scope": "window",
"type": "string",
Expand Down Expand Up @@ -84,7 +89,7 @@
},
"positron": {
"binaryDependencies": {
"kallichore": "0.1.13"
"kallichore": "0.1.15"
}
},
"dependencies": {
Expand Down
3 changes: 2 additions & 1 deletion extensions/kallichore-adapter/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
"configuration.logLevel.debug.description": "Debug messages",
"configuration.logLevel.trace.description": "Verbose tracing messages",
"configuration.logLevel.description": "Log level for the kernel supervisor (restart Positron to apply)",
"configuration.enable.description": "Run Jupyter kernels under the Kallichore supervisor",
"configuration.enable.description": "Run Jupyter kernels under the Kallichore supervisor in Positron Desktop. (The supervisor is always enabled in server configurations.)",
"configuration.showTerminal.description": "Show the host terminal for the Kallichore supervisor",
"configuration.attachOnStartup.description": "Run <f5> before starting up Jupyter kernel (when supported)",
"configuration.sleepOnStartup.description": "Sleep for n seconds before starting up Jupyter kernel (when supported)"
}
24 changes: 22 additions & 2 deletions extensions/kallichore-adapter/src/KallichoreAdapterApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ export class KCApi implements KallichoreAdapterApi {
// Start a timer so we can track server startup time
const startTime = Date.now();

// Consult configuration to see if we should show this terminal
const showTerminal = config.get<boolean>('showTerminal', false);

// Start the server in a new terminal
this._log.info(`Starting Kallichore server ${shellPath} on port ${port}`);
const terminal = vscode.window.createTerminal(<vscode.TerminalOptions>{
Expand All @@ -191,7 +194,7 @@ export class KCApi implements KallichoreAdapterApi {
shellArgs: ['--port', port.toString(), '--token', tokenPath],
env,
message: `*** Kallichore Server (${shellPath}) ***`,
hideFromUser: false,
hideFromUser: !showTerminal,
isTransient: false
});

Expand All @@ -211,12 +214,20 @@ export class KCApi implements KallichoreAdapterApi {
this._log.info(`Kallichore ${status.body.version} server online with ${status.body.sessions} sessions`);
break;
} catch (err) {
const elapsed = Date.now() - startTime;

// ECONNREFUSED is a normal condition during startup; the server
// isn't ready yet. Keep trying until we hit the retry limit,
// about 2 seconds from the time we got a process ID
// established.
if (err.code === 'ECONNREFUSED') {
if (retry < 19) {
// Log every few attempts. We don't want to overwhelm
// the logs, and it's normal for us to encounter a few
// connection refusals before the server is ready.
if (retry % 5 === 0) {
this._log.debug(`Waiting for Kallichore server to start (attempt ${retry + 1}, ${elapsed}ms)`);
}
// Wait a bit and try again
await new Promise((resolve) => setTimeout(resolve, 50));
continue;
Expand All @@ -226,7 +237,16 @@ export class KCApi implements KallichoreAdapterApi {
throw new Error(`Kallichore server did not start after ${Date.now() - startTime}ms`);
}
}
this._log.error(`Failed to get session list from Kallichore; ` +

// If the request times out, go ahead and try again as long as
// it hasn't been more than 10 seconds since we started. This
// can happen if the server is slow to start.
if (err.code === 'ETIMEDOUT' && elapsed < 10000) {
this._log.info(`Request for server status timed out; retrying (attempt ${retry + 1}, ${elapsed}ms)`);
continue;
}

this._log.error(`Failed to get initial server status from Kallichore; ` +
`server may not be running or may not be ready. Check the terminal for errors. ` +
`Error: ${JSON.stringify(err)}`);
throw err;
Expand Down
Loading

0 comments on commit d6e9e79

Please sign in to comment.