Skip to content

Commit

Permalink
chore(frontend): Integrate run functions with KFP v2 API (#8963)
Browse files Browse the repository at this point in the history 
* Separate runServiceApiV1 and runServiceApiV2.

* Add helper function for v2 run in StatusUtils and Utils.
Separate v1 anv v2 status by creating a new file StatusV2.tsx

* Integrate run details page with v2 API.

* Integrate run list page with v2 API.
Update unit tests.

* Integrate archive run with v2 API.
Format.

* Integrate create / clone run with v2 API.

* Integrate retry / terminate run with v2 API.
Note: retrying or terminating v1 run is also using v2 API.

* Integrate compare run with V2 API.

* Add unit tests and snapshots for StatusV2.

* Add logics to get pipeline_spec via pipeline_version_id in Run details
router.
Make pipeline_spec and version_id exclusive in createRun request body.
Remove unused tests (related to run metric)

* Change the previous changes about filter in component/experimentList.
This change should be in the other PR.

* Change filter in listRuns API request.
Update unit tests for run list and experiment list
Update snapshots.

* Remove unused import and format.

* Change relative path back to absolute path.

* Remove duplicated helper function in utils.
Remove unused helper function in status utils.
Added warn / error to handle unexpected state.

* Fix yaml exception bug in clone run.
Change relative path to absolute path.
  • Loading branch information
@jlyaoyuli
jlyaoyuli authored Mar 15, 2023
1 parent 540342a commit ca2004c
Show file tree
Hide file tree
Showing 38 changed files with 8,407 additions and 2,620 deletions.
46 changes: 22 additions & 24 deletions frontend/src/components/ExperimentList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@
*/

import * as React from 'react';
import * as Utils from '../lib/Utils';
import * as Utils from 'src/lib/Utils';
import { ExperimentList, ExperimentListProps } from './ExperimentList';
import TestUtils from '../TestUtils';
import { ApiFilter, PredicateOp } from '../apis/filter';
import { ApiRunStorageState } from '../apis/run';
import { ApiExperimentStorageState } from '../apis/experiment';
import { V2beta1ExperimentStorageState } from '../apisv2beta1/experiment';
import TestUtils from 'src/TestUtils';
import { ApiFilter, PredicateOp } from 'src/apis/filter';
import { V2beta1ExperimentStorageState } from 'src/apisv2beta1/experiment';
import { V2beta1RunStorageState } from 'src/apisv2beta1/run';
import { ExpandState } from './CustomTable';

import { Apis, ExperimentSortKeys, ListRequest } from '../lib/Apis';
import { Apis, ExperimentSortKeys, ListRequest } from 'src/lib/Apis';
import { ReactWrapper, ShallowWrapper, shallow } from 'enzyme';
import { range } from 'lodash';
import { V2beta1Filter, V2beta1PredicateOperation } from 'src/apisv2beta1/filter';

class ExperimentListTest extends ExperimentList {
public _loadExperiments(request: ListRequest): Promise<string> {
Expand All @@ -43,7 +43,7 @@ describe('ExperimentList', () => {
// We mock this because it uses toLocaleDateString, which causes mismatches between local and CI
// test enviroments
const formatDateStringSpy = jest.spyOn(Utils, 'formatDateString');
const listRunsSpy = jest.spyOn(Apis.runServiceApi, 'listRuns');
const listRunsSpy = jest.spyOn(Apis.runServiceApiV2, 'listRuns');

function generateProps(): ExperimentListProps {
return {
Expand Down Expand Up @@ -278,24 +278,23 @@ describe('ExperimentList', () => {
display_name: 'experiment with id: testexperiment1',
},
]);
expect(Apis.runServiceApi.listRuns).toHaveBeenCalledTimes(1);
expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith(
expect(Apis.runServiceApiV2.listRuns).toHaveBeenCalledTimes(1);
expect(Apis.runServiceApiV2.listRuns).toHaveBeenLastCalledWith(
undefined,
'testexperiment1',
'',
10,
'created_at desc',
'EXPERIMENT',
'testexperiment1',
encodeURIComponent(
JSON.stringify({
predicates: [
{
key: 'storage_state',
op: PredicateOp.NOTEQUALS,
string_value: ApiExperimentStorageState.ARCHIVED.toString(),
// TODO(jlyaoyuli): Change to v2 storage state after run integration
operation: V2beta1PredicateOperation.NOTEQUALS,
string_value: V2beta1RunStorageState.ARCHIVED.toString(),
},
],
} as ApiFilter),
} as V2beta1Filter),
),
);
});
Expand Down Expand Up @@ -330,24 +329,23 @@ describe('ExperimentList', () => {
display_name: 'experiment with id: testexperiment1',
},
]);
expect(Apis.runServiceApi.listRuns).toHaveBeenCalledTimes(1);
expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith(
expect(Apis.runServiceApiV2.listRuns).toHaveBeenCalledTimes(1);
expect(Apis.runServiceApiV2.listRuns).toHaveBeenLastCalledWith(
undefined,
'testexperiment1',
'',
10,
'created_at desc',
'EXPERIMENT',
'testexperiment1',
encodeURIComponent(
JSON.stringify({
predicates: [
{
key: 'storage_state',
op: PredicateOp.EQUALS,
string_value: ApiExperimentStorageState.ARCHIVED.toString(),
// TODO(jlyaoyuli): Change to v2 storage state after run integration
operation: V2beta1PredicateOperation.EQUALS,
string_value: V2beta1ExperimentStorageState.ARCHIVED.toString(),
},
],
} as ApiFilter),
} as V2beta1Filter),
),
);
});
Expand Down
16 changes: 8 additions & 8 deletions frontend/src/components/ExperimentList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ import {
V2beta1Experiment,
V2beta1ExperimentStorageState,
} from 'src/apisv2beta1/experiment';
import { errorToMessage } from '../lib/Utils';
import { errorToMessage } from 'src/lib/Utils';
import { RoutePage, RouteParams } from './Router';
import { commonCss } from '../Css';
import { Apis, ExperimentSortKeys, ListRequest } from '../lib/Apis';
import { ApiRunStorageState } from 'src/apis/run';
import RunList from '../pages/RunList';
import { PredicateOp, ApiFilter } from '../apis/filter';
import { commonCss } from 'src/Css';
import { Apis, ExperimentSortKeys, ListRequest } from 'src/lib/Apis';
import { V2beta1RunStorageState } from 'src/apisv2beta1/run';
import RunList from 'src/pages/RunList';
import { PredicateOp, ApiFilter } from 'src/apis/filter';
import produce from 'immer';
import Tooltip from '@material-ui/core/Tooltip';

