Skip to content
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.

Sign up for GitHub

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

Avoid waiting for completions request #9043

Merged
merged 2 commits into from
Feb 16, 2022
Merged

Avoid waiting for completions request #9043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b379428
Avoid waiting for completions request
DonJayamanne Feb 16, 2022
fd8c682
Fixes
DonJayamanne Feb 16, 2022
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/9014.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid waiting for completions during kernel startup (as completions request could fail without a response).
33 changes: 11 additions & 22 deletions src/client/datascience/jupyter/kernels/kernel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,7 @@ import {
ColorThemeKind
} from 'vscode';
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../../../common/application/types';
import {
traceDecorators,
traceError,
traceInfo,
traceInfoIfCI,
traceVerbose,
traceWarning
} from '../../../common/logger';
import { traceError, traceInfo, traceInfoIfCI, traceVerbose, traceWarning } from '../../../common/logger';
import { IFileSystem } from '../../../common/platform/types';
import { IConfigurationService, IDisposable, IDisposableRegistry, Resource } from '../../../common/types';
import { noop } from '../../../common/utils/misc';
Expand Down Expand Up @@ -662,17 +655,8 @@ export class Kernel implements IKernel {
// Restart sessions and retries might make this hard to do correctly otherwise.
notebook.session.registerCommTarget(Identifiers.DefaultCommTarget, noop);

// Request completions to warm up the completion engine (first call always takes a lot longer)
const completionPromise = this.requestEmptyCompletions();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

THe problem is we can see errors in the console, basically kernel is logging errors, however its an unhandled exception and the kernel is not able to send back a failure response.

Hence we end up waiting indefinitely for the promise to resolve as we don't ever get a response.
Fortunately in other places when we make this request, we have a timeout (to not wait and block VS Code for completions).


if (this.kernelConnectionMetadata.kind === 'connectToLiveKernel') {
// No need to wait for this to complete when connecting to a live kernel.
completionPromise.catch(noop);
} else {
traceVerbose('Waiting for completions request to complete');
await completionPromise;
traceVerbose('Completions request completed');
}
// Request completions to warm up the completion engine.
this.requestEmptyCompletions();

if (isLocalConnection(this.kernelConnectionMetadata)) {
await sendTelemetryForPythonKernelExecutable(
Expand Down Expand Up @@ -776,9 +760,14 @@ export class Kernel implements IKernel {
return result;
}

@traceDecorators.verbose('Requesting completions')
private async requestEmptyCompletions() {
await this.session?.requestComplete({
/**
* Do not wait for completions,
* If the completions request crashes then we don't get a response for this request,
* Hence we end up waiting indefinitely.
* https://github.com/microsoft/vscode-jupyter/issues/9014
*/
private requestEmptyCompletions() {
void this.session?.requestComplete({
code: '__file__.',
cursor_pos: 9
});
Expand Down