Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
DaoDaoNoCode committed Apr 16, 2024
1 parent 14c7d67 commit 201fc97
Show file tree
Hide file tree
Showing 13 changed files with 256 additions and 124 deletions.
3 changes: 3 additions & 0 deletions frontend/src/concepts/pipelines/apiHooks/mlmd/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { Context } from '~/third_party/mlmd';

export type MlmdContext = Context;
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { MlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/types';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { Context, Execution, GetExecutionsByContextRequest } from '~/third_party/mlmd';
import { Execution, GetExecutionsByContextRequest } from '~/third_party/mlmd';
import { FAST_POLL_INTERVAL } from '~/utilities/const';
import useFetchState, {
FetchState,
Expand All @@ -9,8 +10,8 @@ import useFetchState, {
} from '~/utilities/useFetchState';

export const useExecutionsFromContext = (
context: Context | null,
pipelineFinished?: boolean,
context: MlmdContext | null,
activelyRefresh?: boolean,
): FetchState<Execution[] | null> => {
const { metadataStoreServiceClient } = usePipelinesAPI();

Expand All @@ -26,6 +27,6 @@ export const useExecutionsFromContext = (
}, [metadataStoreServiceClient, context]);

return useFetchState(call, null, {
refreshRate: !pipelineFinished ? FAST_POLL_INTERVAL : undefined,
refreshRate: !activelyRefresh ? FAST_POLL_INTERVAL : undefined,
});
};
41 changes: 41 additions & 0 deletions frontend/src/concepts/pipelines/apiHooks/mlmd/useMlmdContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import { MlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/types';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import { GetContextByTypeAndNameRequest } from '~/third_party/mlmd';
import { FAST_POLL_INTERVAL } from '~/utilities/const';
import useFetchState, {
FetchState,
FetchStateCallbackPromise,
NotReadyError,
} from '~/utilities/useFetchState';

export const useMlmdContext = (
name?: string,
type?: string,
activelyRefresh?: boolean,
): FetchState<MlmdContext | null> => {
const { metadataStoreServiceClient } = usePipelinesAPI();

const call = React.useCallback<FetchStateCallbackPromise<MlmdContext | null>>(async () => {
if (!type) {
return Promise.reject(new NotReadyError('No context type'));
}
if (!name) {
return Promise.reject(new NotReadyError('No context name'));
}

const request = new GetContextByTypeAndNameRequest();
request.setTypeName(type);
request.setContextName(name);
const res = await metadataStoreServiceClient.getContextByTypeAndName(request);
const context = res.getContext();
if (!context) {
return Promise.reject(new Error('Cannot find specified context'));
}
return context;
}, [metadataStoreServiceClient, type, name]);

return useFetchState(call, null, {
refreshRate: activelyRefresh ? FAST_POLL_INTERVAL : undefined,
});
};
Original file line number Diff line number Diff line change
@@ -1,48 +1,11 @@
import React from 'react';
import { Context } from '~/third_party/mlmd';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import useFetchState, {
FetchState,
FetchStateCallbackPromise,
NotReadyError,
} from '~/utilities/useFetchState';
import { GetContextByTypeAndNameRequest } from '~/third_party/mlmd/generated/ml_metadata/proto/metadata_store_service_pb';
import { FAST_POLL_INTERVAL } from '~/utilities/const';
import { FetchState } from '~/utilities/useFetchState';
import { MlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/types';
import { useMlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/useMlmdContext';

const KFP_V2_RUN_CONTEXT_TYPE = 'system.PipelineRun';

const useMlmdContext = (
name?: string,
type?: string,
pipelineFinished?: boolean,
): FetchState<Context | null> => {
const { metadataStoreServiceClient } = usePipelinesAPI();

const call = React.useCallback<FetchStateCallbackPromise<Context | null>>(async () => {
if (!type) {
return Promise.reject(new NotReadyError('No context type'));
}
if (!name) {
return Promise.reject(new NotReadyError('No context name'));
}

const request = new GetContextByTypeAndNameRequest();
request.setTypeName(type);
request.setContextName(name);
const res = await metadataStoreServiceClient.getContextByTypeAndName(request);
const context = res.getContext();
if (!context) {
return Promise.reject(new Error('Cannot find specified context'));
}
return context;
}, [metadataStoreServiceClient, type, name]);

return useFetchState(call, null, {
refreshRate: !pipelineFinished ? FAST_POLL_INTERVAL : undefined,
});
};

export const usePipelineRunMlmdContext = (
runID?: string,
pipelineFinished?: boolean,
): FetchState<Context | null> => useMlmdContext(runID, KFP_V2_RUN_CONTEXT_TYPE, pipelineFinished);
activelyRefresh?: boolean,
): FetchState<MlmdContext | null> =>
useMlmdContext(runID, KFP_V2_RUN_CONTEXT_TYPE, activelyRefresh);
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { PipelineRunKFv2, RuntimeStateKF, runtimeStateLabels } from '~/concepts/
import { FAST_POLL_INTERVAL } from '~/utilities/const';
import { computeRunStatus } from '~/concepts/pipelines/content/utils';

export const isPipelineRunComplete = (run?: PipelineRunKFv2 | null): boolean => {
export const isPipelineRunFinished = (run?: PipelineRunKFv2 | null): boolean => {
const { label } = computeRunStatus(run);
return [
runtimeStateLabels[RuntimeStateKF.SUCCEEDED],
Expand Down Expand Up @@ -41,13 +41,13 @@ const usePipelineRunById = (
});

const [run] = runData;
const isComplete = isPipelineRunComplete(run);
const isFinished = isPipelineRunFinished(run);

React.useEffect(() => {
if (isComplete) {
if (isFinished) {
setPipelineFinished(true);
}
}, [isComplete]);
}, [isFinished]);

return runData;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { usePipelineTaskTopology } from '~/concepts/pipelines/topology';
import { PipelineRunType } from '~/pages/pipelines/global/runs/types';
import { routePipelineRunsNamespace } from '~/routes';
import PipelineJobReferenceName from '~/concepts/pipelines/content/PipelineJobReferenceName';
import useExecutionsForPipelineRun from '~/concepts/pipelines/content/pipelinesDetails/pipelineRun/useExecutionsForPipelineRun';

const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath, contextPath }) => {
const { runId } = useParams();
Expand All @@ -52,21 +53,16 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath,
RunDetailsTabs.DETAILS,
);
const [selectedId, setSelectedId] = React.useState<string | null>(null);
const {
taskMap,
nodes,
loaded: topologyLoaded,
} = usePipelineTaskTopology(pipelineSpec, runResource ?? undefined);

const loaded = runLoaded && (versionLoaded || !!runResource?.pipeline_spec) && topologyLoaded;
const [executions, executionsLoaded, executionsError] = useExecutionsForPipelineRun(runResource);
const { taskMap, nodes } = usePipelineTaskTopology(
pipelineSpec,
runResource?.run_details,
executions,
);

const loaded = runLoaded && (versionLoaded || !!runResource?.pipeline_spec);
const error = versionError || runError;
if (!loaded && !error) {
return (
<Bullseye>
<Spinner />
</Bullseye>
);
}

if (error) {
return (
Expand All @@ -81,6 +77,14 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath,
);
}

if (!loaded || (!executionsLoaded && !executionsError)) {
return (
<Bullseye>
<Spinner />
</Bullseye>
);
}

return (
<>
<Drawer isExpanded={!!selectedId}>
Expand Down Expand Up @@ -140,12 +144,10 @@ const PipelineRunDetails: PipelineCoreDetailsPageComponent = ({ breadcrumbPath,
</Breadcrumb>
}
headerAction={
loaded && (
<PipelineRunDetailsActions
run={runResource}
onDelete={() => setDeleting(true)}
/>
)
<PipelineRunDetailsActions
run={runResource}
onDelete={() => setDeleting(true)}
/>
}
empty={false}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@ import DownloadDropdown from '~/concepts/pipelines/content/pipelinesDetails/pipe
import { PodStepStateType } from '~/types';
import useDebounceCallback from '~/utilities/useDebounceCallback';
import { PipelineTask } from '~/concepts/pipelines/topology';
import { ExecutionStateKF } from '~/concepts/pipelines/kfTypes';

// TODO: If this gets large enough we should look to make this its own component file
const LogsTab: React.FC<{ task: PipelineTask }> = ({ task }) => {
const podName = task.status?.podName;
const isFailedPod = task.status?.state === 'Failed';
const isFailedPod = task.status?.state === ExecutionStateKF.FAILED;

if (!podName) {
return <>No content</>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { useExecutionsFromContext } from '~/concepts/pipelines/apiHooks/mlmd/useExecutionsFromContext';
import { usePipelineRunMlmdContext } from '~/concepts/pipelines/apiHooks/mlmd/usePipelineRunMlmdContext';
import { isPipelineRunFinished } from '~/concepts/pipelines/apiHooks/usePipelineRunById';
import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes';
import { Execution } from '~/third_party/mlmd';

const useExecutionsForPipelineRun = (
run: PipelineRunKFv2 | null,
): [executions: Execution[] | null, loaded: boolean, error?: Error] => {
const isFinished = isPipelineRunFinished(run);
// contextError means mlmd service is not available, no need to check executions
const [context, , contextError] = usePipelineRunMlmdContext(run?.run_id, !isFinished);
// executionsLoaded is the flag to show the spinner or not
const [executions, executionsLoaded] = useExecutionsFromContext(context, !isFinished);

return [executions, executionsLoaded, contextError];
};

export default useExecutionsForPipelineRun;
16 changes: 16 additions & 0 deletions frontend/src/concepts/pipelines/kfTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,22 @@ export enum RuntimeStateKF {
PAUSED = 'PAUSED',
}

export enum ExecutionStateKF {
NEW = 'New',
RUNNING = 'Running',
COMPLETE = 'Complete',
CANCELED = 'Canceled',
FAILED = 'Failed',
CACHED = 'Cached',
}

export enum ArtifactStateKF {
PENDING = 'Pending',
LIVE = 'Live',
MARKED_FOR_DELETION = 'Marked for deletion',
DELETED = 'Deleted',
}

export const runtimeStateLabels = {
[RuntimeStateKF.RUNTIME_STATE_UNSPECIFIED]: 'Unspecified',
[RuntimeStateKF.PENDING]: 'Pending',
Expand Down
Loading

0 comments on commit 201fc97

Please sign in to comment.