Skip to content

Commit

Permalink
Merge branch 'main' into handle-uncommitted-deps
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Aug 17, 2022
2 parents 1813c21 + e343265 commit ad9900b
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 28 deletions.
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
2 changes: 1 addition & 1 deletion webview/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
"ts-loader": "9.3.1",
"webpack": "5.74.0",
"webpack-cli": "4.10.0",
"webpack-dev-server": "4.9.3"
"webpack-dev-server": "4.10.0"
},
"svgr": {
"typescript": true,
Expand Down
51 changes: 47 additions & 4 deletions webview/src/experiments/components/table/styles.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,22 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding;
background-color: $bg-color;
}

.unselectedExperiment:not(.rowSelected):hover & {
background-color: $row-hover-background-color;
}

.unselectedExperiment:not(.rowSelected) .experimentCell:hover & {
background-color: $cell-hover-background-color;
}

.workspaceWithChanges.unselectedExperiment & {
border: 1px solid $changed-color;
}

.rowSelected.unselectedExperiment & {
background-color: $row-bg-selected-color;
}

.queuedExperiment & {
display: none;
}
Expand All @@ -148,10 +160,28 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding;
background-color: $bg-color;
}

.runningExperiment:not(.rowSelected):hover & {
background-color: $row-hover-background-color;
border-left-color: $row-hover-background-color;
border-bottom-color: $row-hover-background-color;
}

.runningExperiment:not(.rowSelected) .experimentCell:hover & {
background-color: $cell-hover-background-color;
border-left-color: $cell-hover-background-color;
border-bottom-color: $cell-hover-background-color;
}

.workspaceWithChanges.runningExperiment & {
border-right-color: $changed-color;
border-top-color: $changed-color;
}

.rowSelected.runningExperiment & {
background-color: $row-bg-selected-color;
border-left-color: $row-bg-selected-color;
border-bottom-color: $row-bg-selected-color;
}
}
}

Expand Down Expand Up @@ -254,12 +284,14 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding;
position: relative;

&:hover:not(.rowSelected) {
.td {
.td:not(.experimentCell),
.experimentCell:before {
background-color: $row-hover-background-color;
}

&:hover {
background-color: $cell-hover-background-color;
}
.td:hover:not(.experimentCell),
.experimentCell:hover:before {
background-color: $cell-hover-background-color;
}
}

Expand Down Expand Up @@ -709,6 +741,17 @@ $badge-size: 0.85rem;
}

.experimentCell {
&:before {
content: '';
top: 0;
right: 0;
width: 100%;
height: 100%;
z-index: -1;
position: absolute;
background-color: transparent;
}

.innerCell {
.indicatorCount {
display: none;
Expand Down
2 changes: 1 addition & 1 deletion webview/src/shared/variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ $bg-color: var(--vscode-editor-background);
$bullet-size: 9px;
$watermark-color: var(--vscode-descriptionForeground);

$border-color: var(--checkbox-border);
$border-color: var(--vscode-checkbox-border);
$metrics-color: var(--vscode-dvc-metrics);
$params-color: var(--vscode-dvc-params);
$deps-color: var(--vscode-dvc-deps);
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -18383,10 +18383,10 @@ webpack-dev-middleware@^5.3.1:
range-parser "^1.2.1"
schema-utils "^4.0.0"

webpack-dev-server@4.9.3:
version "4.9.3"
resolved "https://registry.yarnpkg.com/webpack-dev-server/-/webpack-dev-server-4.9.3.tgz#2360a5d6d532acb5410a668417ad549ee3b8a3c9"
integrity sha512-3qp/eoboZG5/6QgiZ3llN8TUzkSpYg1Ko9khWX1h40MIEUNS2mDoIa8aXsPfskER+GbTvs/IJZ1QTBBhhuetSw==
webpack-dev-server@4.10.0:
version "4.10.0"
resolved "https://registry.yarnpkg.com/webpack-dev-server/-/webpack-dev-server-4.10.0.tgz#de270d0009eba050546912be90116e7fd740a9ca"
integrity sha512-7dezwAs+k6yXVFZ+MaL8VnE+APobiO3zvpp3rBHe/HmWQ+avwh0Q3d0xxacOiBybZZ3syTZw9HXzpa3YNbAZDQ==
dependencies:
"@types/bonjour" "^3.5.9"
"@types/connect-history-api-fallback" "^1.3.5"
Expand Down

0 comments on commit ad9900b

Please sign in to comment.