Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions extension/src/cli/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ export const MIN_CLI_VERSION = '2.11.0'
export const LATEST_TESTED_CLI_VERSION = '2.13.0'
export const MAX_CLI_VERSION = '3'

export const UNEXPECTED_ERROR_CODE = 255

export enum Command {
ADD = 'add',
CHECKOUT = 'checkout',
Expand Down
37 changes: 36 additions & 1 deletion extension/src/cli/reader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { join } from 'path'
import { EventEmitter } from 'vscode'
import { Disposable, Disposer } from '@hediet/std/disposable'
import { CliResult, CliStarted } from '.'
import { CliReader } from './reader'
import { UNEXPECTED_ERROR_CODE } from './constants'
import { MaybeConsoleError } from './error'
import { CliReader, ExperimentsOutput } from './reader'
import { createProcess } from '../processExecution'
import { getFailingMockedProcess, getMockedProcess } from '../test/util/jest'
import { getProcessEnv } from '../env'
Expand Down Expand Up @@ -76,6 +78,39 @@ describe('CliReader', () => {
executable: 'dvc'
})
})

it('should return the default output if the cli returns an unexpected error (255 exit code)', async () => {
const cwd = __dirname
const error = new Error('unexpected error - something something')
;(error as MaybeConsoleError).exitCode = UNEXPECTED_ERROR_CODE
mockedCreateProcess.mockImplementationOnce(() => {
throw error
})

const cliOutput = await cliReader.expShow(cwd)
expect(cliOutput).toStrictEqual({ workspace: { baseline: {} } })
})

it('should retry the cli given any other type of error', async () => {
const cwd = __dirname
const mockOutput: ExperimentsOutput = {
workspace: {
baseline: {
data: { params: { 'params.yaml': { data: { epochs: 100000000 } } } }
}
}
}
mockedCreateProcess.mockImplementationOnce(() => {
throw new Error('error that should be retried')
})
mockedCreateProcess.mockReturnValueOnce(
getMockedProcess(JSON.stringify(mockOutput))
)

const cliOutput = await cliReader.expShow(cwd)
expect(cliOutput).toStrictEqual(mockOutput)
expect(mockedCreateProcess).toBeCalledTimes(2)
})
})

describe('diff', () => {
Expand Down
13 changes: 10 additions & 3 deletions extension/src/cli/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ export interface ExperimentsOutput {
}
}

const defaultExperimentsOutput: ExperimentsOutput = {
workspace: { baseline: {} }
}

export interface PlotsOutput {
[path: string]: Plot[]
}
Expand All @@ -132,11 +136,14 @@ export class CliReader extends Cli {
cwd: string,
...flags: ExperimentFlag[]
): Promise<ExperimentsOutput> {
return this.readProcessJson<ExperimentsOutput>(
return this.readProcess<ExperimentsOutput>(
cwd,
JSON.parse,
JSON.stringify(defaultExperimentsOutput),
Command.EXPERIMENT,
SubCommand.SHOW,
...flags
...flags,
Flag.SHOW_JSON
)
}

Expand All @@ -158,7 +165,7 @@ export class CliReader extends Cli {
return this.readProcessJson<PlotsOutput>(
cwd,
Command.PLOTS,
'diff',
Command.DIFF,
...revisions,
Flag.OUTPUT_PATH,
TEMP_PLOTS_DIR,
Expand Down
4 changes: 2 additions & 2 deletions extension/src/cli/retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('retry', () => {

const promiseRefresher = jest.fn().mockImplementation(() => promise())

const output = await retry<string>(promiseRefresher, 'Definitely did not')
const output = await retry(promiseRefresher, 'Definitely did not')

expect(output).toStrictEqual(returnValue)

Expand Down Expand Up @@ -45,7 +45,7 @@ describe('retry', () => {
.fn()
.mockImplementation(() => unreliablePromise())

await retry<string>(promiseRefresher, 'Data update')
await retry(promiseRefresher, 'Data update')

expect(promiseRefresher).toBeCalledTimes(4)
expect(mockedDelay).toBeCalledTimes(3)
Expand Down
16 changes: 13 additions & 3 deletions extension/src/cli/retry.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
import { UNEXPECTED_ERROR_CODE } from './constants'
import { MaybeConsoleError } from './error'
import { delay } from '../util/time'
import { Logger } from '../common/logger'

export const retry = async <T>(
getNewPromise: () => Promise<T>,
const isUnexpectedError = (error: unknown): boolean => {
return (error as MaybeConsoleError)?.exitCode === UNEXPECTED_ERROR_CODE
}

export const retry = async (
getNewPromise: () => Promise<string>,
args: string,
waitBeforeRetry = 500
): Promise<T> => {
): Promise<string> => {
try {
return await getNewPromise()
} catch (error: unknown) {
const errorMessage = (error as Error).message
Logger.error(`${args} failed with ${errorMessage} retrying...`)

if (isUnexpectedError(error)) {
return ''
}

await delay(waitBeforeRetry)
return retry(getNewPromise, args, waitBeforeRetry * 2)
}
Expand Down
4 changes: 2 additions & 2 deletions extension/src/experiments/data/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { ExperimentsOutput } from '../../cli/reader'
export const collectFiles = (data: ExperimentsOutput): string[] => {
const files = new Set<string>(
Object.keys({
...data?.workspace.baseline?.data?.params,
...data?.workspace.baseline?.data?.metrics
...data?.workspace?.baseline?.data?.params,
...data?.workspace?.baseline?.data?.metrics
}).filter(Boolean)
)

Expand Down
4 changes: 4 additions & 0 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ export class Experiments extends BaseRepository<TableData> {
}

public getExperimentCount() {
if (!this.columns.hasColumns()) {
return 0
}

return this.experiments.getExperimentCount()
}

Expand Down
15 changes: 8 additions & 7 deletions extension/src/experiments/model/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,14 @@ export class ExperimentsTree

private getDescription() {
const dvcRoots = this.experiments.getDvcRoots()
if (!definedAndNonEmpty(dvcRoots)) {

const total = sum(
dvcRoots.map(dvcRoot =>
this.experiments.getRepository(dvcRoot).getExperimentCount()
)
)

if (!total) {
return
}

Expand All @@ -279,12 +286,6 @@ export class ExperimentsTree
)
)

const total = sum(
dvcRoots.map(dvcRoot =>
this.experiments.getRepository(dvcRoot).getExperimentCount()
)
)

return `${selected} of ${total} (max ${MAX_SELECTED_EXPERIMENTS})`
}

Expand Down