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.

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

Improvements to dismissing a deployment #2522

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e162d4c
Base functionality working, with existing go unit tests
sagerb Jan 9, 2025
c8de8a5
Merge remote-tracking branch 'origin/main' into sagerb-support-dismis…
sagerb Jan 9, 2025
a37f351
add unit tests to confirm new logic in deployment.WriteFile
sagerb Jan 9, 2025
905c2f6
go lint fixes.
sagerb Jan 10, 2025
21c0b1e
Update to publisher log display, unit tests on cancel API (with small…
sagerb Jan 10, 2025
f00ecce
remove flash of success status when canceling a deployment. Also fixe…
sagerb Jan 11, 2025
1cd5c97
Update extensions/vscode/src/views/deployProgress.ts
sagerb Jan 11, 2025
fd46585
PR requested changes
sagerb Jan 13, 2025
f26aa71
PR Changes
sagerb Jan 13, 2025
0304262
PR requested changes
sagerb Jan 13, 2025
f89cf19
lower sensitivity per discussion
sagerb Jan 14, 2025
1ebf6ec
re-use same style for date/times
sagerb Jan 14, 2025
81d645f
Show View Content button for canceled deployments if applicable
sagerb Jan 14, 2025
e1c920e
Merge remote-tracking branch 'origin/main' into sagerb-support-dismis…
sagerb Jan 14, 2025
4e66302
PR requests.
sagerb Jan 14, 2025
ba59b43
Merge pull request #2525 from posit-dev/sagerb-support-dismissed-depl…
sagerb Jan 14, 2025
d302af9
Merge remote-tracking branch 'origin/sagerb-support-dismissed-deploym…
sagerb Jan 14, 2025
b7e337d
slightly reducing sensitivity
sagerb Jan 14, 2025
e7c08b0
reduction in precision
sagerb Jan 14, 2025
158ded1
switch div to template, to remove an unneeded level
sagerb Jan 14, 2025
5e2b8a9
regen statistics
sagerb Jan 14, 2025
0376c4d
Merge pull request #2524 from posit-dev/sagerb-support-dismissed-depl…
sagerb Jan 15, 2025
fe75e84
Rewording canceled to dismissed and show as informational message rat…
sagerb Jan 15, 2025
c1cf4b0
Merge remote-tracking branch 'origin/main' into sagerb-support-dismis…
sagerb Jan 15, 2025
975f886
Merge remote-tracking branch 'origin/sagerb-support-dismissed-deploym…
sagerb Jan 15, 2025
03d4bda
Merge pull request #2537 from posit-dev/sagerb-rename-cancel-deployment
sagerb Jan 16, 2025
4637edc
Merge branch 'main' into sagerb-support-dismissed-deployment
sagerb Jan 16, 2025
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
18 changes: 18 additions & 0 deletions extensions/vscode/src/api/resources/ContentRecords.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,22 @@ export class ContentRecords {
},
);
}

