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

Handle non-standard experiment pipeline configurations #4264

Merged
merged 15 commits into from
Jul 16, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 2 additions & 2 deletions extension/src/cli/dvc/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const autoRegisteredCommands = {
GLOBAL_VERSION: 'globalVersion',
PLOTS_DIFF: 'plotsDiff',
ROOT: 'root',
STAGE_LIST: 'listStages',
STAGE_LIST: 'stageList',
VERSION: 'version'
} as const

Expand Down Expand Up @@ -144,7 +144,7 @@ export class DvcReader extends DvcCli {
return this.executeProcess(options)
}

public async listStages(cwd: string): Promise<string | undefined> {
public async stageList(cwd: string): Promise<string | undefined> {
try {
return await this.executeDvcProcess(cwd, Command.STAGE, SubCommand.LIST)
} catch {}
Expand Down
15 changes: 2 additions & 13 deletions extension/src/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ export abstract class BaseData<
T extends
| { data: PlotsOutputOrError; revs: string[] }
| ExperimentsOutput
| string
| { dag: string; stages: { [pipeline: string]: string | undefined } }
> extends DeferredDisposable {
public readonly onDidUpdate: Event<T>
public readonly onDidChangeDvcYaml: Event<void>

protected readonly dvcRoot: string
protected readonly processManager: ProcessManager
Expand All @@ -36,10 +35,6 @@ export abstract class BaseData<
new EventEmitter()
)

private readonly dvcYamlChanged: EventEmitter<void> = this.dispose.track(
new EventEmitter<void>()
)

constructor(
dvcRoot: string,
internalCommands: InternalCommands,
Expand All @@ -56,8 +51,6 @@ export abstract class BaseData<
this.onDidUpdate = this.updated.event
this.staticFiles = staticFiles

this.onDidChangeDvcYaml = this.dvcYamlChanged.event

this.watchFiles()

this.waitForInitialData()
Expand Down Expand Up @@ -96,13 +89,9 @@ export abstract class BaseData<
) {
void this.managedUpdate(path)
}

if (path.endsWith('dvc.yaml')) {
this.dvcYamlChanged.fire()
}
}
)
}

abstract managedUpdate(path?: string): Promise<unknown>
abstract managedUpdate(path?: string): Promise<void>
}
42 changes: 22 additions & 20 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { ConfigKey, getConfigValue, setUserConfigValue } from '../vscode/config'
import { checkSignalFile, pollSignalFileForProcess } from '../fileSystem'
import { DVCLIVE_ONLY_RUNNING_SIGNAL_FILE } from '../cli/dvc/constants'
import { Response } from '../vscode/response'
import { Pipeline } from '../pipeline'

