From 1c9fe13299512136d489f29fcb88ed352c9fd055 Mon Sep 17 00:00:00 2001 From: Matt Seddon <37993418+mattseddon@users.noreply.github.com> Date: Tue, 25 Jul 2023 08:21:43 +1000 Subject: [PATCH] Show Experiment Remote Status (#4324) * prototype solution * add remote exp refs test fixture * review and refactor * address review comments --- extension/src/cli/git/reader.ts | 15 ++++- extension/src/data/index.ts | 1 + extension/src/experiments/commands/index.ts | 41 ++++++++++---- .../src/experiments/commands/register.ts | 2 +- extension/src/experiments/data/index.ts | 31 +++++++++-- extension/src/experiments/index.ts | 16 +++++- .../src/experiments/model/collect.test.ts | 4 +- extension/src/experiments/model/collect.ts | 36 +++++++++++- extension/src/experiments/model/index.test.ts | 42 +++++++++----- extension/src/experiments/model/index.ts | 36 ++++++++++-- extension/src/experiments/webview/contract.ts | 7 +++ extension/src/experiments/workspace.ts | 6 +- .../fixtures/expShow/base/remoteExpRefs.ts | 3 + .../src/test/fixtures/expShow/base/rows.ts | 7 ++- .../src/test/suite/experiments/index.test.ts | 5 +- .../test/suite/experiments/model/tree.test.ts | 18 ++++++ extension/src/test/suite/experiments/util.ts | 6 ++ extension/src/test/suite/plots/index.test.ts | 1 + extension/src/test/suite/plots/util.ts | 3 + webview/icons/codicons.mjs | 2 + .../components/table/body/Cell.tsx | 17 +++--- .../table/body/ExperimentStatusIndicator.tsx | 55 +++++++++++++++++++ .../experiments/components/table/body/Row.tsx | 8 +-- .../components/table/styles.module.scss | 31 ++++++++--- webview/src/experiments/util/messages.ts | 3 + webview/src/shared/components/icons/Cloud.tsx | 19 +++++++ .../shared/components/icons/CloudUpload.tsx | 19 +++++++ webview/src/shared/components/icons/index.ts | 2 + 28 files changed, 368 insertions(+), 68 deletions(-) create mode 100644 extension/src/test/fixtures/expShow/base/remoteExpRefs.ts create mode 100644 webview/src/experiments/components/table/body/ExperimentStatusIndicator.tsx create mode 100644 webview/src/shared/components/icons/Cloud.tsx create mode 100644 webview/src/shared/components/icons/CloudUpload.tsx diff --git a/extension/src/cli/git/reader.ts b/extension/src/cli/git/reader.ts index affd79d067..e0f9b1498d 100644 --- a/extension/src/cli/git/reader.ts +++ b/extension/src/cli/git/reader.ts @@ -1,6 +1,6 @@ import { resolve } from 'path' import { GitCli } from '.' -import { Command, Flag } from './constants' +import { Command, DEFAULT_REMOTE, Flag } from './constants' import { getOptions } from './options' import { typeCheckCommands } from '..' import { trimAndSplit } from '../../util/stdout' @@ -10,6 +10,7 @@ export const autoRegisteredCommands = { GIT_GET_BRANCHES: 'getBranches', GIT_GET_COMMIT_MESSAGES: 'getCommitMessages', GIT_GET_NUM_COMMITS: 'getNumCommits', + GIT_GET_REMOTE_EXPERIMENT_REFS: 'getRemoteExperimentRefs', GIT_GET_REMOTE_URL: 'getRemoteUrl', GIT_GET_REPOSITORY_ROOT: 'getGitRepositoryRoot', GIT_HAS_CHANGES: 'hasChanges', @@ -68,6 +69,18 @@ export class GitReader extends GitCli { } } + public async getRemoteExperimentRefs(cwd: string): Promise { + const options = getOptions({ + args: [Command.LS_REMOTE, DEFAULT_REMOTE, 'refs/exps/*'], + cwd + }) + try { + return await this.executeProcess(options) + } catch { + return '' + } + } + public async getRemoteUrl(cwd: string): Promise { const options = getOptions({ args: [Command.LS_REMOTE, Flag.GET_URL], cwd }) try { diff --git a/extension/src/data/index.ts b/extension/src/data/index.ts index ac6fd3a57c..9a160d6811 100644 --- a/extension/src/data/index.ts +++ b/extension/src/data/index.ts @@ -13,6 +13,7 @@ export type ExperimentsOutput = { availableNbCommits: { [branch: string]: number } expShow: ExpShowOutput gitLog: string + remoteExpRefs: string rowOrder: { branch: string; sha: string }[] } diff --git a/extension/src/experiments/commands/index.ts b/extension/src/experiments/commands/index.ts index 2d43ae759a..83bc94b23e 100644 --- a/extension/src/experiments/commands/index.ts +++ b/extension/src/experiments/commands/index.ts @@ -43,7 +43,11 @@ const convertUrlTextToLink = (stdout: string) => { } export const getPushExperimentCommand = - (internalCommands: InternalCommands, setup: Setup) => + ( + experiments: WorkspaceExperiments, + internalCommands: InternalCommands, + setup: Setup + ) => ({ dvcRoot, ids }: { dvcRoot: string; ids: string[] }) => { const studioAccessToken = setup.getStudioAccessToken() if ( @@ -56,23 +60,38 @@ export const getPushExperimentCommand = } return Toast.showProgress('exp push', async progress => { + const repository = experiments.getRepository(dvcRoot) + + const updateOnCompletion = () => { + return repository.unsetPushing(ids) + } + progress.report({ increment: 0 }) progress.report({ increment: 25, message: `Pushing ${ids.join(' ')}...` }) const remainingProgress = 75 - const stdout = await internalCommands.executeCommand( - AvailableCommands.EXP_PUSH, - dvcRoot, - ...ids - ) + repository.setPushing(ids) + + try { + const stdout = await internalCommands.executeCommand( + AvailableCommands.EXP_PUSH, + dvcRoot, + ...ids + ) + + void updateOnCompletion() - progress.report({ - increment: remainingProgress, - message: convertUrlTextToLink(stdout) - }) + progress.report({ + increment: remainingProgress, + message: convertUrlTextToLink(stdout) + }) - return Toast.delayProgressClosing(15000) + return Toast.delayProgressClosing(15000) + } catch (error: unknown) { + void updateOnCompletion() + throw error + } }) } diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index e59adb2e17..10c8a384f1 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -277,7 +277,7 @@ export const registerExperimentCommands = ( internalCommands.registerExternalCliCommand( RegisteredCliCommands.EXPERIMENT_VIEW_PUSH, - getPushExperimentCommand(internalCommands, setup) + getPushExperimentCommand(experiments, internalCommands, setup) ) internalCommands.registerExternalCliCommand( diff --git a/extension/src/experiments/data/index.ts b/extension/src/experiments/data/index.ts index ac17fd205d..389c6dc912 100644 --- a/extension/src/experiments/data/index.ts +++ b/extension/src/experiments/data/index.ts @@ -42,6 +42,19 @@ export class ExperimentsData extends BaseData { } public async update(): Promise { + const [{ availableNbCommits, expShow, gitLog, rowOrder }, remoteExpRefs] = + await Promise.all([this.updateExpShow(), this.updateRemoteExpRefs()]) + + return this.notifyChanged({ + availableNbCommits, + expShow, + gitLog, + remoteExpRefs, + rowOrder + }) + } + + private async updateExpShow() { await this.updateBranches() const branches = this.experiments.getBranchesToShow() let gitLog = '' @@ -66,12 +79,7 @@ export class ExperimentsData extends BaseData { ) this.collectFiles({ expShow }) - - return this.notifyChanged({ availableNbCommits, expShow, gitLog, rowOrder }) - } - - protected collectFiles({ expShow }: { expShow: ExpShowOutput }) { - this.collectedFiles = collectFiles(expShow, this.collectedFiles) + return { availableNbCommits, expShow, gitLog, rowOrder } } private async collectGitLogAndOrder( @@ -119,6 +127,10 @@ export class ExperimentsData extends BaseData { this.experiments.setBranches(allBranches) } + private collectFiles({ expShow }: { expShow: ExpShowOutput }) { + this.collectedFiles = collectFiles(expShow, this.collectedFiles) + } + private async watchExpGitRefs(): Promise { const gitRoot = await this.internalCommands.executeCommand( AvailableCommands.GIT_GET_REPOSITORY_ROOT, @@ -149,4 +161,11 @@ export class ExperimentsData extends BaseData { } ) } + + private updateRemoteExpRefs() { + return this.internalCommands.executeCommand( + AvailableCommands.GIT_GET_REMOTE_EXPERIMENT_REFS, + this.dvcRoot + ) + } } diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 07b0ac7d82..f8cf738eb6 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -177,7 +177,8 @@ export class Experiments extends BaseRepository { availableNbCommits, expShow, gitLog, - rowOrder + rowOrder, + remoteExpRefs }: ExperimentsOutput) { const hadCheckpoints = this.hasCheckpoints() const dvcLiveOnly = await this.checkSignalFile() @@ -188,7 +189,8 @@ export class Experiments extends BaseRepository { gitLog, dvcLiveOnly, rowOrder, - availableNbCommits + availableNbCommits, + remoteExpRefs ) ]) @@ -199,6 +201,16 @@ export class Experiments extends BaseRepository { return this.notifyChanged() } + public setPushing(ids: string[]) { + this.experiments.setPushing(ids) + return this.notifyChanged() + } + + public unsetPushing(ids: string[]) { + this.experiments.unsetPushing(ids) + return this.update() + } + public hasCheckpoints() { return this.experiments.hasCheckpoints() } diff --git a/extension/src/experiments/model/collect.test.ts b/extension/src/experiments/model/collect.test.ts index 1e4f36d234..6c2b67f1a3 100644 --- a/extension/src/experiments/model/collect.test.ts +++ b/extension/src/experiments/model/collect.test.ts @@ -2,7 +2,7 @@ import { collectExperiments } from './collect' import { generateTestExpShowOutput } from '../../test/util/experiments' import { ExpShowOutput } from '../../cli/dvc/contract' -const DEFAULT_DATA: [string, boolean] = ['', false] +const DEFAULT_DATA: [string, boolean, string] = ['', false, ''] describe('collectExperiments', () => { it('should return an empty array if no commits are present', () => { @@ -13,7 +13,7 @@ describe('collectExperiments', () => { expect(commits).toStrictEqual([]) }) - const expShowWithTwoCommits: [ExpShowOutput, string, boolean] = [ + const expShowWithTwoCommits: [ExpShowOutput, string, boolean, string] = [ generateTestExpShowOutput( {}, { diff --git a/extension/src/experiments/model/collect.ts b/extension/src/experiments/model/collect.ts index eba3873292..c6580d4152 100644 --- a/extension/src/experiments/model/collect.ts +++ b/extension/src/experiments/model/collect.ts @@ -9,6 +9,7 @@ import { extractColumns } from '../columns/extract' import { CommitData, Experiment, + GitRemoteStatus, RunningExperiment, isQueued, isRunning @@ -29,6 +30,7 @@ import { RegisteredCommands } from '../../commands/external' import { Resource } from '../../resourceLocator' import { shortenForLabel } from '../../util/string' import { COMMITS_SEPARATOR } from '../../cli/git/constants' +import { trimAndSplit } from '../../util/stdout' export type ExperimentItem = { command?: { @@ -233,6 +235,21 @@ const collectExecutorInfo = ( } } +const collectRemoteStatus = ( + experiment: Experiment, + remoteExpShas: Set +): void => { + if ( + !experiment.sha || + ![undefined, ExperimentStatus.SUCCESS].includes(experiment.status) + ) { + return + } + experiment.gitRemoteStatus = remoteExpShas.has(experiment.sha) + ? GitRemoteStatus.ON_REMOTE + : GitRemoteStatus.NOT_ON_REMOTE +} + const collectRunningExperiment = ( acc: ExperimentsAccumulator, experiment: Experiment @@ -249,7 +266,8 @@ const collectRunningExperiment = ( const collectExpRange = ( acc: ExperimentsAccumulator, baseline: Experiment, - expRange: ExpRange + expRange: ExpRange, + remoteExpShas: Set ): void => { const { revs, executor } = expRange if (!revs?.length) { @@ -289,6 +307,7 @@ const collectExpRange = ( } collectExecutorInfo(experiment, executor) + collectRemoteStatus(experiment, remoteExpShas) collectRunningExperiment(acc, experiment) addToMapArray(acc.experimentsByCommit, baselineId, experiment) @@ -340,10 +359,20 @@ const collectCliError = ( } } +const collectRemoteExpShas = (remoteExpRefs: string): Set => { + const remoteExpShas = new Set() + for (const ref of trimAndSplit(remoteExpRefs)) { + const [sha] = ref.split(/\s/) + remoteExpShas.add(sha) + } + return remoteExpShas +} + export const collectExperiments = ( expShow: ExpShowOutput, gitLog: string, - dvcLiveOnly: boolean + dvcLiveOnly: boolean, + remoteExpRefs: string ): ExperimentsAccumulator => { const acc: ExperimentsAccumulator = { cliError: undefined, @@ -358,6 +387,7 @@ export const collectExperiments = ( } const commitData = collectCommitsData(gitLog) + const remoteExpShas = collectRemoteExpShas(remoteExpRefs) for (const expState of expShow) { const baseline = collectExpState(acc, expState, commitData) @@ -368,7 +398,7 @@ export const collectExperiments = ( } for (const expRange of experiments) { - collectExpRange(acc, baseline, expRange) + collectExpRange(acc, baseline, expRange, remoteExpShas) } } diff --git a/extension/src/experiments/model/index.test.ts b/extension/src/experiments/model/index.test.ts index 7e491a6314..ad72da670a 100644 --- a/extension/src/experiments/model/index.test.ts +++ b/extension/src/experiments/model/index.test.ts @@ -7,6 +7,7 @@ import gitLogFixture from '../../test/fixtures/expShow/base/gitLog' import rowOrderFixture from '../../test/fixtures/expShow/base/rowOrder' import outputFixture from '../../test/fixtures/expShow/base/output' import rowsFixture from '../../test/fixtures/expShow/base/rows' +import remoteExpRefsFixture from '../../test/fixtures/expShow/base/remoteExpRefs' import deeplyNestedRowsFixture from '../../test/fixtures/expShow/deeplyNested/rows' import deeplyNestedOutputFixture from '../../test/fixtures/expShow/deeplyNested/output' import uncommittedDepsFixture from '../../test/fixtures/expShow/uncommittedDeps/output' @@ -40,8 +41,9 @@ const DEFAULT_DATA: [ string, boolean, { branch: string; sha: string }[], - { [branch: string]: number } -] = ['', false, [], { main: 2000 }] + { [branch: string]: number }, + string +] = ['', false, [], { main: 2000 }, ''] type TransformAndSetInputs = [ExpShowOutput, ...typeof DEFAULT_DATA] @@ -53,7 +55,8 @@ describe('ExperimentsModel', () => { gitLogFixture, false, rowOrderFixture, - { main: 6 } + { main: 6 }, + remoteExpRefsFixture ) expect(model.getRowData()).toStrictEqual(rowsFixture) }) @@ -69,7 +72,8 @@ describe('ExperimentsModel', () => { { branch: 'main', sha: 'a49e03966a1f9f1299ec222ebc4bed8625d2c54d' }, { branch: 'main', sha: '4f7b50c3d171a11b6cfcd04416a16fc80b61018d' } ], - { main: 700 } + { main: 700 }, + '' ) expect(model.getRowData()).toStrictEqual( expect.objectContaining(survivalRowsFixture) @@ -105,7 +109,7 @@ describe('ExperimentsModel', () => { } ) - model.transformAndSet(dvcLiveOnly, '', true, [], { main: 2000 }) + model.transformAndSet(dvcLiveOnly, '', true, [], { main: 2000 }, '') // eslint-disable-next-line @typescript-eslint/no-explicit-any const runningWorkspace = (model as any).workspace expect(runningWorkspace?.executor).toStrictEqual(EXPERIMENT_WORKSPACE_ID) @@ -201,7 +205,8 @@ describe('ExperimentsModel', () => { '', false, [{ branch: 'main', sha: '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77' }], - { main: 10 } + { main: 10 }, + '' ) expect(model.getRowData()).toStrictEqual( expect.objectContaining(deeplyNestedRowsFixture) @@ -215,7 +220,8 @@ describe('ExperimentsModel', () => { '', false, [{ branch: 'main', sha: '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77' }], - { main: 10 } + { main: 10 }, + '' ) expect(model.getRowData()).toStrictEqual( expect.objectContaining(dataTypesRowsFixture) @@ -449,7 +455,8 @@ describe('ExperimentsModel', () => { [], { main: 2000 - } + }, + remoteExpRefsFixture ] const transientErrorData: TransformAndSetInputs = [ @@ -470,7 +477,8 @@ describe('ExperimentsModel', () => { [], { main: 2000 - } + }, + remoteExpRefsFixture ] model.transformAndSet(...runningExperimentData) @@ -548,9 +556,16 @@ describe('ExperimentsModel', () => { error: { msg: errorMsg, type: 'caught error' } } ] - model.transformAndSet(data, gitLogFixture, false, [], { - main: 6 - }) + model.transformAndSet( + data, + gitLogFixture, + false, + [], + { + main: 6 + }, + remoteExpRefsFixture + ) expect(model.getCliError()).toStrictEqual(errorMsg) @@ -559,7 +574,8 @@ describe('ExperimentsModel', () => { gitLogFixture, false, rowOrderFixture, - { main: 6 } + { main: 6 }, + remoteExpRefsFixture ) expect(model.getCliError()).toBe(undefined) diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index 700e390f0e..3553500345 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -23,6 +23,7 @@ import { Experiment, isQueued, isRunning, + GitRemoteStatus, RunningExperiment } from '../webview/contract' import { definedAndNonEmpty, reorderListSubset } from '../../util/array' @@ -74,6 +75,8 @@ export class ExperimentsModel extends ModelWithPersistence { private filters: Map = new Map() + private pushing = new Set() + private currentSorts: SortDefinition[] private running: RunningExperiment[] = [] private startedRunning: Set = new Set() @@ -125,7 +128,8 @@ export class ExperimentsModel extends ModelWithPersistence { gitLog: string, dvcLiveOnly: boolean, rowOrder: { branch: string; sha: string }[], - availableNbCommits: { [branch: string]: number } + availableNbCommits: { [branch: string]: number }, + remoteExpRefs: string ) { const { cliError, @@ -134,7 +138,7 @@ export class ExperimentsModel extends ModelWithPersistence { hasCheckpoints, runningExperiments, workspace - } = collectExperiments(expShow, gitLog, dvcLiveOnly) + } = collectExperiments(expShow, gitLog, dvcLiveOnly, remoteExpRefs) const { hasMoreCommits, isShowingMoreCommits } = collectAddRemoveCommitsDetails(availableNbCommits, (branch: string) => @@ -296,6 +300,16 @@ export class ExperimentsModel extends ModelWithPersistence { this.persistStatus() } + public setPushing(ids: string[]) { + this.pushing = new Set(ids) + } + + public unsetPushing(ids: string[]) { + for (const id of ids) { + this.pushing.delete(id) + } + } + public getLabels() { return this.getCombinedList().map(({ label }) => label) } @@ -517,10 +531,20 @@ export class ExperimentsModel extends ModelWithPersistence { private getExperimentsByCommit(commit: Experiment) { const experiments = this.experimentsByCommit .get(commit.id) - ?.map(experiment => ({ - ...this.addDetails(experiment), - branch: commit.branch - })) + ?.map(originalExperiment => { + const experiment = { + ...this.addDetails(originalExperiment), + branch: commit.branch + } + + if (experiment.gitRemoteStatus !== undefined) { + experiment.gitRemoteStatus = this.pushing.has(originalExperiment.id) + ? GitRemoteStatus.PUSHING + : originalExperiment.gitRemoteStatus + } + + return experiment + }) if (!experiments) { return } diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts index 256f1acbfc..f2f31feac8 100644 --- a/extension/src/experiments/webview/contract.ts +++ b/extension/src/experiments/webview/contract.ts @@ -28,6 +28,12 @@ export type CommitData = { date: string } +export enum GitRemoteStatus { + NOT_ON_REMOTE = 'not-on-remote', + PUSHING = 'pushing', + ON_REMOTE = 'on-remote' +} + export type Experiment = { commit?: CommitData Created?: string @@ -41,6 +47,7 @@ export type Experiment = { metrics?: MetricOrParamColumns outs?: MetricOrParamColumns params?: MetricOrParamColumns + gitRemoteStatus?: GitRemoteStatus selected?: boolean sha?: string starred?: boolean diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 39231afc50..953caae74a 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -134,7 +134,11 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< return } - const pushCommand = getPushExperimentCommand(this.internalCommands, setup) + const pushCommand = getPushExperimentCommand( + this, + this.internalCommands, + setup + ) return pushCommand({ dvcRoot, ids }) } diff --git a/extension/src/test/fixtures/expShow/base/remoteExpRefs.ts b/extension/src/test/fixtures/expShow/base/remoteExpRefs.ts new file mode 100644 index 0000000000..2afd1c434e --- /dev/null +++ b/extension/src/test/fixtures/expShow/base/remoteExpRefs.ts @@ -0,0 +1,3 @@ +const data = `42b8736b08170529903cd203a1f40382a4b4a8cd refs/exps/a9/b32d14966b9be1396f2211d9eb743359708a07/test-branch` + +export default data diff --git a/extension/src/test/fixtures/expShow/base/rows.ts b/extension/src/test/fixtures/expShow/base/rows.ts index 9ef0e6151d..dbf6913c37 100644 --- a/extension/src/test/fixtures/expShow/base/rows.ts +++ b/extension/src/test/fixtures/expShow/base/rows.ts @@ -1,5 +1,8 @@ import { join } from '../../../util/path' -import { Commit } from '../../../../experiments/webview/contract' +import { + Commit, + GitRemoteStatus +} from '../../../../experiments/webview/contract' import { copyOriginalColors } from '../../../../experiments/model/status/colors' import { shortenForLabel } from '../../../../util/string' import { @@ -248,6 +251,7 @@ const rowsFixture: Commit[] = [ test: true } }, + gitRemoteStatus: GitRemoteStatus.ON_REMOTE, selected: false, sha: '42b8736b08170529903cd203a1f40382a4b4a8cd', starred: false, @@ -370,6 +374,7 @@ const rowsFixture: Commit[] = [ test: true } }, + gitRemoteStatus: GitRemoteStatus.NOT_ON_REMOTE, selected: false, sha: 'f0f918662b4f8c47819ca154a23029bf9b47d4f3', starred: false, diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 51d73ba9f5..35ac9d0512 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -623,7 +623,7 @@ suite('Experiments Test Suite', () => { }).timeout(WEBVIEW_TEST_TIMEOUT) it('should handle a message to push an experiment', async () => { - const { experiments } = buildExperiments({ disposer: disposable }) + const { experiments } = stubWorkspaceExperimentsGetters(disposable) await experiments.isReady() const mockExpId = 'exp-e7a67' @@ -633,6 +633,7 @@ suite('Experiments Test Suite', () => { const executeCommandSpy = spy(commands, 'executeCommand') const mockExpPush = stub(DvcExecutor.prototype, 'expPush') + stub(experiments, 'update').resolves(undefined) const mockGetStudioAccessToken = stub( Setup.prototype, @@ -1638,6 +1639,7 @@ suite('Experiments Test Suite', () => { availableNbCommits: { main: 20 }, gitLog: '', expShow: data, + remoteExpRefs: '', rowOrder: [ { sha: '2d879497587b80b2d9e61f072d9dbe9c07a65357', branch: 'main' } ] @@ -2117,6 +2119,7 @@ suite('Experiments Test Suite', () => { availableNbCommits: { main: 20 }, gitLog: '', expShow: defaultExperimentsData, + remoteExpRefs: '', rowOrder: [] }) await dataUpdated diff --git a/extension/src/test/suite/experiments/model/tree.test.ts b/extension/src/test/suite/experiments/model/tree.test.ts index 5295a156b1..e934d4c5b9 100644 --- a/extension/src/test/suite/experiments/model/tree.test.ts +++ b/extension/src/test/suite/experiments/model/tree.test.ts @@ -349,6 +349,11 @@ suite('Experiments Tree Test Suite', () => { it('should be able to push an experiment with dvc.views.experimentsTree.pushExperiment', async () => { bypassProgressCloseDelay() + const { experiments } = stubWorkspaceExperimentsGetters(disposable) + await experiments.isReady() + + const mockUpdate = stub(experiments, 'update').resolves(undefined) + const mockExperimentId = 'exp-to-push' const mockExperiment = { dvcRoot: dvcDemoPath, @@ -369,12 +374,18 @@ suite('Experiments Tree Test Suite', () => { ) expect(mockExpPush).to.be.calledWithExactly(dvcDemoPath, mockExperimentId) + expect(mockUpdate).to.be.calledOnce }) it('should be able to push the provided experiment with dvc.views.experimentsTree.pushExperiment (if no experiments are selected)', async () => { bypassProgressCloseDelay() const mockExperiment = 'exp-to-push' + const { experiments } = stubWorkspaceExperimentsGetters(disposable) + await experiments.isReady() + + const mockUpdate = stub(experiments, 'update').resolves(undefined) + const mockExpPush = stub(DvcExecutor.prototype, 'expPush').resolves('') stubPrivatePrototypeMethod( @@ -392,6 +403,7 @@ suite('Experiments Tree Test Suite', () => { ) expect(mockExpPush).to.be.calledWithExactly(dvcDemoPath, mockExperiment) + expect(mockUpdate).to.be.calledOnce }) it('should be able to push multiple experiments with dvc.views.experimentsTree.pushExperiment', async () => { @@ -400,6 +412,11 @@ suite('Experiments Tree Test Suite', () => { const mockSecondExperimentId = 'second-exp-pushed' const mockQueuedExperimentLabel = 'queued-excluded' + const { experiments } = stubWorkspaceExperimentsGetters(disposable) + await experiments.isReady() + + const mockUpdate = stub(experiments, 'update').resolves(undefined) + const mockExpPush = stub(DvcExecutor.prototype, 'expPush').resolves('') stubPrivatePrototypeMethod( @@ -438,6 +455,7 @@ suite('Experiments Tree Test Suite', () => { mockFirstExperimentId, mockSecondExperimentId ) + expect(mockUpdate).to.be.calledOnce }) it('should be able to stop multiple running experiments with dvc.views.experimentsTree.stopExperiment', async () => { diff --git a/extension/src/test/suite/experiments/util.ts b/extension/src/test/suite/experiments/util.ts index 9f02feee6b..0f09f71823 100644 --- a/extension/src/test/suite/experiments/util.ts +++ b/extension/src/test/suite/experiments/util.ts @@ -5,6 +5,7 @@ import { Disposer } from '../../../extension' import expShowFixture from '../../fixtures/expShow/base/output' import gitLogFixture from '../../fixtures/expShow/base/gitLog' import rowOrderFixture from '../../fixtures/expShow/base/rowOrder' +import remoteExpRefsFixture from '../../fixtures/expShow/base/remoteExpRefs' import { buildMockMemento, dvcDemoPath } from '../../util' import { buildDependencies, @@ -25,6 +26,7 @@ export const DEFAULT_EXPERIMENTS_OUTPUT = { availableNbCommits: { main: 5 }, expShow: expShowFixture, gitLog: gitLogFixture, + remoteExpRefs: '', rowOrder: rowOrderFixture } @@ -34,6 +36,7 @@ export const buildExperiments = ({ dvcRoot = dvcDemoPath, expShow = expShowFixture, gitLog = gitLogFixture, + remoteExpRefs = remoteExpRefsFixture, rowOrder = rowOrderFixture, stageList = 'train' }: { @@ -42,6 +45,7 @@ export const buildExperiments = ({ dvcRoot?: string expShow?: ExpShowOutput gitLog?: string + remoteExpRefs?: string rowOrder?: { branch: string; sha: string }[] stageList?: string | null }) => { @@ -95,6 +99,7 @@ export const buildExperiments = ({ availableNbCommits, expShow, gitLog, + remoteExpRefs, rowOrder }) @@ -226,6 +231,7 @@ export const buildExperimentsData = ( stub(gitReader, 'getBranches').resolves([currentBranch, 'one']) stub(gitReader, 'getCommitMessages').resolves(commitOutput) stub(gitReader, 'getNumCommits').resolves(404) + stub(gitReader, 'getRemoteExperimentRefs').resolves('') const mockGetBranchesToShow = stub().returns(['main']) const mockSetBranches = stub() diff --git a/extension/src/test/suite/plots/index.test.ts b/extension/src/test/suite/plots/index.test.ts index 4ca7a1490f..d3d10abf7c 100644 --- a/extension/src/test/suite/plots/index.test.ts +++ b/extension/src/test/suite/plots/index.test.ts @@ -141,6 +141,7 @@ suite('Plots Test Suite', () => { availableNbCommits: { main: 6 }, expShow: updatedExpShowFixture as ExpShowOutput, gitLog: newCommit + COMMITS_SEPARATOR + gitLogFixture, + remoteExpRefs: '', rowOrder: [{ branch: 'main', sha: newCommit }, ...rowOrderFixture] }) diff --git a/extension/src/test/suite/plots/util.ts b/extension/src/test/suite/plots/util.ts index 3c734d92e7..d726ed1a3b 100644 --- a/extension/src/test/suite/plots/util.ts +++ b/extension/src/test/suite/plots/util.ts @@ -34,6 +34,7 @@ export const buildPlots = async ({ plotsDiff = undefined, expShow = expShowFixtureWithoutErrors, gitLog = gitLogFixture, + remoteExpRefs = '', rowOrder = rowOrderFixture }: { availableNbCommits?: { [branch: string]: number } @@ -41,6 +42,7 @@ export const buildPlots = async ({ plotsDiff?: PlotsOutput | undefined expShow?: ExpShowOutput gitLog?: string + remoteExpRefs?: string rowOrder?: { branch: string; sha: string }[] }) => { const { internalCommands, mockPlotsDiff, messageSpy, resourceLocator } = @@ -100,6 +102,7 @@ export const buildPlots = async ({ availableNbCommits, expShow, gitLog, + remoteExpRefs, rowOrder }) diff --git a/webview/icons/codicons.mjs b/webview/icons/codicons.mjs index 7a5c988732..074688bade 100644 --- a/webview/icons/codicons.mjs +++ b/webview/icons/codicons.mjs @@ -7,6 +7,8 @@ export const codicons = [ 'chevron-down', 'chevron-right', 'close', + 'cloud-upload', + 'cloud', 'copy', 'ellipsis', 'error', diff --git a/webview/src/experiments/components/table/body/Cell.tsx b/webview/src/experiments/components/table/body/Cell.tsx index e9358e26ed..6629d7c4ef 100644 --- a/webview/src/experiments/components/table/body/Cell.tsx +++ b/webview/src/experiments/components/table/body/Cell.tsx @@ -1,9 +1,8 @@ import { flexRender } from '@tanstack/react-table' import React, { ReactNode } from 'react' -import { VSCodeProgressRing } from '@vscode/webview-ui-toolkit/react' import cx from 'classnames' -import { isRunning } from 'dvc/src/experiments/webview/contract' import { CellRowActionsProps, CellRowActions } from './CellRowActions' +import { ExperimentStatusIndicator } from './ExperimentStatusIndicator' import styles from '../styles.module.scss' import { CellValue, isValueWithChanges } from '../content/Cell' import { CellProp, RowProp } from '../../../util/interfaces' @@ -39,7 +38,7 @@ const RowExpansionButton: React.FC = ({ row }) => ) -export const FirstCell: React.FC< +export const StubCell: React.FC< CellProp & CellRowActionsProps & { changesIfWorkspace: boolean } > = ({ cell, changesIfWorkspace, ...rowActionsProps }) => { const { @@ -52,7 +51,7 @@ export const FirstCell: React.FC< } } = cell const { - original: { error, status, label, id, description = '' } + original: { error, status, gitRemoteStatus, label, id, description = '' } } = row const { toggleExperiment } = rowActionsProps @@ -61,11 +60,11 @@ export const FirstCell: React.FC<
- {isRunning(status) && ( - - )} + {getIsPlaceholder() ? null : ( diff --git a/webview/src/experiments/components/table/body/ExperimentStatusIndicator.tsx b/webview/src/experiments/components/table/body/ExperimentStatusIndicator.tsx new file mode 100644 index 0000000000..73b362f383 --- /dev/null +++ b/webview/src/experiments/components/table/body/ExperimentStatusIndicator.tsx @@ -0,0 +1,55 @@ +import React from 'react' +import { VSCodeProgressRing } from '@vscode/webview-ui-toolkit/react' +import cx from 'classnames' +import { + ExperimentStatus, + GitRemoteStatus, + isRunning +} from 'dvc/src/experiments/webview/contract' +import { CellHintTooltip } from './CellHintTooltip' +import styles from '../styles.module.scss' +import { clickAndEnterProps } from '../../../../util/props' +import { pushExperiment } from '../../../util/messages' +import { Cloud, CloudUpload } from '../../../../shared/components/icons' +import { Icon } from '../../../../shared/components/Icon' + +type ExperimentStatusIndicatorProps = { + status: ExperimentStatus | undefined + gitRemoteStatus: GitRemoteStatus | undefined + id: string +} + +export const ExperimentStatusIndicator: React.FC< + ExperimentStatusIndicatorProps +> = ({ id, status, gitRemoteStatus }) => { + if (isRunning(status) || gitRemoteStatus === GitRemoteStatus.PUSHING) { + return ( + + ) + } + + if (gitRemoteStatus === GitRemoteStatus.NOT_ON_REMOTE) { + return ( + +
pushExperiment(id))} + > + +
+ + ) + } + + if (gitRemoteStatus === GitRemoteStatus.ON_REMOTE) { + return ( + +
+ +
+ + ) + } +} diff --git a/webview/src/experiments/components/table/body/Row.tsx b/webview/src/experiments/components/table/body/Row.tsx index 499ba215ec..14e0daeb57 100644 --- a/webview/src/experiments/components/table/body/Row.tsx +++ b/webview/src/experiments/components/table/body/Row.tsx @@ -2,7 +2,7 @@ import cx from 'classnames' import React, { useCallback, useContext, useMemo } from 'react' import { useSelector } from 'react-redux' import { EXPERIMENT_WORKSPACE_ID } from 'dvc/src/cli/dvc/contract' -import { FirstCell, CellWrapper } from './Cell' +import { StubCell, CellWrapper } from './Cell' import { RowContextMenu } from './RowContextMenu' import styles from '../styles.module.scss' import { RowProp } from '../../../util/interfaces' @@ -25,7 +25,7 @@ export const RowContent: React.FC< ) const { getVisibleCells, original, getIsExpanded, subRows } = row const { branch, displayColor, error, starred, id } = original - const [firstCell, ...cells] = getVisibleCells() + const [stubCell, ...cells] = getVisibleCells() const isWorkspace = id === EXPERIMENT_WORKSPACE_ID const changesIfWorkspace = isWorkspace ? changes : undefined @@ -82,8 +82,8 @@ export const RowContent: React.FC< aria-selected={isRowSelected} data-testid={isWorkspace && 'workspace-row'} > - export const addConfiguration = () => sendMessage({ type: MessageFromWebviewType.ADD_CONFIGURATION }) +export const pushExperiment = (id: string) => + sendMessage({ payload: [id], type: MessageFromWebviewType.PUSH_EXPERIMENT }) + export const reorderColumns = (newOrder: string[]) => sendMessage({ payload: newOrder, diff --git a/webview/src/shared/components/icons/Cloud.tsx b/webview/src/shared/components/icons/Cloud.tsx new file mode 100644 index 0000000000..2935ae38a5 --- /dev/null +++ b/webview/src/shared/components/icons/Cloud.tsx @@ -0,0 +1,19 @@ +import * as React from 'react' +import type { SVGProps } from 'react' +const Cloud = (props: SVGProps) => ( + + + +) +export default Cloud diff --git a/webview/src/shared/components/icons/CloudUpload.tsx b/webview/src/shared/components/icons/CloudUpload.tsx new file mode 100644 index 0000000000..6b4fdfeb85 --- /dev/null +++ b/webview/src/shared/components/icons/CloudUpload.tsx @@ -0,0 +1,19 @@ +import * as React from 'react' +import type { SVGProps } from 'react' +const CloudUpload = (props: SVGProps) => ( + + + +) +export default CloudUpload diff --git a/webview/src/shared/components/icons/index.ts b/webview/src/shared/components/icons/index.ts index 1d4e51cddf..9b84911d67 100644 --- a/webview/src/shared/components/icons/index.ts +++ b/webview/src/shared/components/icons/index.ts @@ -7,6 +7,8 @@ export { default as ChevronDown } from './ChevronDown' export { default as ChevronRight } from './ChevronRight' export { default as Clock } from './Clock' export { default as Close } from './Close' +export { default as Cloud } from './Cloud' +export { default as CloudUpload } from './CloudUpload' export { default as Copy } from './Copy' export { default as Ellipsis } from './Ellipsis' export { default as Error } from './Error'