Gate show eval log and summary commands behind CLI v2.8.4 #1243
Conversation
| await commands.executeCommand( | ||
| 'setContext', 'codeql.supportsEvalLog', await this.cliConstraints.supportsPerQueryEvalLog() | ||
| ); |
There was a problem hiding this comment.
I like the simplicity of this change. Is this the best place to execute this command, though? It will be invoked every time the extension asks for the CodeQL CLI version, which may be frequent. I expect this command to be executed each time there is a potential change to the configured CLI -- see what the CliConfigListener and QueryServerConfigListener classes in config.ts do, and whether that can be adapted to suit your purpose.
There was a problem hiding this comment.
This is actually what is happening. The only time this._version is undefined is after a config change, where the version may have potentially changed. The version and context are cached until the next time the config changes. By putting it in this if statement, we are guaranteed to call the command the minimal number of times.
There was a problem hiding this comment.
Fancy. Looks good then! Perhaps add a comment to explain that, so I don't pester you about it again :)
There was a problem hiding this comment.
Will do, thanks!!
extensions/ql-vscode/src/cli.ts
Outdated
| @@ -1276,7 +1279,7 @@ export class CliVersionConstraint { | |||
| /** | |||
| * CLI version that supports rotating structured logs to produce one per query. | |||
| */ | |||
| public static CLI_VERSION_WITH_PER_QUERY_EVAL_LOG = new SemVer('2.8.4'); | |||
| public static CLI_VERSION_WITH_PER_QUERY_EVAL_LOG = new SemVer('2.9.0'); | |||
There was a problem hiding this comment.
Why the change of version here? I believe everything we need for this is in 2.8.4.
There was a problem hiding this comment.
Ah, I got a bit mixed up with all the different PRs — the incoming versions to log to the query server output (that I had been planning to merge into your PR) will be in 2.9.0, but it does seem unnecessary to wait until 2.9.0 for these changes.
So after I change it back, if a user is using 2.8.4, they would see the commands (and log files) but not any output in the query server console; and if they are using 2.9.0+ they would see the query server console output as well. I think that is reasonable (without adding another gate for 2.9.0).
angelapwen
changed the title
This PR will make it so that the "Show Evaluation Log (Raw)" and "Show Evaluation Log (Summary)" commands in the query history view only if the CLI version is >2.9.0. Previously, the commands always showed regardless of CLI version, and the user received an error pop-up if their CLI checkout was an earlier version. Once this is merged to the appropriate base branch, this change will allow us to merge #1186 before CLI v2.8.4 is released.
Checklist
ready-for-doc-reviewlabel there.