export const ExperimentsScale = {
...omit(ColumnType, 'TIMESTAMP'),
Expand All @@ -64,8 +65,9 @@ export class Experiments extends BaseRepository<TableData> {

public readonly viewKey = ViewKey.EXPERIMENTS

private readonly data: ExperimentsData
private readonly pipeline: Pipeline

private readonly data: ExperimentsData
private readonly experiments: ExperimentsModel
private readonly columns: ColumnsModel

Expand Down Expand Up @@ -96,17 +98,16 @@ export class Experiments extends BaseRepository<TableData> {
private dvcLiveOnlyCleanupInitialized = false
private dvcLiveOnlySignalFile: string

private readonly addStage: () => Promise<boolean>
private readonly selectBranches: (
branchesSelected: string[]
) => Promise<string[] | undefined>

constructor(
dvcRoot: string,
internalCommands: InternalCommands,
pipeline: Pipeline,
resourceLocator: ResourceLocator,
workspaceState: Memento,
addStage: () => Promise<boolean>,
selectBranches: (
branchesSelected: string[]
) => Promise<string[] | undefined>,
Expand All @@ -120,7 +121,7 @@ export class Experiments extends BaseRepository<TableData> {
)

this.internalCommands = internalCommands
this.addStage = addStage
this.pipeline = pipeline
this.selectBranches = selectBranches

this.onDidChangeIsExperimentsFileFocused = this.experimentsFileFocused.event
Expand All @@ -146,11 +147,6 @@ export class Experiments extends BaseRepository<TableData> {
)

this.dispose.track(this.data.onDidUpdate(data => this.setState(data)))
this.dispose.track(
this.data.onDidChangeDvcYaml(() =>
this.webviewMessages.changeHasConfig(true)
)
)

this.dispose.track(
workspace.onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
Expand Down Expand Up @@ -435,7 +431,8 @@ export class Experiments extends BaseRepository<TableData> {
}

public async modifyWorkspaceParamsAndRun(
commandId: ModifiedExperimentAndRunCommandId
commandId: ModifiedExperimentAndRunCommandId,
cwd: string
) {
const paramsToModify = await this.pickAndModifyParams()
if (!paramsToModify) {
Expand All @@ -444,13 +441,13 @@ export class Experiments extends BaseRepository<TableData> {

await this.internalCommands.executeCommand<string>(
commandId,
this.dvcRoot,
cwd,
...paramsToModify
)
return this.notifyChanged()
}

public async modifyWorkspaceParamsAndQueue() {
public async modifyWorkspaceParamsAndQueue(cwd: string) {
const paramsToModify = await this.pickAndModifyParams()
if (!paramsToModify) {
return
Expand All @@ -459,7 +456,7 @@ export class Experiments extends BaseRepository<TableData> {
await Toast.showOutput(
this.internalCommands.executeCommand<string>(
AvailableCommands.EXP_QUEUE,
this.dvcRoot,
cwd,
...paramsToModify
)
)
Expand Down Expand Up @@ -521,16 +518,26 @@ export class Experiments extends BaseRepository<TableData> {
return this.columns.getRelativeMetricsFiles()
}

public getPipelineCwd() {
return this.pipeline.getCwd()
}

protected sendInitialWebviewData() {
return this.webviewMessages.sendWebviewMessage()
}

private setupInitialData() {
const waitForInitialData = this.dispose.track(
this.onDidChangeExperiments(() => {
this.onDidChangeExperiments(async () => {
await this.pipeline.isReady()
this.deferred.resolve()
this.dispose.untrack(waitForInitialData)
waitForInitialData.dispose()
this.dispose.track(
this.pipeline.onDidUpdate(() =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[F] This replaces changeHasConfig

this.webviewMessages.sendWebviewMessage()
)
)
})
)
}
Expand All @@ -555,15 +562,10 @@ export class Experiments extends BaseRepository<TableData> {
this.dvcRoot,
this.experiments,
this.columns,
this.pipeline,
() => this.getWebview(),
() => this.notifyChanged(),
() => this.selectColumns(),
() =>
this.internalCommands.executeCommand(
AvailableCommands.STAGE_LIST,
this.dvcRoot
),
() => this.addStage(),
(branchesSelected: string[]) => this.selectBranches(branchesSelected),
() => this.data.update()
)
Expand Down
1 change: 0 additions & 1 deletion extension/src/experiments/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ export type TableData = {
hasConfig: boolean
hasMoreCommits: Record<string, boolean>
hasRunningWorkspaceExperiment: boolean
hasValidDvcYaml: boolean
isShowingMoreCommits: Record<string, boolean>
rows: Commit[]
selectedForPlotsCount: number
Expand Down
31 changes: 7 additions & 24 deletions extension/src/experiments/webview/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,20 @@ import { SortDefinition } from '../model/sortBy'
import { getPositiveIntegerInput } from '../../vscode/inputBox'
import { Title } from '../../vscode/title'
import { ConfigKey, setConfigValue } from '../../vscode/config'
import { hasDvcYamlFile } from '../../fileSystem'
import { NUM_OF_COMMITS_TO_INCREASE } from '../../cli/dvc/constants'
import { Pipeline } from '../../pipeline'

export class WebviewMessages {
private readonly dvcRoot: string

private readonly experiments: ExperimentsModel
private readonly columns: ColumnsModel
private readonly pipeline: Pipeline

private readonly getWebview: () => BaseWebview<TableData> | undefined
private readonly notifyChanged: () => void
private readonly selectColumns: () => Promise<void>

private readonly hasStages: () => Promise<string | undefined>

private hasConfig = false
private hasValidDvcYaml = true

private readonly addStage: () => Promise<boolean>
private readonly selectBranches: (
branchesSelected: string[]
) => Promise<string[] | undefined>
Expand All @@ -49,11 +44,10 @@ export class WebviewMessages {
dvcRoot: string,
experiments: ExperimentsModel,
columns: ColumnsModel,
pipeline: Pipeline,
getWebview: () => BaseWebview<TableData> | undefined,
notifyChanged: () => void,
selectColumns: () => Promise<void>,
hasStages: () => Promise<string>,
addStage: () => Promise<boolean>,
selectBranches: (
branchesSelected: string[]
) => Promise<string[] | undefined>,
Expand All @@ -62,22 +56,12 @@ export class WebviewMessages {
this.dvcRoot = dvcRoot
this.experiments = experiments
this.columns = columns
this.pipeline = pipeline
this.getWebview = getWebview
this.notifyChanged = notifyChanged
this.selectColumns = selectColumns
this.hasStages = hasStages
this.addStage = addStage
this.selectBranches = selectBranches
this.update = update

void this.changeHasConfig()
}

public async changeHasConfig(update?: boolean) {
const stages = await this.hasStages()
this.hasValidDvcYaml = !hasDvcYamlFile(this.dvcRoot) || stages !== undefined
this.hasConfig = !!stages
update && this.sendWebviewMessage()
}

public sendWebviewMessage() {
Expand Down Expand Up @@ -267,20 +251,19 @@ export class WebviewMessages {
this.experiments.getAvailableBranchesToShow().length > 0,
hasCheckpoints: this.experiments.hasCheckpoints(),
hasColumns: this.columns.hasNonDefaultColumns(),
hasConfig: this.hasConfig,
hasConfig: !!(this.pipeline.hasStage() || this.pipeline.hasPipeline()),
mattseddon marked this conversation as resolved.
Show resolved Hide resolved
hasMoreCommits: this.experiments.getHasMoreCommits(),
hasRunningWorkspaceExperiment:
this.experiments.hasRunningWorkspaceExperiment(),
hasValidDvcYaml: this.hasValidDvcYaml,
isShowingMoreCommits: this.experiments.getIsShowingMoreCommits(),
rows: this.experiments.getRowData(),
selectedForPlotsCount: this.experiments.getSelectedRevisions().length,
sorts: this.experiments.getSorts()
}
}

private async addConfiguration() {
await this.addStage()
private addConfiguration() {
return this.pipeline.checkOrAddPipeline()
}

private async setMaxTableHeadDepth() {
Expand Down
Loading
Loading