Skip to content

Commit 8ec1977

Browse files
authored
Merge pull request #1291 from github/aeisenberg/handle-remote-cancel
Handle cancelling of remote queries
2 parents 3e388fe + 2ca0060 commit 8ec1977

5 files changed

Lines changed: 106 additions & 11 deletions

File tree

extensions/ql-vscode/src/authentication.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const GITHUB_AUTH_PROVIDER_ID = 'github';
77
// https://docs.github.com/apps/building-oauth-apps/understanding-scopes-for-oauth-apps
88
const SCOPES = ['repo'];
99

10-
/**
10+
/**
1111
* Handles authentication to GitHub, using the VS Code [authentication API](https://code.visualstudio.com/api/references/vscode-api#authentication).
1212
*/
1313
export class Credentials {
@@ -18,6 +18,15 @@ export class Credentials {
1818
// eslint-disable-next-line @typescript-eslint/no-empty-function
1919
private constructor() { }
2020

21+
/**
22+
* Initializes an instance of credentials with an octokit instance.
23+
*
24+
* Do not call this method until you know you actually need an instance of credentials.
25+
* since calling this method will require the user to log in.
26+
*
27+
* @param context The extension context.
28+
* @returns An instance of credentials.
29+
*/
2130
static async initialize(context: vscode.ExtensionContext): Promise<Credentials> {
2231
const c = new Credentials();
2332
c.registerListeners(context);

extensions/ql-vscode/src/query-history.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import { QueryStatus } from './query-status';
3636
import { slurpQueryHistory, splatQueryHistory } from './query-serialization';
3737
import * as fs from 'fs-extra';
3838
import { CliVersionConstraint } from './cli';
39+
import { Credentials } from './authentication';
40+
import { cancelRemoteQuery } from './remote-queries/gh-actions-api-client';
3941

4042
/**
4143
* query-history.ts
@@ -316,7 +318,7 @@ export class QueryHistoryManager extends DisposableObject {
316318
private qs: QueryServerClient,
317319
private dbm: DatabaseManager,
318320
private queryStorageDir: string,
319-
ctx: ExtensionContext,
321+
private ctx: ExtensionContext,
320322
private queryHistoryConfigListener: QueryHistoryConfig,
321323
private doCompareCallback: (
322324
from: CompletedLocalQueryInfo,
@@ -512,6 +514,10 @@ export class QueryHistoryManager extends DisposableObject {
512514
this.registerQueryHistoryScrubber(queryHistoryConfigListener, ctx);
513515
}
514516

517+
private getCredentials() {
518+
return Credentials.initialize(this.ctx);
519+
}
520+
515521
/**
516522
* Register and create the history scrubber.
517523
*/
@@ -816,7 +822,7 @@ export class QueryHistoryManager extends DisposableObject {
816822
}
817823

818824
if (finalSingleItem.evalLogSummaryLocation) {
819-
await this.tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
825+
await this.tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
820826
} else {
821827
this.warnNoEvalLogSummary();
822828
}
@@ -830,11 +836,20 @@ export class QueryHistoryManager extends DisposableObject {
830836
// In the future, we may support cancelling remote queries, but this is not a short term plan.
831837
const { finalSingleItem, finalMultiSelect } = this.determineSelection(singleItem, multiSelect);
832838

833-
(finalMultiSelect || [finalSingleItem]).forEach((item) => {
834-
if (item.status === QueryStatus.InProgress && item.t === 'local') {
835-
item.cancel();
839+
const selected = finalMultiSelect || [finalSingleItem];
840+
const results = selected.map(async item => {
841+
if (item.status === QueryStatus.InProgress) {
842+
if (item.t === 'local') {
843+
item.cancel();
844+
} else if (item.t === 'remote') {
845+
void showAndLogInformationMessage('Cancelling variant analysis. This may take a while.');
846+
const credentials = await this.getCredentials();
847+
await cancelRemoteQuery(credentials, item.remoteQuery);
848+
}
836849
}
837850
});
851+
852+
await Promise.all(results);
838853
}
839854

840855
async handleShowQueryText(

extensions/ql-vscode/src/remote-queries/gh-actions-api-client.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,18 @@ export async function getRemoteQueryIndex(
7474
};
7575
}
7676

77+
export async function cancelRemoteQuery(
78+
credentials: Credentials,
79+
remoteQuery: RemoteQuery
80+
): Promise<void> {
81+
const octokit = await credentials.getOctokit();
82+
const { actionsWorkflowRunId, controllerRepository: { owner, name } } = remoteQuery;
83+
const response = await octokit.request(`POST /repos/${owner}/${name}/actions/runs/${actionsWorkflowRunId}/cancel`);
84+
if (response.status >= 300) {
85+
throw new Error(`Error cancelling variant analysis: ${response.status} ${response?.data?.message || ''}`);
86+
}
87+
}
88+
7789
export async function downloadArtifactFromLink(
7890
credentials: Credentials,
7991
storagePath: string,

extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as fs from 'fs-extra';
66
import { Credentials } from '../authentication';
77
import { CodeQLCliServer } from '../cli';
88
import { ProgressCallback } from '../commandRunner';
9-
import { createTimestampFile, showAndLogErrorMessage, showInformationMessageWithAction } from '../helpers';
9+
import { createTimestampFile, showAndLogErrorMessage, showAndLogInformationMessage, showInformationMessageWithAction } from '../helpers';
1010
import { Logger } from '../logging';
1111
import { runRemoteQuery } from './run-remote-query';
1212
import { RemoteQueriesInterfaceManager } from './remote-queries-interface';
@@ -155,13 +155,20 @@ export class RemoteQueriesManager extends DisposableObject {
155155
queryItem.status = QueryStatus.Failed;
156156
}
157157
} else if (queryWorkflowResult.status === 'CompletedUnsuccessfully') {
158-
queryItem.failureReason = queryWorkflowResult.error;
159-
queryItem.status = QueryStatus.Failed;
160-
void showAndLogErrorMessage(`Variant analysis execution failed. Error: ${queryWorkflowResult.error}`);
158+
if (queryWorkflowResult.error?.includes('cancelled')) {
159+
// workflow was cancelled on the server
160+
queryItem.failureReason = 'Cancelled';
161+
queryItem.status = QueryStatus.Failed;
162+
void showAndLogInformationMessage('Variant analysis was cancelled');
163+
} else {
164+
queryItem.failureReason = queryWorkflowResult.error;
165+
queryItem.status = QueryStatus.Failed;
166+
void showAndLogErrorMessage(`Variant analysis execution failed. Error: ${queryWorkflowResult.error}`);
167+
}
161168
} else if (queryWorkflowResult.status === 'Cancelled') {
162169
queryItem.failureReason = 'Cancelled';
163170
queryItem.status = QueryStatus.Failed;
164-
void showAndLogErrorMessage('Variant analysis monitoring was cancelled');
171+
void showAndLogErrorMessage('Variant analysis was cancelled');
165172
} else if (queryWorkflowResult.status === 'InProgress') {
166173
// Should not get here. Only including this to ensure `assertNever` uses proper type checking.
167174
void showAndLogErrorMessage(`Unexpected status: ${queryWorkflowResult.status}`);
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { expect } from 'chai';
2+
import * as sinon from 'sinon';
3+
import { Credentials } from '../../../authentication';
4+
import { cancelRemoteQuery } from '../../../remote-queries/gh-actions-api-client';
5+
import { RemoteQuery } from '../../../remote-queries/remote-query';
6+
7+
describe('gh-actions-api-client', () => {
8+
let sandbox: sinon.SinonSandbox;
9+
let mockCredentials: Credentials;
10+
let mockResponse: sinon.SinonStub<any, Promise<{ status: number }>>;
11+
12+
beforeEach(() => {
13+
sandbox = sinon.createSandbox();
14+
mockCredentials = {
15+
getOctokit: () => Promise.resolve({
16+
request: mockResponse
17+
})
18+
} as unknown as Credentials;
19+
});
20+
21+
afterEach(() => {
22+
sandbox.restore();
23+
});
24+
25+
describe('cancelRemoteQuery', () => {
26+
it('should cancel a remote query', async () => {
27+
mockResponse = sinon.stub().resolves({ status: 202 });
28+
await cancelRemoteQuery(mockCredentials, createMockRemoteQuery());
29+
30+
expect(mockResponse.calledOnce).to.be.true;
31+
expect(mockResponse.firstCall.args[0]).to.equal('POST /repos/github/codeql/actions/runs/123/cancel');
32+
});
33+
34+
it('should fail to cancel a remote query', async () => {
35+
mockResponse = sinon.stub().resolves({ status: 409, data: { message: 'Uh oh!' } });
36+
37+
await expect(cancelRemoteQuery(mockCredentials, createMockRemoteQuery())).to.be.rejectedWith(/Error cancelling variant analysis: 409 Uh oh!/);
38+
expect(mockResponse.calledOnce).to.be.true;
39+
expect(mockResponse.firstCall.args[0]).to.equal('POST /repos/github/codeql/actions/runs/123/cancel');
40+
});
41+
42+
function createMockRemoteQuery(): RemoteQuery {
43+
return {
44+
actionsWorkflowRunId: 123,
45+
controllerRepository: {
46+
owner: 'github',
47+
name: 'codeql'
48+
}
49+
} as unknown as RemoteQuery;
50+
}
51+
});
52+
});

0 commit comments

Comments
 (0)