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.

Sign up for GitHub

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

Jump to bottom

docs(kfp): choose a workflow executor. Part of kubeflow/pipelines#5718 #2857

Merged
merged 4 commits into from
Aug 11, 2021
Merged

docs(kfp): choose a workflow executor. Part of kubeflow/pipelines#5718 #2857

merged 4 commits into from
Aug 11, 2021

Conversation

Bobgy
Copy link
Contributor

@Bobgy Bobgy commented Aug 6, 2021 •
edited
Loading

@google-cla google-cla bot added the cla: yes label Aug 6, 2021
@Bobgy Bobgy mentioned this pull request Aug 6, 2021
Closed
10 tasks
@Bobgy
Copy link
Contributor Author

Bobgy commented Aug 6, 2021

/hold
wait for 1.7.0 release to merge the PR

zijianjoy
zijianjoy reviewed Aug 6, 2021
View reviewed changes
Copy link
Contributor

@zijianjoy zijianjoy left a comment

Choose a reason for hiding this comment

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

/lgtm

Awesome documentation! Thank you Yuan for unblocking all KFP users! Maybe we can add the executor options in https://deploy-preview-2857--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/installation/standalone-deployment/#deploying-kubeflow-pipelines, so user can make a decision when they first install KFP.

content/en/docs/components/pipelines/installation/choose-executor.md Show resolved Hide resolved
shuesc1
shuesc1 reviewed Aug 6, 2021
View reviewed changes
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
content/en/docs/components/pipelines/installation/choose-executor.md Outdated Show resolved Hide resolved
@Bobgy
Copy link
Contributor Author

Bobgy commented Aug 9, 2021

Thank you @zijianjoy @shuesc1 for the detailed review!
They are super helpful to me.

@zijianjoy
Copy link
Contributor

/lgtm

@Bobgy Bobgy mentioned this pull request Aug 10, 2021
Closed
@Bobgy
Copy link
Contributor Author

Bobgy commented Aug 11, 2021

/unhold
Because the documentation clearly says it's only supported in 1.7+, so we can merge now.

@Bobgy
Copy link
Contributor Author

Bobgy commented Aug 11, 2021

/approve

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy

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 2d910be into master Aug 11, 2021
@Bobgy Bobgy deleted the kfp-emissary branch August 11, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shuesc1 shuesc1 shuesc1 left review comments

@zijianjoy zijianjoy zijianjoy left review comments

@alfsuse alfsuse Awaiting requested review from alfsuse

@joeliedtke joeliedtke Awaiting requested review from joeliedtke

Assignees

@zijianjoy zijianjoy

Labels
approved cla: yes lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Bobgy @zijianjoy @google-oss-robot @shuesc1