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

feat(frontend): Runtime DAG in RunDetailsV2. Fix #6673 #6694

Merged
merged 3 commits into from
Oct 7, 2021

Conversation

zijianjoy
Copy link
Collaborator

@zijianjoy zijianjoy commented Oct 6, 2021

Description of your changes:

Fix #6673
Partial #6686
Partial #6400

  1. Mock backend using V2 pipeline spec.
  2. Instead of mocking MLMD backend, port the existing MLMD store which has corresponded V2 compatible run's MLMD info.
  3. Build RunDetailsRouter which switch between KFPv1 and v2 based on manifest format.
  4. Show dynamic DAG based on pipeline spec and MLMD.
  5. SidePanel/SubDAG/RunDetails/etc. are not ready yet.
poc-runtimedag.mov

Checklist:

@zijianjoy
Copy link
Collaborator Author

/test kubeflow-pipelines-samples-v2

@zijianjoy
Copy link
Collaborator Author

/retest-required

@zijianjoy
Copy link
Collaborator Author

Merging this for now to enable upcoming tasks. Feel free to add comments and I will accommodate them in new PR.

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot google-oss-robot merged commit 74c7773 into kubeflow:master Oct 7, 2021
Copy link
Contributor

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

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

/lgtm
only some nits

data.run.pipeline_spec.workflow_manifest
) {
// TODO(zijianjoy): We need to switch to use pipeline_manifest for new API implementation.
const isIR = isIrPipeline(data.run.pipeline_spec.workflow_manifest);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: avoid the word IR in code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It makes sense, updated.

const { isSuccess, data } = useQuery<ApiRunDetail, Error>(
['run_detail', { id: runId }],
() => Apis.runServiceApi.getRun(runId),
{ staleTime: 30000 },
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it would be easier for me to understand if you make this a constant or comment about what the unit is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is a good point, extracting to constant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Draw static graph first, then update static graph with runtime MLMD execution and artifact, and edge.
3 participants