Add show logs to context menu of experiments running in the queue #3347
Conversation
|
you got me |
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
extension/src/cli/dvc/viewer.ts
Outdated
| return this.config.getCliPath() | ||
| } | ||
|
|
||
| private createProcess({ cwd, args }: { cwd: string; args: Args }): Process { |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
extension/src/cli/dvc/viewer.ts
Outdated
| private currentProcess: Process | undefined | ||
| private readonly config: Config | ||
|
|
||
| constructor( |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
extension/src/cli/dvc/viewer.ts
Outdated
| return this.run(cwd, Command.QUEUE, QueueSubCommand.LOGS, expName, '-f') | ||
| } | ||
|
|
||
| public stop() { |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
extension/src/cli/dvc/viewer.ts
Outdated
| this.sendTelemetryEvent({ command, duration, exitCode, killed, stderr }) | ||
| } | ||
|
|
||
| private sendTelemetryEvent({ |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
extension/src/cli/dvc/viewer.ts
Outdated
| ) | ||
| } | ||
|
|
||
| private notifyCompleted({ |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
|
@mattseddon Will it work for failed/successful experiments that were run by the queue also? |
|
Looks great! Q: is there a way to control the Terminal name? To include the experiment name in it:
So that I can see the list of Tasks (experiments) / terminals like in this case ^^, switch between them, etc? |
|
|
||
| private isActive = false | ||
|
|
||
| constructor( |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
| } | ||
|
|
||
| protected createInstance() { | ||
| return new Promise<void>(resolve => { |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
| ) | ||
| } | ||
|
|
||
| private notifyActiveStatus() { |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
| }) | ||
| } | ||
|
|
||
| private deleteReferenceOnClose() { |
There was a problem hiding this comment.
Identical blocks of code found in 2 locations. Consider refactoring.
| import { StopWatch } from '../../util/time' | ||
|
|
||
| export class SingleUsePseudoTerminal extends BasePseudoTerminal { | ||
| constructor( |
There was a problem hiding this comment.
Function constructor has 42 lines of code (exceeds 30 allowed). Consider refactoring.
|
Code Climate has analyzed commit 63b13de and detected 13 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 66.1% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.4% (-0.5% change). View more on Code Climate. |
It can. Not sure if all of the required data is actually in
Yep, was getting there. |
|
Need to refactor before I can get this in. |
Part of #3178
WIP.
Lots of duplication in the PR that
canwill be removed. Need to think more about how to set this out so that it can be used under certain circumstances for things likedvc pull. Thinking maybeDemo
Screen.Recording.2023-02-24.at.3.48.00.pm.mov