Skip to content

Commit 4b93d34

Browse files
Merge pull request #2784 from github/robertbrignull/saving-progress
Make progress notifications more useful when calling loadExternalApiUsages
2 parents 5922bdf + 1e2acb0 commit 4b93d34

File tree

2 files changed

+108
-60
lines changed

2 files changed

+108
-60
lines changed

extensions/ql-vscode/src/model-editor/external-api-usage-queries.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ export async function prepareExternalApiQuery(
5858
return true;
5959
}
6060

61+
export const externalApiQueriesProgressMaxStep = 2000;
62+
6163
export async function runExternalApiQueries(
6264
mode: Mode,
6365
{
@@ -76,6 +78,11 @@ export async function runExternalApiQueries(
7678
// For a reference of what this should do in the future, see the previous implementation in
7779
// https://github.com/github/vscode-codeql/blob/089d3566ef0bc67d9b7cc66e8fd6740b31c1c0b0/extensions/ql-vscode/src/data-extensions-editor/external-api-usage-query.ts#L33-L72
7880

81+
progress({
82+
message: "Resolving QL packs",
83+
step: 1,
84+
maxStep: externalApiQueriesProgressMaxStep,
85+
});
7986
const additionalPacks = getOnDiskWorkspaceFolders();
8087
const extensionPacks = Object.keys(
8188
await cliServer.resolveQlpacks(additionalPacks, true),
@@ -92,7 +99,12 @@ export async function runExternalApiQueries(
9299
queryStorageDir,
93100
additionalPacks,
94101
extensionPacks,
95-
progress,
102+
progress: (update) =>
103+
progress({
104+
step: update.step + 500,
105+
maxStep: externalApiQueriesProgressMaxStep,
106+
message: update.message,
107+
}),
96108
token,
97109
// We need to create a lock file, because the query is inside our own pack
98110
createLockFile: true,
@@ -105,8 +117,8 @@ export async function runExternalApiQueries(
105117
// Read the results and covert to internal representation
106118
progress({
107119
message: "Decoding results",
108-
step: 1100,
109-
maxStep: 1500,
120+
step: 1600,
121+
maxStep: externalApiQueriesProgressMaxStep,
110122
});
111123

112124
const bqrsChunk = await readQueryResults({
@@ -119,8 +131,8 @@ export async function runExternalApiQueries(
119131

120132
progress({
121133
message: "Finalizing results",
122-
step: 1450,
123-
maxStep: 1500,
134+
step: 1950,
135+
maxStep: externalApiQueriesProgressMaxStep,
124136
});
125137

126138
return decodeBqrsToExternalApiUsages(bqrsChunk);

extensions/ql-vscode/src/model-editor/model-editor-view.ts

Lines changed: 91 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ import { promptImportGithubDatabase } from "../databases/database-fetcher";
2727
import { App } from "../common/app";
2828
import { showResolvableLocation } from "../databases/local-databases/locations";
2929
import { redactableError } from "../common/errors";
30-
import { runExternalApiQueries } from "./external-api-usage-queries";
30+
import {
31+
externalApiQueriesProgressMaxStep,
32+
runExternalApiQueries,
33+
} from "./external-api-usage-queries";
3134
import { Method, Usage } from "./method";
3235
import { ModeledMethod } from "./modeled-method";
3336
import { ExtensionPack } from "./shared/extension-pack";
@@ -190,7 +193,10 @@ export class ModelEditorView extends AbstractWebview<
190193

191194
break;
192195
case "refreshMethods":
193-
await this.loadExternalApiUsages();
196+
await withProgress((progress) => this.loadExternalApiUsages(progress), {
197+
cancellable: false,
198+
});
199+
194200
void telemetryListener?.sendUIInteraction(
195201
"model-editor-refresh-methods",
196202
);
@@ -202,17 +208,39 @@ export class ModelEditorView extends AbstractWebview<
202208

203209
break;
204210
case "saveModeledMethods":
205-
await saveModeledMethods(
206-
this.extensionPack,
207-
this.databaseItem.name,
208-
this.databaseItem.language,
209-
msg.methods,
210-
msg.modeledMethods,
211-
this.mode,
212-
this.cliServer,
213-
this.app.logger,
211+
await withProgress(
212+
async (progress) => {
213+
progress({
214+
step: 1,
215+
maxStep: 500 + externalApiQueriesProgressMaxStep,
216+
message: "Writing model files",
217+
});
218+
await saveModeledMethods(
219+
this.extensionPack,
220+
this.databaseItem.name,
221+
this.databaseItem.language,
222+
msg.methods,
223+
msg.modeledMethods,
224+
this.mode,
225+
this.cliServer,
226+
this.app.logger,
227+
);
228+
229+
await Promise.all([
230+
this.setViewState(),
231+
this.loadExternalApiUsages((update) =>
232+
progress({
233+
...update,
234+
step: update.step + 500,
235+
maxStep: 500 + externalApiQueriesProgressMaxStep,
236+
}),
237+
),
238+
]);
239+
},
240+
{
241+
cancellable: false,
242+
},
214243
);
215-
await Promise.all([this.setViewState(), this.loadExternalApiUsages()]);
216244
void telemetryListener?.sendUIInteraction(
217245
"model-editor-save-modeled-methods",
218246
);
@@ -249,7 +277,12 @@ export class ModelEditorView extends AbstractWebview<
249277
break;
250278
case "switchMode":
251279
this.mode = msg.mode;
252-
await Promise.all([this.setViewState(), this.loadExternalApiUsages()]);
280+
await Promise.all([
281+
this.setViewState(),
282+
withProgress((progress) => this.loadExternalApiUsages(progress), {
283+
cancellable: false,
284+
}),
285+
]);
253286
void telemetryListener?.sendUIInteraction("model-editor-switch-modes");
254287

255288
break;
@@ -274,7 +307,9 @@ export class ModelEditorView extends AbstractWebview<
274307

275308
await Promise.all([
276309
this.setViewState(),
277-
this.loadExternalApiUsages(),
310+
withProgress((progress) => this.loadExternalApiUsages(progress), {
311+
cancellable: false,
312+
}),
278313
this.loadExistingModeledMethods(),
279314
]);
280315
}
@@ -317,48 +352,49 @@ export class ModelEditorView extends AbstractWebview<
317352
}
318353
}
319354

320-
protected async loadExternalApiUsages(): Promise<void> {
321-
await withProgress(
322-
async (progress) => {
323-
try {
324-
const cancellationTokenSource = new CancellationTokenSource();
325-
const queryResult = await runExternalApiQueries(this.mode, {
326-
cliServer: this.cliServer,
327-
queryRunner: this.queryRunner,
328-
databaseItem: this.databaseItem,
329-
queryStorageDir: this.queryStorageDir,
330-
queryDir: this.queryDir,
331-
progress: (update) => progress({ ...update, maxStep: 1500 }),
332-
token: cancellationTokenSource.token,
333-
});
334-
if (!queryResult) {
335-
return;
336-
}
337-
this.methods = queryResult;
355+
protected async loadExternalApiUsages(
356+
progress: ProgressCallback,
357+
): Promise<void> {
358+
try {
359+
const cancellationTokenSource = new CancellationTokenSource();
360+
const queryResult = await runExternalApiQueries(this.mode, {
361+
cliServer: this.cliServer,
362+
queryRunner: this.queryRunner,
363+
databaseItem: this.databaseItem,
364+
queryStorageDir: this.queryStorageDir,
365+
queryDir: this.queryDir,
366+
progress: (update) =>
367+
progress({
368+
...update,
369+
message: `Loading models: ${update.message}`,
370+
}),
371+
token: cancellationTokenSource.token,
372+
});
373+
if (!queryResult) {
374+
return;
375+
}
376+
this.methods = queryResult;
338377

339-
await this.postMessage({
340-
t: "setMethods",
341-
methods: this.methods,
342-
});
343-
if (this.isMostRecentlyActiveView(this)) {
344-
await this.updateMethodsUsagePanelState(
345-
this.methods,
346-
this.databaseItem,
347-
this.hideModeledApis,
348-
);
349-
}
350-
} catch (err) {
351-
void showAndLogExceptionWithTelemetry(
352-
this.app.logger,
353-
this.app.telemetry,
354-
redactableError(
355-
asError(err),
356-
)`Failed to load external API usages: ${getErrorMessage(err)}`,
357-
);
358-
}
359-
},
360-
{ cancellable: false },
361-
);
378+
await this.postMessage({
379+
t: "setMethods",
380+
methods: this.methods,
381+
});
382+
if (this.isMostRecentlyActiveView(this)) {
383+
await this.updateMethodsUsagePanelState(
384+
this.methods,
385+
this.databaseItem,
386+
this.hideModeledApis,
387+
);
388+
}
389+
} catch (err) {
390+
void showAndLogExceptionWithTelemetry(
391+
this.app.logger,
392+
this.app.telemetry,
393+
redactableError(
394+
asError(err),
395+
)`Failed to load external API usages: ${getErrorMessage(err)}`,
396+
);
397+
}
362398
}
363399

364400
protected async generateModeledMethods(): Promise<void> {

0 commit comments

Comments
 (0)