Add Commands for exp queue, exp run --run-all, and exp gc
#251
Conversation
No tests yet, but there is info message functionality which seems to work well.
extension/src/cli/index.ts
Outdated
| }) | ||
| window.showInformationMessage(stdout) | ||
| } catch (e) { | ||
| window.showErrorMessage(`Failed to queue an Experiment!\n${e.message}`) |
There was a problem hiding this comment.
This doesn't actually format with a newline, and will need to be changed.
There was a problem hiding this comment.
The try/catch currently catches all errors, including ones like a locked repo that the others don't catch. We may want to add a common try/catch that handles errors like these for all commands, with the possible exception of the Webview commands which may want to report errors in a webview.
rogermparent
changed the title
…to queue-exp-command
| @@ -78,3 +78,10 @@ export const status = async ( | |||
| const { stdout } = await execCommand(options, Commands.STATUS) | |||
| return JSON.parse(stdout) | |||
| } | |||
|
|
|||
| export const queueExperiment = async ( | |||
There was a problem hiding this comment.
[C] Feels like all of the functions of the form:
const { stdout } = await execCommand(options, Commands.COMMAND)
return stdout
could get collapsed to a single function that we pass options and a command now.
There was a problem hiding this comment.
This is solved in #271
|
|
||
| disposer.track( | ||
| commands.registerCommand('dvc.queueExperiment', async () => { | ||
| return queueExperimentCommand(config) |
There was a problem hiding this comment.
[Q] how does config get into this function?
There was a problem hiding this comment.
I can see it comes through registerCommands. Could it be stale after a change?
There was a problem hiding this comment.
I don't think it can be stale here, as we only ever have one Config object instantiated and are just passing around its reference.
To make this reference stale, we would have to somehow dispose of the current Config and create a new one mid-run without reactivating the extension, which is a case I could only see happening through Hot Reload, and even that I'm not sure of.
Outside of that situation, config is always a reference to a single Config object stored in the extension and only disposed of at the end of the Extension's lifecycle, so calls made to it through this reference will be just as up-to-date as anywhere else.
* Add "Run Queued Experiments" command * Add test for "runQueuedExperiments" * Experiment Commands #3: Add command for `exp gc` with selectable flags (#269) * Add command for GC * Change gc to exp gc, and add leading `--` to enum flags * Add GC command tests and export GC QuickPickItem interface * Rename exp gc enum entry and reader command * Change gcExperiments to experimentGarbageCollect * Replace test() with it() for consistency * Experiment Commands #4: Addressing comments from 1-3 (#271) * Change execCommand to resolve to stdout and simplify its consumers * runDvcCommand => runCommand * Reorganize command enum to distinguish exp commands with a prefix * DvcGc => Gc
rogermparent
changed the title
exp queue, exp run --run-all, and exp gc
Builds on #266 to adopt the new allcaps enum convention.
Related to #229, which is part of #239.
#268 builds on this.
This PR adds a "Queue Experiments"/
dvc exp run --queuecommand that reports its output viashowInformationMessage, andshowErrorMessage.