Expand Down Expand Up @@ -197,8 +197,8 @@ export class ExperimentList extends React.PureComponent<ExperimentListProps, Exp
noFilterBox={true}
storageState={
this.props.storageState === V2beta1ExperimentStorageState.ARCHIVED
? ApiRunStorageState.ARCHIVED
: ApiRunStorageState.AVAILABLE
? V2beta1RunStorageState.ARCHIVED
: V2beta1RunStorageState.AVAILABLE
}
disableSorting={true}
disableSelection={true}
Expand Down
13 changes: 3 additions & 10 deletions frontend/src/components/viewers/MetricsDropdown.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,8 @@ describe('MetricsDropdown', () => {
scalarMetricsArtifacts = [
{
run: {
run: {
id: '1',
name: 'run1',
},
pipeline_runtime: {
workflow_manifest: '',
},
run_id: '1',
display_name: 'run1',
},
executionArtifacts: [
{
Expand Down Expand Up @@ -179,9 +174,7 @@ describe('MetricsDropdown', () => {
const scalarMetricsArtifactsNoRunName: RunArtifact[] = [
{
run: {
run: {
id: '1',
},
run_id: '1',
},
executionArtifacts: [
{
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/components/viewers/MetricsDropdown.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,9 @@ function getDropdownSubItems(executionArtifacts: ExecutionArtifact[]) {
function getDropdownItems(filteredRunArtifacts: RunArtifact[]) {
const dropdownItems: DropdownItem[] = [];
for (const runArtifact of filteredRunArtifacts) {
const runName = runArtifact.run.run?.name;
const runName = runArtifact.run.display_name;
if (!runName) {
logDisplayNameWarning('run', runArtifact.run.run!.id!);
logDisplayNameWarning('run', runArtifact.run.run_id!);
continue;
}

Expand All @@ -342,7 +342,7 @@ function getLinkedArtifactFromSelectedItem(
selectedItem: SelectedItem,
): LinkedArtifact | undefined {
const filteredRunArtifact = filteredRunArtifacts.find(
runArtifact => runArtifact.run.run?.name === selectedItem.itemName,
runArtifact => runArtifact.run.display_name === selectedItem.itemName,
);

const executionArtifact = filteredRunArtifact?.executionArtifacts.find(executionArtifact => {
Expand Down
39 changes: 26 additions & 13 deletions frontend/src/lib/Apis.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
// limitations under the License.

import * as portableFetch from 'portable-fetch';
import { ExperimentServiceApi, FetchAPI } from '../apis/experiment';
import { ExperimentServiceApi as ExperimentServiceApiV2 } from '../apisv2beta1/experiment';
import { JobServiceApi } from '../apis/job';
import { ApiPipeline, ApiPipelineVersion, PipelineServiceApi } from '../apis/pipeline';
import { RunServiceApi } from '../apis/run';
import { ApiVisualization, VisualizationServiceApi } from '../apis/visualization';
import { HTMLViewerConfig } from '../components/viewers/HTMLViewer';
import { PlotType } from '../components/viewers/Viewer';
import { ExperimentServiceApi, FetchAPI } from 'src/apis/experiment';
import { ExperimentServiceApi as ExperimentServiceApiV2 } from 'src/apisv2beta1/experiment';
import { JobServiceApi } from 'src/apis/job';
import { ApiPipeline, ApiPipelineVersion, PipelineServiceApi } from 'src/apis/pipeline';
import { RunServiceApi as RunServiceApiV1 } from 'src/apis/run';
import { RunServiceApi as RunServiceApiV2 } from 'src/apisv2beta1/run';
import { ApiVisualization, VisualizationServiceApi } from 'src/apis/visualization';
import { HTMLViewerConfig } from 'src/components/viewers/HTMLViewer';
import { PlotType } from 'src/components/viewers/Viewer';
import * as Utils from './Utils';
import { buildQuery } from './Utils';
import { StoragePath, StorageService } from './WorkflowParser';
Expand Down Expand Up @@ -184,15 +185,26 @@ export class Apis {
return this._pipelineServiceApi;
}

public static get runServiceApi(): RunServiceApi {
if (!this._runServiceApi) {
this._runServiceApi = new RunServiceApi(
public static get runServiceApi(): RunServiceApiV1 {
if (!this._runServiceApiV1) {
this._runServiceApiV1 = new RunServiceApiV1(
{ basePath: this.basePath },
undefined,
crossBrowserFetch,
);
}
return this._runServiceApi;
return this._runServiceApiV1;
}

public static get runServiceApiV2(): RunServiceApiV2 {
if (!this._runServiceApiV2) {
this._runServiceApiV2 = new RunServiceApiV2(
{ basePath: this.basePath },
undefined,
crossBrowserFetch,
);
}
return this._runServiceApiV2;
}

public static get visualizationServiceApi(): VisualizationServiceApi {
Expand Down Expand Up @@ -397,7 +409,8 @@ export class Apis {
private static _experimentServiceApiV2?: ExperimentServiceApiV2;
private static _jobServiceApi?: JobServiceApi;
private static _pipelineServiceApi?: PipelineServiceApi;
private static _runServiceApi?: RunServiceApi;
private static _runServiceApiV1?: RunServiceApiV1;
private static _runServiceApiV2?: RunServiceApiV2;
private static _visualizationServiceApi?: VisualizationServiceApi;

/**
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/lib/Buttons.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import AddIcon from '@material-ui/icons/Add';
import CollapseIcon from '@material-ui/icons/UnfoldLess';
import ExpandIcon from '@material-ui/icons/UnfoldMore';
import { QUERY_PARAMS, RoutePage } from '../components/Router';
import { ToolbarActionMap } from '../components/Toolbar';
import { PageProps } from '../pages/Page';
import { QUERY_PARAMS, RoutePage } from 'src/components/Router';
import { ToolbarActionMap } from 'src/components/Toolbar';
import { PageProps } from 'src/pages/Page';
import { Apis } from './Apis';
import { URLParser } from './URLParser';
import { errorToMessage, s } from './Utils';
Expand Down Expand Up @@ -408,7 +408,7 @@ export default class Buttons {
selectedIds,
'Retry this run?',
useCurrent,
id => Apis.runServiceApi.retryRun(id),
id => Apis.runServiceApiV2.retryRun(id),
callback,
'Retry',
'run',
Expand All @@ -429,7 +429,7 @@ export default class Buttons {
`be stopped if it's running when it's archived. Use the Restore action to restore the ` +
`run${s(selectedIds)} to ${selectedIds.length === 1 ? 'its' : 'their'} original location.`,
useCurrent,
id => Apis.runServiceApi.archiveRun(id),
id => Apis.runServiceApiV2.archiveRun(id),
callback,
'Archive',
'run',
Expand All @@ -447,7 +447,7 @@ export default class Buttons {
selectedIds.length === 1 ? 'this run to its' : 'these runs to their'
} original location?`,
useCurrent,
id => Apis.runServiceApi.unarchiveRun(id),
id => Apis.runServiceApiV2.unarchiveRun(id),
callback,
'Restore',
'run',
Expand Down Expand Up @@ -540,7 +540,7 @@ export default class Buttons {
'Do you want to terminate this run? This action cannot be undone. This will terminate any' +
' running pods, but they will not be deleted.',
useCurrentResource,
id => Apis.runServiceApi.terminateRun(id),
id => Apis.runServiceApiV2.terminateRun(id),
callback,
'Terminate',
'run',
Expand Down
73 changes: 71 additions & 2 deletions frontend/src/lib/StatusUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
* limitations under the License.
*/

import { logger } from '../lib/Utils';
import { NodeStatus } from '../third_party/mlmd/argo_template';
import { logger } from 'src/lib/Utils';
import { NodeStatus } from 'src/third_party/mlmd/argo_template';
import { V2beta1RuntimeState } from 'src/apisv2beta1/run';

export const statusBgColors = {
error: '#fce8e6',
Expand All @@ -41,6 +42,18 @@ export enum NodePhase {
OMITTED = 'Omitted',
}

export const statusProtoMap = new Map<V2beta1RuntimeState, string>([
[V2beta1RuntimeState.RUNTIMESTATEUNSPECIFIED, 'Unknown'],
[V2beta1RuntimeState.PENDING, 'Pending'],
[V2beta1RuntimeState.RUNNING, 'Running'],
[V2beta1RuntimeState.SUCCEEDED, 'Succeeded'],
[V2beta1RuntimeState.SKIPPED, 'Skipped'],
[V2beta1RuntimeState.FAILED, 'Failed'],
[V2beta1RuntimeState.CANCELING, 'Canceling'],
[V2beta1RuntimeState.CANCELED, 'Canceled'],
[V2beta1RuntimeState.PAUSED, 'Paused'],
]);

export function hasFinished(status?: NodePhase): boolean {
switch (status) {
case NodePhase.SUCCEEDED: // Fall through
Expand Down Expand Up @@ -116,3 +129,59 @@ function wasNodeCached(node: NodeStatus): boolean {
? false
: artifacts.some(artifact => artifact.s3 && !artifact.s3.key.includes(node.id));
}

// separate these helper function for paritial v2 api integration
export function hasFinishedV2(state?: V2beta1RuntimeState): boolean {
switch (state) {
case V2beta1RuntimeState.SUCCEEDED: // Fall through
case V2beta1RuntimeState.SKIPPED: // Fall through
case V2beta1RuntimeState.FAILED: // Fall through
case V2beta1RuntimeState.CANCELED:
return true;
case V2beta1RuntimeState.PENDING: // Fall through
case V2beta1RuntimeState.RUNNING: // Fall through
case V2beta1RuntimeState.CANCELING: // Fall through
case V2beta1RuntimeState.RUNTIMESTATEUNSPECIFIED:
return false;
default:
logger.warn('Unknown state:', state);
throw new Error('Unexpected runtime state!');
}
}

export function statusToBgColorV2(state?: V2beta1RuntimeState, nodeMessage?: string): string {
state = checkIfTerminatedV2(state, nodeMessage);
switch (state) {
case V2beta1RuntimeState.FAILED:
return statusBgColors.error;
case V2beta1RuntimeState.PENDING:
return statusBgColors.notStarted;
case V2beta1RuntimeState.CANCELING:
// fall through
case V2beta1RuntimeState.RUNNING:
return statusBgColors.running;
case V2beta1RuntimeState.SUCCEEDED:
return statusBgColors.succeeded;
case V2beta1RuntimeState.SKIPPED:
// fall through
case V2beta1RuntimeState.CANCELED:
return statusBgColors.terminatedOrSkipped;
case V2beta1RuntimeState.RUNTIMESTATEUNSPECIFIED:
// fall through
default:
logger.verbose('Unknown state:', state);
return statusBgColors.notStarted;
}
}

export function checkIfTerminatedV2(
state?: V2beta1RuntimeState,
nodeMessage?: string,
): V2beta1RuntimeState | undefined {
// Argo considers terminated runs as having "Failed", so we have to examine the failure message to
// determine why the run failed.
if (state === V2beta1RuntimeState.FAILED && nodeMessage === 'terminated') {
state = V2beta1RuntimeState.CANCELED;
}
return state;
}
Loading

0 comments on commit ca2004c

Please sign in to comment.