-
Notifications
You must be signed in to change notification settings - Fork 226
Print end-of-query summary logs to Query Server Console #1264
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
Changes from 5 commits
7e603d2
e996660
24ef6e9
cdc18dc
14f680f
886e75d
e986acf
8faa7c0
a691d6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -259,6 +259,14 @@ export function findQueryLogFile(resultPath: string): string { | |||||
| return path.join(resultPath, 'query.log'); | ||||||
| } | ||||||
|
|
||||||
| export function findQueryStructLogFile(resultPath: string): string { | ||||||
| export function findQueryEvalLogFile(resultPath: string): string { | ||||||
| return path.join(resultPath, 'evaluator-log.jsonl'); | ||||||
| } | ||||||
|
|
||||||
| export function findQueryEvalLogSummaryFile(resultPath: string): string { | ||||||
| return path.join(resultPath, 'evaluator-log.jsonl.summary'); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a valid json or jsonl file? If so, it should probably use the proper file extension. Maybe:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not — it's a generated text file summarizing the contents of the jsonl file.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK...then maybe avoid the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems agreeable to me, I'll hold to see if @edoardopirovano who named the summary file originally has objections 😸
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just calling it |
||||||
| } | ||||||
|
|
||||||
| export function findQueryEvalLogEndSummaryFile(resultPath: string): string { | ||||||
| return path.join(resultPath, 'eoq-evaluator-log.jsonl.summary'); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, it's not a JSONL file but rather a condensed version of the summary text file. I didn't know how to name it so appended
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this one now that the |
||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -95,8 +95,16 @@ export class QueryEvaluationInfo { | |
| return qsClient.findQueryLogFile(this.querySaveDir); | ||
| } | ||
|
|
||
| get structLogPath() { | ||
| return qsClient.findQueryStructLogFile(this.querySaveDir); | ||
| get evalLogPath() { | ||
| return qsClient.findQueryEvalLogFile(this.querySaveDir); | ||
| } | ||
|
|
||
| get evalLogSummaryPath() { | ||
| return qsClient.findQueryEvalLogSummaryFile(this.querySaveDir); | ||
| } | ||
|
|
||
| get evalLogEndSummaryPath() { | ||
| return qsClient.findQueryEvalLogEndSummaryFile(this.querySaveDir); | ||
| } | ||
|
|
||
| get resultsPaths() { | ||
|
|
@@ -164,8 +172,9 @@ export class QueryEvaluationInfo { | |
| if (queryInfo && await qs.cliServer.cliConstraints.supportsPerQueryEvalLog()) { | ||
| await qs.sendRequest(messages.startLog, { | ||
| db: dataset, | ||
| logPath: this.structLogPath, | ||
| logPath: this.evalLogPath, | ||
| }); | ||
|
|
||
| } | ||
| const params: messages.EvaluateQueriesParams = { | ||
| db: dataset, | ||
|
|
@@ -186,9 +195,21 @@ export class QueryEvaluationInfo { | |
| if (queryInfo && await qs.cliServer.cliConstraints.supportsPerQueryEvalLog()) { | ||
| await qs.sendRequest(messages.endLog, { | ||
| db: dataset, | ||
| logPath: this.structLogPath, | ||
| logPath: this.evalLogPath, | ||
| }); | ||
| queryInfo.evalLogLocation = this.structLogPath; | ||
| if (await this.hasEvalLog()) { | ||
| queryInfo.evalLogLocation = this.evalLogPath; | ||
| await qs.cliServer.generateLogSummary(this.evalLogPath, this.evalLogSummaryPath, this.evalLogEndSummaryPath); | ||
| fs.readFile(this.evalLogEndSummaryPath, (err, buffer) => { | ||
| if (err) { | ||
| throw new Error(`Could not read structured evaluator log end of summary file at ${this.evalLogEndSummaryPath}.`); | ||
| } | ||
| void qs.logger.log(' --- Evaluator Log Summary --- ', { additionalLogLocation: this.logPath }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will add the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The additionalLogLocation will add the contents to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, the user is meant to read this line (and the following lines) only from the Query Server Console. I can remove the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are much more familiar with this feature than I am. I am fine with it if you decide to leave this in.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think dropping the |
||
| void qs.logger.log(buffer.toString(), { additionalLogLocation: this.logPath }); | ||
| }); | ||
| } else { | ||
| void showAndLogWarningMessage(`Failed to write structured evaluator log to ${this.evalLogPath}.`); | ||
| } | ||
| } | ||
| } | ||
| return result || { | ||
|
|
@@ -303,6 +324,13 @@ export class QueryEvaluationInfo { | |
| return this.dilPath; | ||
| } | ||
|
|
||
| /** | ||
| * Holds if this query already has a completed structured evaluator log | ||
| */ | ||
| async hasEvalLog(): Promise<boolean> { | ||
| return fs.pathExists(this.evalLogPath); | ||
| } | ||
|
|
||
| /** | ||
| * Creates the CSV file containing the results of this query. This will only be called if the query | ||
| * does not have interpreted results and the CSV file does not already exist. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.