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

Refactor: Store EvaluatorLogPaths object on LocalQueryInfo #3803

Merged
merged 4 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
20 changes: 10 additions & 10 deletions extensions/ql-vscode/src/log-insights/log-scanner-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,19 @@ export class LogScannerService extends DisposableObject {
public async scanEvalLog(query: QueryHistoryInfo | undefined): Promise<void> {
this.diagnosticCollection.clear();

if (
query?.t !== "local" ||
query.evalLogSummaryLocation === undefined ||
query.jsonEvalLogSummaryLocation === undefined
) {
if (query?.t !== "local" || query.evalutorLogPaths === undefined) {
return;
}

const diagnostics = await this.scanLog(
query.jsonEvalLogSummaryLocation,
query.evalLogSummarySymbolsLocation,
);
const uri = Uri.file(query.evalLogSummaryLocation);
const { summarySymbols, jsonSummary, humanReadableSummary } =
query.evalutorLogPaths;

if (jsonSummary === undefined || humanReadableSummary === undefined) {
return;
}

const diagnostics = await this.scanLog(jsonSummary, summarySymbols);
const uri = Uri.file(humanReadableSummary);
this.diagnosticCollection.set(uri, diagnostics);
}

Expand Down
17 changes: 10 additions & 7 deletions extensions/ql-vscode/src/query-history/query-history-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ export class QueryHistoryManager extends DisposableObject {

private async warnNoEvalLogSummary(item: LocalQueryInfo) {
const evalLogLocation =
item.evalLogLocation ?? item.initialInfo.outputDir?.evalLogPath;
item.evalutorLogPaths?.log ?? item.initialInfo.outputDir?.evalLogPath;

// Summary log file doesn't exist.
if (evalLogLocation && (await pathExists(evalLogLocation))) {
Expand All @@ -801,7 +801,7 @@ export class QueryHistoryManager extends DisposableObject {
}

const evalLogLocation =
item.evalLogLocation ?? item.initialInfo.outputDir?.evalLogPath;
item.evalutorLogPaths?.log ?? item.initialInfo.outputDir?.evalLogPath;

if (evalLogLocation && (await pathExists(evalLogLocation))) {
await tryOpenExternalFile(this.app.commands, evalLogLocation);
Expand All @@ -816,12 +816,15 @@ export class QueryHistoryManager extends DisposableObject {
}

// If the summary file location wasn't saved, display error
if (!item.evalLogSummaryLocation) {
if (!item.evalutorLogPaths?.humanReadableSummary) {
await this.warnNoEvalLogSummary(item);
return;
}

await tryOpenExternalFile(this.app.commands, item.evalLogSummaryLocation);
await tryOpenExternalFile(
this.app.commands,
item.evalutorLogPaths.humanReadableSummary,
);
}

async handleShowEvalLogViewer(item: QueryHistoryInfo) {
Expand All @@ -830,15 +833,15 @@ export class QueryHistoryManager extends DisposableObject {
}

// If the JSON summary file location wasn't saved, display error
if (item.jsonEvalLogSummaryLocation === undefined) {
if (item.evalutorLogPaths?.jsonSummary === undefined) {
await this.warnNoEvalLogSummary(item);
return;
}

// TODO(angelapwen): Stream the file in.
try {
const evalLogData: EvalLogData[] = await parseViewerData(
item.jsonEvalLogSummaryLocation,
item.evalutorLogPaths.jsonSummary,
);
const evalLogTreeBuilder = new EvalLogTreeBuilder(
item.getQueryName(),
Expand All @@ -847,7 +850,7 @@ export class QueryHistoryManager extends DisposableObject {
this.evalLogViewer.updateRoots(await evalLogTreeBuilder.getRoots());
} catch {
throw new Error(
`Could not read evaluator log summary JSON file to generate viewer data at ${item.jsonEvalLogSummaryLocation}.`,
`Could not read evaluator log summary JSON file to generate viewer data at ${item.evalutorLogPaths.jsonSummary}.`,
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ export function mapLocalQueryInfoToDto(
return {
initialInfo: mapInitialQueryInfoToDto(query.initialInfo),
t: "local",
evalLogLocation: query.evalLogLocation,
evalLogSummaryLocation: query.evalLogSummaryLocation,
jsonEvalLogSummaryLocation: query.jsonEvalLogSummaryLocation,
evalLogSummarySymbolsLocation: query.evalLogSummarySymbolsLocation,
evalLogLocation: query.evalutorLogPaths?.log,
evalLogSummaryLocation: query.evalutorLogPaths?.humanReadableSummary,
jsonEvalLogSummaryLocation: query.evalutorLogPaths?.jsonSummary,
evalLogSummarySymbolsLocation: query.evalutorLogPaths?.summarySymbols,
failureReason: query.failureReason,
completedQuery:
query.completedQuery && mapCompletedQueryToDto(query.completedQuery),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ export function mapLocalQueryItemToDomainModel(
localQuery.failureReason,
localQuery.completedQuery &&
mapCompletedQueryInfoToDomainModel(localQuery.completedQuery),
localQuery.evalLogLocation,
localQuery.evalLogSummaryLocation,
localQuery.jsonEvalLogSummaryLocation,
localQuery.evalLogSummarySymbolsLocation,
localQuery.evalLogLocation
? {
log: localQuery.evalLogLocation,
humanReadableSummary: localQuery.evalLogSummaryLocation,
jsonSummary: localQuery.jsonEvalLogSummaryLocation,
summarySymbols: localQuery.evalLogSummarySymbolsLocation,
endSummary: undefined,
}
: undefined,
);
}

Expand Down
10 changes: 2 additions & 8 deletions extensions/ql-vscode/src/query-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,7 @@ export class LocalQueryInfo {
private cancellationSource?: CancellationTokenSource, // used to cancel in progress queries
public failureReason?: string,
public completedQuery?: CompletedQueryInfo,
public evalLogLocation?: string,
public evalLogSummaryLocation?: string,
public jsonEvalLogSummaryLocation?: string,
public evalLogSummarySymbolsLocation?: string,
public evalutorLogPaths?: EvaluatorLogPaths,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public evalutorLogPaths?: EvaluatorLogPaths,
public evaluatorLogPaths?: EvaluatorLogPaths,

) {
/**/
}
Expand All @@ -229,10 +226,7 @@ export class LocalQueryInfo {

/** Sets the paths to the various structured evaluator logs. */
public setEvaluatorLogPaths(logPaths: EvaluatorLogPaths): void {
this.evalLogLocation = logPaths.log;
this.evalLogSummaryLocation = logPaths.humanReadableSummary;
this.jsonEvalLogSummaryLocation = logPaths.jsonSummary;
this.evalLogSummarySymbolsLocation = logPaths.summarySymbols;
this.evalutorLogPaths = logPaths;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.evalutorLogPaths = logPaths;
this.evaluatorLogPaths = logPaths;

}

/**
Expand Down
Loading