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

Rebase to KFP 1.0 #224

Closed
animeshsingh opened this issue Jul 11, 2020 · 7 comments · Fixed by #251
Closed

Rebase to KFP 1.0 #224

animeshsingh opened this issue Jul 11, 2020 · 7 comments · Fixed by #251

Comments

@animeshsingh
Copy link
Collaborator

https://github.com/kubeflow/pipelines/tree/1.0.0-rc.5

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
kind/feature 0.70

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@Tomcli
Copy link
Member

Tomcli commented Jul 20, 2020

FYI we should rebase using the official release from today
https://github.com/kubeflow/pipelines/releases/tag/1.0.0

Tasks:

  • Backend
  • Frontend
  • SDK
  • Manifest

@Tomcli
Copy link
Member

Tomcli commented Jul 21, 2020

Additional tasks for 1.0 multiusers:

  • Implement a separate KFP-Tekton controller for injecting default S3 credentials for artifacts. In multi-user, pipelines are not suppose to mount on a secret, instead it should be injected by the runtime controller. (This feature is part of the new Argo controller, but since Tekton has not yet implement the concept of artifact, we need to add our own custom implementation via webhook injection)
  • Create configmap for KFP-Tekton specific config. For artifact and logging, we have our own implementation because Tekton doesn't support it out of the box. Therefore, we need a dedicated configmap to conifigure the settings on our artifact and logging archive.
  • Update UI to handle artifacts with the new multiuser code.

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
area/frontend 0.67
area/backend 0.55

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@ckadner
Copy link
Member

ckadner commented Jul 25, 2020

The KFP 1.0.0 Rebase is a work in progress that requires collaboration between API, UI, SDK experts.

Proposed process:

  • PR KFP 1.0.0 rebase and SDK update #244 targets the kfp-1.0-rebase branch, @Tomcli @drewbutlerbb4 review it, minor comments can be addressed there and if there are no obvious problems we merge that PR
  • @Tomcli and @drewbutlerbb4 checkout the kfp-1.0-rebase branch locally and push further PRs to the the kfp-1.0-rebase branch with additional code changes that are required to make the API and UI work with the KFP 1.0 code base
  • after all PRs on the kfp-1.0-rebase branch are merged, the kfp-tekton-bot squashes all commits from the kfp-1.0-rebase branch and creates a single PR onto the master branch with all consolidate KFP 1.0 rebase/merge changes

TODOs after PR #244:

  • frontend/src/pages/RunDetails.tsx: 91: // TODO: kfp 1.0.0 merge
  • frontend/src/pages/RunDetails.tsx: 869: // TODO: kfp 1.0.0 merge
  • backend/Dockerfile: 50: # TODO: kfp 1.0.0 merge
  • backend/metadata_writer/src/metadata_writer.py: 106: # TODO: kfp 1.0.0 merge
  • backend/src/common/util/workflow.go: 281: // TODO: kfp 1.0.0 merge
  • manifests/kustomize/base/kustomization.yaml: 16: # TODO: kfp 1.0.0 merge
  • manifests/kustomize/base/pipeline/ml-pipeline-apiserver-deployment.yaml: 1: # TODO: kfp 1.0.0 merge

@Tomcli
Copy link
Member

Tomcli commented Jul 28, 2020

Side note: The artifact and execution page doesn't have pagination because the metadata service has not yet support it, so users will try to query all artifacts (including the ones owned by other users) when they land the artifact page which can be slow for frequently use.

Tracking issue: kubeflow/pipelines#3226

@Tomcli
Copy link
Member

Tomcli commented Aug 3, 2020

For the new artifact design with multi user, we are planning to move on to tekton 0.15 for using new context variables as our s3 key. This way the implementation can be a bit more close to argo.
https://github.com/tektoncd/pipeline/blob/master/docs/variables.md

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

Successfully merging a pull request may close this issue.

3 participants