Skip to content

Commit

Permalink
wip: remove id from TypeScript command
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonsequitur
jonsequitur committed Sep 15, 2023
1 parent eb1d776 commit 2af4347
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/polyglot-notebooks-vscode-common/src/dynamicGrammarSemanticTokenProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ export class DynamicGrammarSemanticTokenProvider {
});

const isEmpty = (text: string) => {
return text == null || text.match(/^\s*$/) !== null;
return text === null || text.match(/^\s*$/) !== null;
};

// prepare grammar scope loader
Expand Down
15 changes: 6 additions & 9 deletions src/polyglot-notebooks-vscode-common/src/interactiveClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,8 @@ export class InteractiveClient {
targetKernelName: language
}
);
if (configuration !== undefined && configuration.id !== undefined) {
command.setId(configuration.id);
}
const commandId = command.id;

const commandToken = command.token;
try {
return this.submitCode(command, language, eventEnvelope => {
if (this.deferredOutput.length > 0) {
Expand All @@ -173,7 +171,7 @@ export class InteractiveClient {
switch (eventEnvelope.eventType) {
// if kernel languages were added, handle those events here
case CommandSucceededType:
if (eventEnvelope.command?.id === commandId) {
if (eventEnvelope.command?.token === commandToken) {
// only complete this promise if it's the root command
resolve(!failureReported);
}
Expand All @@ -184,7 +182,7 @@ export class InteractiveClient {
const errorOutput = this.config.createErrorOutput(err.message, this.getNextOutputId());
outputReporter(errorOutput);
failureReported = true;
if (eventEnvelope.command?.id === commandId) {
if (eventEnvelope.command?.token === commandToken) {
// only complete this promise if it's the root command
reject(err);
}
Expand Down Expand Up @@ -404,20 +402,19 @@ export class InteractiveClient {
return new Promise<void>((resolve, reject) => {
let failureReported = false;
const token = command.getOrCreateToken();
const id = command.id;
const commandType = command.commandType;
let disposable = this.subscribeToKernelTokenEvents(token, eventEnvelope => {
switch (eventEnvelope.eventType) {
case CommandFailedType:
let err = <CommandFailed>eventEnvelope.event;
failureReported = true;
if (eventEnvelope.command?.id === id) {
if (eventEnvelope.command?.token === token) {
disposable.dispose();
reject(err);
}
break;
case CommandSucceededType:
if (eventEnvelope.command?.id === id) {
if (eventEnvelope.command?.token === token) {
disposable.dispose();
resolve();
}
Expand Down
15 changes: 2 additions & 13 deletions src/polyglot-notebooks/src/commandsAndEvents.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export interface KernelEventEnvelopeModel {

export interface KernelCommandEnvelopeModel {
token?: string;
id?: string;
commandType: contracts.KernelCommandType;
command: contracts.KernelCommand;
routingSlip?: string[];
Expand All @@ -46,7 +45,6 @@ export class KernelCommandEnvelope {

private _childCommandCounter: number = 0;
private _routingSlip: CommandRoutingSlip = new CommandRoutingSlip();
private _id: string;
private _token?: string;
private _parentCommand?: KernelCommandEnvelope;

Expand All @@ -56,11 +54,10 @@ export class KernelCommandEnvelope {

const guidBytes = uuid.parse(uuid.v4());
const data = new Uint8Array(guidBytes);
this._id = toBase64String(data);
}

public get id(): string {
return this._id;
public get token(): string | undefined {
return this._token;
}

public get routingSlip(): CommandRoutingSlip {
Expand All @@ -78,10 +75,6 @@ export class KernelCommandEnvelope {
this._parentCommand = parentCommand;
}

public setId(id: string) {
this._id = id;
}

public getOrCreateToken(): string {
if (this._token) {
return this._token;
Expand Down Expand Up @@ -129,7 +122,6 @@ export class KernelCommandEnvelope {
commandType: this.commandType,
command: this.command,
routingSlip: this._routingSlip.toArray(),
id: this._id,
token: this.getOrCreateToken()
};

Expand All @@ -139,9 +131,6 @@ export class KernelCommandEnvelope {
public static fromJson(model: KernelCommandEnvelopeModel): KernelCommandEnvelope {
const command = new KernelCommandEnvelope(model.commandType, model.command);
command._routingSlip = CommandRoutingSlip.fromUris(model.routingSlip || []);
if (model.id) {
command._id = model.id;
}
command._token = model.token;
return command;
}
Expand Down
2 changes: 1 addition & 1 deletion src/polyglot-notebooks/src/kernel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export async function submitCommandAndGetResult<TEvent extends commandsAndEvents
break;
case commandsAndEvents.CommandSucceededType:
if (areCommandsTheSame(eventEnvelope.command!, commandEnvelope)
&& (eventEnvelope.command?.id === commandEnvelope.id)) {
&& (eventEnvelope.command?.token === commandEnvelope.token)) {
if (!handled) {//? ($ ? eventEnvelope : {})
handled = true;
completionSource.reject('Command was handled before reporting expected result.');
Expand Down
5 changes: 1 addition & 4 deletions src/polyglot-notebooks/src/kernelInvocationContext.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,7 @@ export function areCommandsTheSame(envelope1: commandsAndEvents.KernelCommandEnv
if (!sameCommandType) {
return false;
}
const sameCommandId = envelope1?.id === envelope2?.id; //?
if (!sameCommandId) {
return false;
}

const sameToken = envelope1?.getOrCreateToken() === envelope2?.getOrCreateToken(); //?
if (!sameToken) {
return false;
Expand Down
15 changes: 7 additions & 8 deletions src/polyglot-notebooks/src/proxyKernel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ export class ProxyKernel extends Kernel {

private async _commandHandler(commandInvocation: IKernelCommandInvocation): Promise<void> {
const commandToken = commandInvocation.commandEnvelope.getOrCreateToken();
const commandId = commandInvocation.commandEnvelope.id;
const completionSource = new PromiseCompletionSource<commandsAndEvents.KernelEventEnvelope>();
const command = commandInvocation.commandEnvelope;
// fix : is this the right way? We are trying to avoid forwarding events we just did forward
Expand All @@ -78,7 +77,7 @@ export class ProxyKernel extends Kernel {
}
else if (envelope.command!.getOrCreateToken() === commandToken) {

Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] processing event, envelopeid=${envelope.command!.id}, commandid=${commandId}`);
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] processing event, envelopeToken=${envelope.command!.token}, commandToken=${commandToken}`);
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] processing event, ${JSON.stringify(envelope)}`);

try {
Expand Down Expand Up @@ -113,12 +112,12 @@ export class ProxyKernel extends Kernel {
break;
case commandsAndEvents.CommandFailedType:
case commandsAndEvents.CommandSucceededType:
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] finished, envelopeid=${envelope.command!.id}, commandid=${commandId}`);
if (envelope.command!.id === commandId) {
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] resolving promise, envelopeid=${envelope.command!.id}, commandid=${commandId}`);
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] finished, token=${envelope.command!.token}, commandToken=${commandToken}`);
if (envelope.command!.token === commandToken) {
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] resolving promise, envelopeToken=${envelope.command!.token}, commandToken=${commandToken}`);
completionSource.resolve(envelope);
} else {
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] not resolving promise, envelopeid=${envelope.command!.id}, commandid=${commandId}`);
Logger.default.info(`proxy name=${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] not resolving promise, envelopeToken=${envelope.command!.token}, commandToken=${commandToken}`);
this.delegatePublication(envelope, commandInvocation.context);
}
break;
Expand Down Expand Up @@ -147,12 +146,12 @@ export class ProxyKernel extends Kernel {
}
Logger.default.info(`proxy ${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] forwarding command ${commandInvocation.commandEnvelope.commandType} to ${commandInvocation.commandEnvelope.command.destinationUri}`);
this._sender.send(commandInvocation.commandEnvelope);
Logger.default.info(`proxy ${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] about to await with token ${commandToken} and commandid ${commandId}`);
Logger.default.info(`proxy ${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] about to await with token ${commandToken}`);
const enventEnvelope = await completionSource.promise;
if (enventEnvelope.eventType === commandsAndEvents.CommandFailedType) {
commandInvocation.context.fail((<commandsAndEvents.CommandFailed>enventEnvelope.event).message);
}
Logger.default.info(`proxy ${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] done awaiting with token ${commandToken}} and commandid ${commandId}`);
Logger.default.info(`proxy ${this.name}[local uri:${this.kernelInfo.uri}, remote uri:${this.kernelInfo.remoteUri}] done awaiting with token ${commandToken}}`);
}
catch (e) {
commandInvocation.context.fail((<any>e).message);
Expand Down
2 changes: 1 addition & 1 deletion src/polyglot-notebooks/src/webview/activation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function configure(global: any, context: KernelMessagingApi) {
if (arg.envelope && arg.webViewId === webViewId) {
const envelope = <connection.KernelCommandOrEventEnvelopeModel><any>(arg.envelope);
if (connection.isKernelEventEnvelopeModel(envelope)) {
Logger.default.info(`channel got ${envelope.eventType} with token ${envelope.command?.token} and id ${envelope.command?.id}`);
Logger.default.info(`channel got ${envelope.eventType} with token ${envelope.command?.token}`);
const event = KernelEventEnvelope.fromJson(envelope);
remoteToLocal.next(event);
} else {
Expand Down

0 comments on commit 2af4347

Please sign in to comment.