Skip to content
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

Call exp show with multiple branches #3827

Merged
merged 15 commits into from
May 16, 2023
3 changes: 2 additions & 1 deletion extension/src/cli/dvc/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ export enum ExperimentFlag {
NO_FETCH = '--no-fetch',
QUEUE = '--queue',
RESET = '--reset',
NUM_COMMIT = '-n'
NUM_COMMIT = '-n',
REV = '--rev'
}

export enum GcPreserveFlag {
Expand Down
2 changes: 2 additions & 0 deletions extension/src/cli/dvc/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,13 @@ export type ExecutorState = {
export type ExpWithError = {
rev: string
name?: string
branch: string
} & DvcError

type ExpWithData = {
rev: string
name?: string
branch: string
data: ExpData
}

Expand Down
5 changes: 4 additions & 1 deletion extension/src/cli/dvc/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ describe('CliReader', () => {
const cliOutput = await dvcReader.expShow(cwd)
expect(cliOutput).toStrictEqual([
{
branch: '',
error: { msg: unexpectedStderr, type: 'Caught error' },
rev: EXPERIMENT_WORKSPACE_ID
}
Expand All @@ -110,7 +111,9 @@ describe('CliReader', () => {
)

const cliOutput = await dvcReader.expShow(cwd)
expect(cliOutput).toStrictEqual([{ rev: EXPERIMENT_WORKSPACE_ID }])
expect(cliOutput).toStrictEqual([
{ branch: '', rev: EXPERIMENT_WORKSPACE_ID }
])
})
})

Expand Down
4 changes: 3 additions & 1 deletion extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ export class DvcReader extends DvcCli {
Flag.JSON
)
if (isDvcError(output) || isEmpty(output)) {
return [{ rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }]
return [
{ branch: '', rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }
]
}
return output
}
Expand Down
2 changes: 1 addition & 1 deletion extension/src/cli/git/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ export enum Command {

export enum Flag {
ALL = '--all',
COUNT = '--count',
DIRECTORIES = '-d',
DIRECTORY = '--directory',
DOT = '.',
EXCLUDE_STANDARD = '--exclude-standard',
FORCE = '-f',
FULL_HISTORY = '--full-history',
GET_URL = '--get-url',
HARD = '--hard',
NAME_ONLY = '--name-only',
Expand Down
29 changes: 19 additions & 10 deletions extension/src/cli/git/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { isDirectory } from '../../fileSystem'
export const autoRegisteredCommands = {
GIT_GET_BRANCHES: 'getBranches',
GIT_GET_COMMIT_MESSAGES: 'getCommitMessages',
sroy3 marked this conversation as resolved.
Show resolved Hide resolved
GIT_GET_CURRENT_BRANCH: 'getCurrentBranch',
GIT_GET_NUM_COMMITS: 'getNumCommits',
GIT_GET_REMOTE_URL: 'getRemoteUrl',
GIT_GET_REPOSITORY_ROOT: 'getGitRepositoryRoot',
Expand Down Expand Up @@ -80,18 +81,13 @@ export class GitReader extends GitCli {
return new Set([...files, ...dirs])
}

public async getNumCommits(cwd: string) {
const options = getOptions(
cwd,
Command.REV_LIST,
Flag.FULL_HISTORY,
Flag.ALL
)
public async getNumCommits(cwd: string, branch: string) {
const options = getOptions(cwd, Command.REV_LIST, Flag.COUNT, branch)
try {
const revisions = await this.executeProcess(options)
return trimAndSplit(revisions).length
const nbCommits = await this.executeProcess(options)
return Number.parseInt(nbCommits)
} catch {
return ''
return 0
}
}

Expand All @@ -105,6 +101,19 @@ export class GitReader extends GitCli {
}
}

public async getCurrentBranch(cwd: string): Promise<string> {
const options = getOptions(cwd, Command.BRANCH)
try {
const branches = await this.executeProcess(options)
return trimAndSplit(branches)[0]
.replace('* (', '')
.replace(')', '')
.replace('HEAD detached at ', '')
} catch {
return ''
}
}

private async getUntrackedDirectories(cwd: string): Promise<string[]> {
const options = getOptions(
cwd,
Expand Down
1 change: 1 addition & 0 deletions extension/src/experiments/columns/collect/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ describe('collectRelativeMetricsFiles', () => {
expect(
collectRelativeMetricsFiles([
{
branch: 'current',
rev: EXPERIMENT_WORKSPACE_ID,
error: { msg: 'I broke', type: 'not important' }
}
Expand Down
1 change: 1 addition & 0 deletions extension/src/experiments/data/collect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('collectFiles', () => {
it('should handle an error being returned', () => {
const workspaceOnly = [
{
branch: 'current',
error: { msg: 'bad things are happening', type: 'today' },
rev: EXPERIMENT_WORKSPACE_ID
}
Expand Down
81 changes: 69 additions & 12 deletions extension/src/experiments/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { getRelativePattern } from '../../fileSystem/relativePattern'
sroy3 marked this conversation as resolved.
Show resolved Hide resolved
import { createFileSystemWatcher } from '../../fileSystem/watcher'
import { AvailableCommands, InternalCommands } from '../../commands/internal'
import { ExpShowOutput } from '../../cli/dvc/contract'
import { EXPERIMENT_WORKSPACE_ID, ExpShowOutput } from '../../cli/dvc/contract'
import { BaseData } from '../../data'
import { DOT_DVC, ExperimentFlag } from '../../cli/dvc/constants'
import { gitPath } from '../../cli/git/constants'
Expand Down Expand Up @@ -41,17 +41,10 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
}

public async update(): Promise<void> {
const flags = this.experiments.getIsBranchesView()
? [ExperimentFlag.ALL_BRANCHES]
: [
ExperimentFlag.NUM_COMMIT,
this.experiments.getNbOfCommitsToShow().toString()
]
const data = await this.internalCommands.executeCommand<ExpShowOutput>(
AvailableCommands.EXP_SHOW,
this.dvcRoot,
...flags
)
const isBranchesView = this.experiments.getIsBranchesView()
const data: ExpShowOutput = isBranchesView
? ((await this.expShow([ExperimentFlag.ALL_BRANCHES])) as ExpShowOutput)
: await this.updateCommitsView()

this.collectFiles(data)

Expand All @@ -62,6 +55,70 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
this.collectedFiles = collectFiles(data, this.collectedFiles)
}

private async getBranchesToShowWithCurrent() {
const currentBranch = await this.internalCommands.executeCommand<string>(
AvailableCommands.GIT_GET_CURRENT_BRANCH,
this.dvcRoot
)

const branches = this.experiments.getBranchesToShow()

if (!branches.includes(currentBranch)) {
branches.push(currentBranch)
this.experiments.setBranchesToShow(branches)
}
return { branches, currentBranch }
}

private async expShow(flags: (ExperimentFlag | string)[], branch?: string) {
const data = await this.internalCommands.executeCommand<ExpShowOutput>(
AvailableCommands.EXP_SHOW,
this.dvcRoot,
...flags
)
return data.map(exp => ({ ...exp, branch }))
}

private async updateCommitsView() {
const data: ExpShowOutput = []

const { branches, currentBranch } =
await this.getBranchesToShowWithCurrent()

const output = []
for (const branch of branches) {
const branchFlags = [
ExperimentFlag.REV,
branch,
ExperimentFlag.NUM_COMMIT,
this.experiments.getNbOfCommitsToShow(branch).toString()
]
output.push(
new Promise<void>((resolve, reject) => {
this.expShow(branchFlags, branch)
.then(output => {
const newData = output as ExpShowOutput
if (branch !== currentBranch) {
const workspaceIndex = newData.findIndex(
exp => exp.rev === EXPERIMENT_WORKSPACE_ID
)
newData.splice(workspaceIndex, 1)
}
data.push(...newData)
resolve()
})
.catch(() => {
reject(new Error('Output failed'))
sroy3 marked this conversation as resolved.
Show resolved Hide resolved
})
})
)
}

await Promise.all(output)

return data
}

private async updateAvailableBranchesToSelect() {
const allBranches = await this.internalCommands.executeCommand<string[]>(
AvailableCommands.GIT_GET_BRANCHES,
Expand Down
5 changes: 3 additions & 2 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,10 +594,11 @@ export class Experiments extends BaseRepository<TableData> {
),
() => this.addStage(),
(branchesSelected: string[]) => this.selectBranches(branchesSelected),
() =>
(branch: string) =>
this.internalCommands.executeCommand<number>(
AvailableCommands.GIT_GET_NUM_COMMITS,
this.dvcRoot
this.dvcRoot,
branch
),
() => this.data.update()
)
Expand Down
14 changes: 10 additions & 4 deletions extension/src/experiments/model/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,17 @@ const collectExpState = (
expState: ExpState,
commitData: { [sha: string]: CommitData }
): Experiment | undefined => {
const { rev, name } = expState
const { rev, name, branch } = expState
const label =
rev === EXPERIMENT_WORKSPACE_ID
? EXPERIMENT_WORKSPACE_ID
: name || shortenForLabel(rev)
const id = name || label

const experiment: Experiment = { id, label }
const experiment: Experiment = { id, label } as unknown as Experiment
sroy3 marked this conversation as resolved.
Show resolved Hide resolved

const baseline = transformExpState(experiment, expState)
baseline.branch = branch

if (rev === EXPERIMENT_WORKSPACE_ID && !name) {
acc.workspace = baseline
Expand Down Expand Up @@ -247,7 +248,7 @@ const collectExpRange = (
}
const expState = revs[0]

const { name, rev } = expState
const { name, rev, branch } = expState

const label =
rev === EXPERIMENT_WORKSPACE_ID
Expand All @@ -256,6 +257,7 @@ const collectExpRange = (

const experiment = transformExpState(
{
branch,
id: name || label,
label
},
Expand Down Expand Up @@ -317,7 +319,11 @@ export const collectExperiments = (
experimentsByCommit: new Map(),
hasCheckpoints: hasCheckpoints(output),
runningExperiments: [],
workspace: { id: EXPERIMENT_WORKSPACE_ID, label: EXPERIMENT_WORKSPACE_ID }
workspace: {
branch: '',
sroy3 marked this conversation as resolved.
Show resolved Hide resolved
id: EXPERIMENT_WORKSPACE_ID,
label: EXPERIMENT_WORKSPACE_ID
}
}

const commitData = getCommitData(commitsOutput)
Expand Down
12 changes: 8 additions & 4 deletions extension/src/experiments/model/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,21 @@ describe('ExperimentsModel', () => {
it('should set the number of commits to show correctly', () => {
const model = new ExperimentsModel('', buildMockMemento())

model.setNbfCommitsToShow(42)
model.setNbfCommitsToShow(42, 'special-branch')

expect(model.getNbOfCommitsToShow()).toBe(42)
expect(model.getNbOfCommitsToShow('special-branch')).toBe(42)
})

it('should persist the number of commits to show when changing it', () => {
const memento = buildMockMemento()
const model = new ExperimentsModel('', memento)

model.setNbfCommitsToShow(42)
model.setNbfCommitsToShow(42, 'main')

expect(memento.get(PersistenceKey.NUMBER_OF_COMMITS_TO_SHOW)).toBe(42)
expect(memento.get(PersistenceKey.NUMBER_OF_COMMITS_TO_SHOW)).toStrictEqual(
{
main: 42
}
)
})
})
20 changes: 14 additions & 6 deletions extension/src/experiments/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class ExperimentsModel extends ModelWithPersistence {
private availableColors: Color[]
private coloredStatus: ColoredStatus
private starredExperiments: StarredExperiments
private numberOfCommitsToShow: number
private numberOfCommitsToShow: Record<string, number>
private isBranchesView: boolean
private branchesToShow: string[] = []
private availableBranchesToShow: string[] = []
Expand Down Expand Up @@ -92,11 +92,15 @@ export class ExperimentsModel extends ModelWithPersistence {
PersistenceKey.EXPERIMENTS_STARS,
{}
)
this.numberOfCommitsToShow = this.revive<number>(
this.numberOfCommitsToShow = this.revive<Record<string, number>>(
PersistenceKey.NUMBER_OF_COMMITS_TO_SHOW,
DEFAULT_NUM_OF_COMMITS_TO_SHOW
{}
)

if (typeof this.numberOfCommitsToShow === 'number') {
this.numberOfCommitsToShow = {}
}

this.branchesToShow = this.revive<string[]>(
PersistenceKey.EXPERIMENTS_BRANCHES,
[]
Expand Down Expand Up @@ -410,12 +414,16 @@ export class ExperimentsModel extends ModelWithPersistence {
return this.finishedRunning
}

public setNbfCommitsToShow(numberOfCommitsToShow: number) {
this.numberOfCommitsToShow = numberOfCommitsToShow
public setNbfCommitsToShow(numberOfCommitsToShow: number, branch: string) {
this.numberOfCommitsToShow[branch] = numberOfCommitsToShow
this.persistNbOfCommitsToShow()
}

public getNbOfCommitsToShow() {
public getNbOfCommitsToShow(branch: string) {
return this.numberOfCommitsToShow[branch] || DEFAULT_NUM_OF_COMMITS_TO_SHOW
}

public getAllNbOfCommitsToShow() {
return this.numberOfCommitsToShow
}

Expand Down
Loading