// Returns:
// 200 - success
// 404 - not found
// 500 - internal server error
cancelDeployment(deploymentName: string, dir: string, localId: string) {
const encodedName = encodeURIComponent(deploymentName);
dotNomad marked this conversation as resolved.
Show resolved Hide resolved
const encodedLocalId = encodeURIComponent(localId);
return this.client.post<ContentRecord>(
`deployments/${encodedName}/cancel/${encodedLocalId}`,
{},
{
params: {
dir,
},
},
);
}
}
2 changes: 2 additions & 0 deletions extensions/vscode/src/api/types/contentRecords.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type ContentRecordRecord = {
serverUrl: string;
saveName: string;
createdAt: string;
localId: string;
abortedAt: string;
configurationName: string;
type: ContentType;
deploymentError: AgentError | null;
Expand Down
2 changes: 1 addition & 1 deletion extensions/vscode/src/api/types/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1254,7 +1254,7 @@ export interface PublishFailure extends EventStreamMessage {
data: {
dashboardUrl: string;
url: string;
canceled?: string; // not defined if not user cancelled. Value of "true" if true.
canceled?: string; // not defined if not user canceled. Value of "true" if true.
// and other non-defined attributes
};
error: string; // translated internally
Expand Down
25 changes: 17 additions & 8 deletions extensions/vscode/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ export function displayEventStreamMessage(msg: EventStreamMessage): string {
if (msg.data.dashboardUrl) {
return `Deployment failed, click to view Connect logs: ${msg.data.dashboardUrl}`;
}
if (msg.data.canceled === "true") {
return "Deployment dismissed";
}
return "Deployment failed";
}
if (msg.error !== undefined) {
Expand All @@ -95,8 +98,8 @@ export class EventStream extends Readable implements Disposable {
private messages: EventStreamMessage[] = [];
// Map to store event callbacks
private callbacks: Map<string, EventStreamRegistration[]> = new Map();
// Cancelled Event Streams - Suppressed when received
private cancelledLocalIDs: string[] = [];
// Canceled Event Streams - Suppressed when received
private canceledLocalIDs: string[] = [];

/**
* Creates a new instance of the EventStream class.
Expand Down Expand Up @@ -170,19 +173,25 @@ export class EventStream extends Readable implements Disposable {
* @returns undefined
*/
public suppressMessages(localId: string) {
this.cancelledLocalIDs.push(localId);
this.canceledLocalIDs.push(localId);
}

private processMessage(msg: EventStreamMessage) {
const localId = msg.data.localId;
if (localId && this.cancelledLocalIDs.includes(localId)) {
// Some log messages passed on from Connect include
// the localId using snake_case, rather than pascalCase.
// To filter correctly, we need to check for both.

const localId = msg.data.localId || msg.data.local_id;
if (localId && this.canceledLocalIDs.includes(localId)) {
// suppress and ignore
return;
}

// Trace message
// console.debug(
// `eventSource trace: ${event.type}: ${JSON.stringify(event)}`,
// );
// Uncomment the following code if you want to dump every message to the
// console as it is received.
// console.debug(`eventSource trace: ${msg.type}: ${JSON.stringify(msg)}`);

// Add the message to the messages array
this.messages.push(msg);
// Emit a 'message' event with the message as the payload
Expand Down
2 changes: 1 addition & 1 deletion extensions/vscode/src/multiStepInputs/newCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export async function newCredential(
totalSteps: -1,
data: {
// each attribute is initialized to undefined
// to be returned when it has not been cancelled
// to be returned when it has not been canceled
url: startingServerUrl, // eventual type is string
apiKey: <string | undefined>undefined, // eventual type is string
name: <string | undefined>undefined, // eventual type is string
Expand Down
2 changes: 1 addition & 1 deletion extensions/vscode/src/multiStepInputs/newDeployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export async function newDeployment(
title: "Select Entrypoint File (main file for your project)",
});
if (!fileUris || !fileUris[0]) {
// cancelled.
// canceled.
continue;
}
const fileUri = fileUris[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export async function selectNewOrExistingConfig(
totalSteps: -1,
data: {
// each attribute is initialized to undefined
// to be returned when it has not been cancelled to assist type guards
// to be returned when it has not been canceled to assist type guards
// Note: We can't initialize existingConfigurationName to a specific initial
// config, as we then wouldn't be able to detect if the user hit ESC to exit
// the selection. :-(
Expand Down
20 changes: 20 additions & 0 deletions extensions/vscode/src/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,26 @@ export class PublisherState implements Disposable {
}
}

updateContentRecord(
newValue: ContentRecord | PreContentRecord | PreContentRecordWithConfig,
) {
const existingContentRecord = this.findContentRecord(
newValue.saveName,
newValue.projectDir,
);
if (existingContentRecord) {
const crIndex = this.contentRecords.findIndex(
(contentRecord) =>
contentRecord.deploymentPath === existingContentRecord.deploymentPath,
);
if (crIndex !== -1) {
this.contentRecords[crIndex] = newValue;
} else {
this.contentRecords.push(newValue);
}
}
}

async getSelectedConfiguration() {
const contentRecord = await this.getSelectedContentRecord();
if (!contentRecord) {
Expand Down
4 changes: 4 additions & 0 deletions extensions/vscode/src/test/unit-test-utils/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export const preContentRecordFactory = Factory.define<PreContentRecord>(
serverUrl: `https://connect-test-${sequence}/connect`,
saveName: `Report ${sequence}`,
createdAt: new Date().toISOString(),
localId: "",
abortedAt: "",
configurationName: `report-GUD${sequence}`,
type: ContentType.RMD,
deploymentError: null,
Expand All @@ -67,6 +69,8 @@ export const contentRecordFactory = Factory.define<ContentRecord>(
serverUrl: `https://connect-test-${sequence}/connect`,
saveName: `Report ${sequence}`,
createdAt: new Date().toISOString(),
localId: "",
abortedAt: "",
deployedAt: new Date().toISOString(),
configurationName: `report-GUD${sequence}`,
type: ContentType.RMD,
Expand Down
75 changes: 56 additions & 19 deletions extensions/vscode/src/views/deployProgress.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,28 @@
// Copyright (C) 2024 by Posit Software, PBC.

import { ProgressLocation, Uri, env, window } from "vscode";
import { EventStreamMessage, eventMsgToString } from "src/api";
import {
EventStreamMessage,
eventMsgToString,
useApi,
ContentRecord,
PreContentRecord,
PreContentRecordWithConfig,
} from "src/api";
import { EventStream, UnregisterCallback } from "src/events";
import { getSummaryStringFromError } from "src/utils/errors";

export function deployProject(localID: string, stream: EventStream) {
type UpdateActiveContentRecordCB = (
contentRecord: ContentRecord | PreContentRecord | PreContentRecordWithConfig,
) => void;

export function deployProject(
deploymentName: string,
dir: string,
localID: string,
stream: EventStream,
updateActiveContentRecordCB: UpdateActiveContentRecordCB,
) {
window.withProgress(
{
location: ProgressLocation.Notification,
Expand All @@ -27,25 +45,44 @@ export function deployProject(localID: string, stream: EventStream) {
registrations.forEach((cb) => cb.unregister());
};

token.onCancellationRequested(() => {
token.onCancellationRequested(async () => {
const api = await useApi();
streamID = "NEVER_A_VALID_STREAM";
unregisterAll();
// inject a psuedo end of publishing event
stream.injectMessage({
type: "publish/failure",
time: Date.now().toString(),
data: {
dashboardUrl: "",
url: "",
// and other non-defined attributes
localId: localID,
cancelled: "true",
message:
"Deployment has been dismissed (but will continue to be processed on the Connect Server). Deployment status will be reset to the prior known state.",
},
error: "Deployment has been dismissed.",
});
stream.suppressMessages(localID);
try {
const response = await api.contentRecords.cancelDeployment(
deploymentName,
dir,
localID,
);

// update the UX locally
updateActiveContentRecordCB(response.data);

// we must have been successful...
// inject a psuedo end of publishing event
stream.injectMessage({
type: "publish/failure",
time: Date.now().toString(),
data: {
dashboardUrl: "",
url: "",
// and other non-defined attributes
localId: localID,
canceled: "true",
message:
"Deployment has been dismissed, but will continue to be processed on the Connect Server.",
},
error: "Deployment has been dismissed.",
});
stream.suppressMessages(localID);
} catch (error: unknown) {
const summary = getSummaryStringFromError(
"deployProject, token.onCancellationRequested",
error,
);
window.showErrorMessage(`Unable to dismiss deployment: ${summary}`);
}
resolveCB();
});

Expand Down
24 changes: 22 additions & 2 deletions extensions/vscode/src/views/homeView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
useApi,
AllContentRecordTypes,
EnvironmentConfig,
PreContentRecordWithConfig,
} from "src/api";
import { EventStream } from "src/events";
import { getPythonInterpreterPath, getRInterpreterPath } from "../utils/vscode";
Expand Down Expand Up @@ -208,7 +209,13 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
secrets,
r,
);
deployProject(response.data.localId, this.stream);
deployProject(
deploymentName,
projectDir,
response.data.localId,
this.stream,
this.updateActiveContentRecordLocally.bind(this),
);
} catch (error: unknown) {
// Most failures will occur on the event stream. These are the ones which
// are immediately rejected as part of the API request to initiate deployment.
Expand Down Expand Up @@ -310,6 +317,19 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
}
}

private updateActiveContentRecordLocally(
activeContentRecord:
| ContentRecord
| PreContentRecord
| PreContentRecordWithConfig,
) {
// update our local state, so we don't wait on file refreshes
this.state.updateContentRecord(activeContentRecord);

// refresh the webview
this.updateWebViewViewContentRecords();
}

private onPublishStart() {
this.webviewConduit.sendMsg({
kind: HostToWebviewMessageType.PUBLISH_START,
Expand Down Expand Up @@ -949,7 +969,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable {
activeConfig.configuration.environment,
);
if (name === undefined) {
// Cancelled by the user
// Canceled by the user
return;
}

Expand Down
33 changes: 27 additions & 6 deletions extensions/vscode/src/views/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ enum LogStageStatus {
inProgress,
completed,
failed,
canceled,
}

type LogStage = {
Expand Down Expand Up @@ -182,14 +183,17 @@ export class LogsTreeDataProvider implements TreeDataProvider<LogsTreeItem> {
});

this.stream.register("publish/failure", async (msg: EventStreamMessage) => {
this.publishingStage.status = LogStageStatus.failed;
const failedOrCanceledStatus = msg.data.canceled
? LogStageStatus.canceled
: LogStageStatus.failed;
this.publishingStage.status = failedOrCanceledStatus;
this.publishingStage.events.push(msg);

this.stages.forEach((stage) => {
if (stage.status === LogStageStatus.notStarted) {
stage.status = LogStageStatus.neverStarted;
} else if (stage.status === LogStageStatus.inProgress) {
stage.status = LogStageStatus.failed;
stage.status = failedOrCanceledStatus;
}
});

Expand All @@ -204,11 +208,19 @@ export class LogsTreeDataProvider implements TreeDataProvider<LogsTreeItem> {
errorMessage = handleEventCodedError(msg);
} else {
errorMessage =
msg.data.cancelled === "true"
? `Deployment cancelled: ${msg.data.message}`
msg.data.canceled === "true"
? msg.data.message
: `Deployment failed: ${msg.data.message}`;
}
const selection = await window.showErrorMessage(errorMessage, ...options);
let selection: string | undefined;
if (msg.data.canceled === "true") {
selection = await window.showInformationMessage(
errorMessage,
...options,
);
} else {
selection = await window.showErrorMessage(errorMessage, ...options);
}
if (selection === showLogsOption) {
await commands.executeCommand(Commands.Logs.Focus);
} else if (selection === enhancedError?.buttonStr) {
Expand Down Expand Up @@ -259,7 +271,11 @@ export class LogsTreeDataProvider implements TreeDataProvider<LogsTreeItem> {
(msg: EventStreamMessage) => {
const stage = this.stages.get(stageName);
if (stage) {
stage.status = LogStageStatus.failed;
if (msg.data.canceled === "true") {
stage.status = LogStageStatus.canceled;
} else {
stage.status = LogStageStatus.failed;
}
stage.events.push(msg);
}
this.refresh();
Expand Down Expand Up @@ -413,6 +429,11 @@ export class LogsTreeStageItem extends TreeItem {
this.iconPath = new ThemeIcon("error");
this.collapsibleState = TreeItemCollapsibleState.Expanded;
break;
case LogStageStatus.canceled:
this.label = this.stage.inactiveLabel;
this.iconPath = new ThemeIcon("circle-slash");
this.collapsibleState = TreeItemCollapsibleState.Expanded;
break;
}
}
}
Expand Down
Loading
Loading