[MLMD] Upstreamed all KFP Lineage changes till 2020-03-06.

avdaredevil commented 4 years ago

Addresses: #217 fixes #149

About

Upstreamed all KFP Lineage changes till 2020-03-06
Changes:
- Router Updates
- Stripping out a ton of pipelines, runs and archives code from updates
- Deep linking support added with a combination of HashRouter patch
- Execution and Artifact details page now includes all the key-value pairs from metadata, with the new table layout

Meta

/area metadata /area front-end /priority p1 /assign @avdaredevil /cc @kwasi @Bobgy @jlewi

This change is

k8s-ci-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign avdaredevil You can assign the PR to them by writing /assign @avdaredevil in a comment when ready.

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

Needs approval from an approver in each of these files: - **[frontend/OWNERS](https://github.com/kubeflow/metadata/blob/master/frontend/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

k8s-ci-robot commented 4 years ago

@avdaredevil: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
kubeflow-metadata-presubmit	fe128d3cf724391458acb51614fe399b4afb74d5	link	`/test kubeflow-metadata-presubmit`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).

jlewi commented 4 years ago

/assign @zhenghuiwang

zhenghuiwang commented 4 years ago

LGTM. Thanks for making the changes. Any plan to componentize the code so that we don't need a manual merge them?

presubmit tests should be passed once you rebase.

Bobgy commented 4 years ago

@zhenghuiwang Current problem is we don't have enough bandwidth (and priority) on componentizing all these. I will try refactoring some as I develop major new features at best effort.

zhenghuiwang commented 4 years ago

@zhenghuiwang Current problem is we don't have enough bandwidth (and priority) on componentizing all these. I will try refactoring some as I develop major new features at best effort.

Thanks @Bobgy

avdaredevil commented 4 years ago

Attn: @jlewi

From Meeting Notes:

This PR is stuck because there are tests failing:
- Some of these tests were added in Metadata and don't exist in Pipelines
- As there is a lot of code complexity from Pipelines and no benchmark to base these tests, they will either need to be removed or updated manually (for which I don't have any cycles)
Ownership?
- Most code here is from KFP, with notable contributions from Jason, AP, Kwasi, etc.
- Since the original codebase is forked off KFP, we'd like to:
- Migrate the Details pages and Routing logic to kubeflow/frontend (for shareability)
  - Important for customers to provide a platform agnostic layer that shows metadata from all sources
- OR, load the UI off KFP OSS
  - Easier, but makes Kubeflow opinionated, and may be a struggle as new metadata queries may come to KFP directly rather than being addressed as PRs from the community

jlewi commented 4 years ago

@zhenghuiwang @neuromage any suggestions on the path forward here?

zhenghuiwang commented 4 years ago

Thanks @avdaredevil for the summarization of the situation and possible solutions.

Abstracting out the shared logic/components looks right to me. If there isn't much difference between kubeflow metadata UI and Pipeline UI then abstracting out shouldn't require much effort. If KFP metadata UI is very specific to KFP, then loading it directly seems not a proper solution. Having the reusable abstraction will reduce the work of integrating it to these two UIs.

Ownership?

Since the original codebase is forked off KFP, we'd like to:

Migrate the Details pages and Routing logic to kubeflow/frontend (for shareability)

Important for customers to provide a platform agnostic layer that shows metadata from all sources

avdaredevil commented 4 years ago

Ping @zhenghuiwang @Bobgy.

For now should we disable broken tests, mark as skip, and merge. (we can verify manually, since I tested this manually a while back)

zhenghuiwang commented 4 years ago

Sounds good to me.

For now should we disable broken tests, mark as skip, and merge. (we can verify manually, since I tested this manually a while back)

Bobgy commented 4 years ago

SGTM too

jlewi commented 4 years ago

@avdaredevil looks like you got the greenlight to disable tests to get this merged.

jlewi commented 4 years ago

@avdaredevil Any plans to update this and merge it?

jlewi commented 4 years ago

Merging manually just so we can close this out.

kubeflow / metadata