Improve the KFP / User Guides / Core Functions docs #3795

thesuperzapper · 2024-07-07T01:44:01Z

This PR makes significant formatting updates and rewording to the Kubeflow Pipelines / User Guides / Core Functions, and Migrate to Kubeflow Pipelines v2 guides.

For the most part this is a formatting update, but I draw reviwers attention to a content change I made on the Connect the SDK to the API page. I have updated the Python script given as an example for connecting the KFP SDK with Dex. I have used the same reference example as in the deployKF docs. (Note, it will work for all other distributions, including manifests, as long as they use Dex).

The most important changes in this PR are:

Re-naming the Core Functions pages to have shorter titles to be more clear
Re-ordering the Core Functions pages to better tell a story and be readable from top to bottom
Significantly improving the Migrate to Kubeflow Pipelines v2 page which was very hard to read before.

The easiest way to review this PR will be to look at the Netlify preview and review the following pages:

Kubeflow Pipelines
- User Guides:
  - Migrate to Kubeflow Pipelines v2
  - Core Functions:
    - (all pages)

After we merge this, we will need to do the same thing for the "Create Components" and "Data Handling" sections.

thesuperzapper · 2024-07-07T01:48:11Z

@hbelmiro I would appreciate your review of this.

I believe it significantly improves the readability and usefulness of the "Core Functions" and "Migrate to Kubeflow Pipelines v2" guides.

/assign @hbelmiro

hbelmiro · 2024-07-08T17:49:45Z

It would be nice if @rimolive and @diegolovison could also take a look.

diegolovison · 2024-07-08T18:02:17Z

content/en/docs/components/pipelines/user-guides/core-functions/platform-specific-features.md


-One of the benefits of KFP is cross-platform portability. The KFP SDK compiles pipeline definitions to [IR YAML][ir-yaml] which can be read and executed by different backends, including the [Kubeflow Pipelines open source backend][oss-be] and [Vertex AI Pipelines](https://cloud.google.com/vertex-ai/docs/pipelines/introduction).
+One of the benefits of KFP is cross-platform portability. 
+The KFP SDK compiles pipeline definitions to [IR YAML][ir-yaml] which can be read and executed by different backends, including the Kubeflow Pipelines [open source backend][oss-be] and [Vertex AI Pipelines](https://cloud.google.com/vertex-ai/docs/pipelines/introduction).


I think the docs must be doc-neutral. May we remove Vertex AI ?

Please see #3795 (comment)

diegolovison · 2024-07-08T18:04:10Z

content/en/docs/components/pipelines/user-guides/core-functions/run-a-pipeline.md


 ```python
-client.create_run_from_pipeline_package('pipeline.yaml', arguments={'param': 'a', 'other_param': 2})
+#from kfp import compiler, dsl


Would you like to keep the code commented?

Yes, the idea is to highlight that there are two stages to this:

Create the YAML

Submit the YAML

I also dont want people accidentially overwiting files by bildly runing the compile code.

diegolovison · 2024-07-08T18:04:48Z

content/en/docs/components/pipelines/user-guides/core-functions/run-a-pipeline.md

 ```

-See the [KFP SDK Client reference documentation][kfp-sdk-api-ref-client] for a detailed description of the `Client` constructor and method parameters.
+## Run Pipeline - KFP CLI


I think the first step should be how to connect KFP CLI to the host

Let's create a follow up issue, because I am not trying to add significant content in this PR, only improve formatting and layout.

I also don't have that information readily available to write up.

diegolovison · 2024-07-08T18:06:28Z

Hi @thesuperzapper

I left some comments. It will be good if you agree with my suggestion do it in a new commit. Easy for review :)
We can squash and merge later.

Tks

rimolive · 2024-07-08T18:08:18Z

content/en/docs/components/pipelines/user-guides/core-functions/caching.md

@@ -26,7 +26,7 @@ be marked with a green "arrow from cloud" icon.
 ## How to use caching

 Caching is enabled by default for all components in KFP. You can disable caching
-for a component by calling `.set_caching_options(False)` on a task object.
+for a component by calling [`.set_caching_options(enable_caching=False)`](https://kubeflow-pipelines.readthedocs.io/en/latest/source/dsl.html#kfp.dsl.PipelineTask.set_caching_options) on a task object.


Isn't safe to pin this link to a specific SDK version?

content/en/docs/components/pipelines/user-guides/core-functions/cli.md

content/en/docs/components/pipelines/user-guides/core-functions/connect-api.md

content/en/docs/components/pipelines/user-guides/core-functions/platform-specific-features.md

rimolive · 2024-07-08T18:17:19Z

I also added some comments but overall lgtm.

content/en/docs/components/pipelines/user-guides/migration.md

hbelmiro · 2024-07-09T12:59:45Z

content/en/docs/components/pipelines/user-guides/core-functions/connect-api.md

-* [Using the Kubeflow Pipelines SDK](/docs/components/pipelines/legacy-v1/tutorials/sdk-examples/)
-* [Kubeflow Pipelines SDK Reference](https://kubeflow-pipelines.readthedocs.io/en/stable/)
-* [Experiment with the Kubeflow Pipelines API](/docs/components/pipelines/legacy-v1/tutorials/api-pipelines/)
+<br>


Do we really need this <br>?

Personally, I think the page is very hard to read without it, it provides separation between two very large sections.

hbelmiro · 2024-07-09T13:11:35Z

...ent/en/docs/components/pipelines/user-guides/core-functions/execute-kfp-pipelines-locally.md


-In the following example, we use the `DockerRunner` type. Runner types are covered in more detail below.
+Local execution comes with several limitations:


Suggested change

Local execution comes with several limitations:

It comes with several limitations:

I think this proposed change makes it unclear what the following bullet points are talking about if you are just skimming the page.

content/en/docs/components/pipelines/user-guides/core-functions/platform-specific-features.md

hbelmiro · 2024-07-09T13:26:19Z

content/en/docs/components/pipelines/user-guides/core-functions/platform-specific-features.md


-In general, platform-specific plugin libraries provide functions that act on tasks similarly to [task-level configuration methods][task-level-config-methods] provided by the KFP SDK directly. Platform-specific plugin libraries may also provide pre-baked components.
+<!-- TODO: add docs on how to create a platform-specific authoring library -->


Rather than adding a TODO comment, please create an issue.

hbelmiro · 2024-07-09T13:55:10Z

content/en/docs/components/pipelines/user-guides/migration.md

+--- | ---
+SDK v1 | The `1.x.x` versions of the [`kfp`](https://pypi.org/project/kfp/1.8.22/) Python SDK.
+SDK v2 | The `2.x.x` versions of the [`kfp`](https://pypi.org/project/kfp/) Python SDK.
+SDK v1 (v2-namespace) | The preview V2 module that was available in the V1 SDK (e.g. `from kfp.v2 import *`).<br>_Only ever used by [Google Cloud Vertex AI Pipelines][vertex-pipelines] users._


I would avoid mentioning products to be more vendor-neutral.

Please see #3795 (comment)

hbelmiro · 2024-07-09T13:59:46Z

content/en/docs/components/pipelines/user-guides/migration.md


-## SDK v1 to SDK v2
+### **Migrate from 'SDK v1' to 'SDK v2'**

 KFP SDK v2 is generally not backward compatible with user code that uses the KFP SDK v1 main namespace. This section describes some of the important breaking changes and migration steps to upgrade to KFP SDK v2.

 We indicate whether each breaking change affects [KFP OSS backend][oss-be-v1] users or [Google Cloud Vertex AI Pipelines][vertex-pipelines] users.


I would avoid mentioning products to be more vendor-neutral.
Breaking changes that affect Vertex AI should be on the Vertex AI documentation. Can we mention only the breaking changes that affect KFP (and remove the "**Affects:** KFP OSS users" warning)?

Please see #3795 (comment)

thesuperzapper · 2024-08-19T19:49:09Z

A few people raised concerns about keeping the content which talks about Vertex AI (on the migration guide). Let's discuss this in a follow up issue, the existing page already mentions Vertex AI, and I don't feel comfortable removing it without a larger community discussion that should not block this PR.

Personally, I think it's important to clarify that there are some parts of the Kubeflow Pipelines SDK are only for Vertex AI (and not used by the open source Kubeflow Pipelines). If we as a community disagree with the presence of this code, it should be removed from the SDK, not silently dropped from the documentation.

thesuperzapper · 2024-08-19T19:53:04Z

I have also fixed an issue which is being reported since the release of 1.9.0 in the Kubeflow Pipelines "Connect the SDK to the API - Outside Cluster" guide:

400: Bad request; unsupported request method. Can't connect the Pipelines SDK to Kubeflow Pipelines with Kind for Kubeflow 1.9 manifests#2844

The issue is simply that Kubeflow 1.9.0 introduced a OIDC "approve grant" screen (which the old script was getting stuck on), see the preview site for the updated code:

https://deploy-preview-3795--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/core-functions/connect-api/#kubeflow-platform---outside-the-cluster

thesuperzapper · 2024-08-19T20:06:53Z

@james-jwu or @chensun or @terrytangyuan can you please approve this important update to the KFP docs.

Among other things, it fixes two critical issues impacting users:

Fixes the "connect SDK outside cluster" script, which is no longer working on Kubeflow 1.9.0:
- https://deploy-preview-3795--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/core-functions/connect-api/#kubeflow-platform---outside-the-cluster
Improves the KFP V2 migration guide, as users are struggling to follow it right now:
- https://deploy-preview-3795--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/migration/

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

juliusvonkohout · 2024-08-20T05:23:10Z

content/en/docs/components/pipelines/user-guides/core-functions/connect-api.md

-
-### Example for Dex
+{{% alert title="Tip" color="info" %}}
+The process to authenticate the Pipelines SDK from outside the cluster will vary by [Kubeflow Distribution](/docs/started/installing-kubeflow/#packaged-distributions) and identity provider.


I think this DEX part should link to the Kubeflow Platform page in general https://www.kubeflow.org/docs/started/installing-kubeflow/#kubeflow-platform instead of directly to the distributions. Furthermore we should say that below DEX there is oauth2-proxy that supports machine to machine authentication with Kubernetes Tokens.

Updated link in 95d20ea.

Let's raise a separate PR which adds a "Kubeflow Platform - Dex Token Exchange", or whatever you want to call it, presuming you are talking about the ability of Dex to exchange external JWTs for its own ones:

https://dexidp.io/docs/guides/token-exchange/

If you are talking about suggesting people exfiltrate their Kubernetes ServiceAccount tokens to use off-cluster I strongly recommend against that.

juliusvonkohout · 2024-08-20T05:27:04Z

I think the DEX part in comnect-api.md should link to the Kubeflow Platform page in general https://www.kubeflow.org/docs/started/installing-kubeflow/#kubeflow-platform instead of directly to the distributions. Furthermore we should say that below DEX there is oauth2-proxy that supports machine to machine authentication with Kubernetes tokens. For machine accounts you should rely on service accounts and not usernames+passwords. Oauth2-proxy should have a more stable interface and be more platform/distribution agnostic in the future. But for some experimentation Dex might be enough for new users.

CC also @kimwnasptd for platform and @andreyvelich @terrytangyuan from the KSC since it is a large change.

juliusvonkohout · 2024-08-20T05:44:14Z

The question is when and how do we want to address the Dex/oauth2-proxy changes in kubeflow/manifests#2844 (comment)

@kimwnasptd @thesuperzapper
@kromanow94 I am fine with presenting both in the documentation and adjusting it slightly as proposed in the comment.

juliusvonkohout · 2024-08-20T05:56:30Z

@thesuperzapper maybe it makes sense to link to the recently added https://github.com/kubeflow/manifests/blob/master/tests%2Fgh-actions%2Ftest_dex_login.py and adjust it if needed. Because this way we always have it up to date and it is tested regularly. Code on the website can soon deprecate or get out of synchronization.

thesuperzapper · 2024-08-20T16:07:27Z

@juliusvonkohout we need to focus on getting this merged so the Dex script on the website actually works with Kubeflow 1.9.0. All previous users will be using the dex approach.

Can you confirm that the new script under "Kubeflow Platform - Outside the Cluster" works on your test 1.9.0 clusters?

In terms of future updates like including the m2m stuff on this page, let's do that in a follow-up PR, so we don't block the fix to the dex script.

PS: I think it's more user friendly to include all the information directly on the website, so we can add explanations and comments, and not break if you update your test script.

PSS: once we merge this PR, please update your test login script to the new one from this PR, because its currently disabled on your repo (hence why the script on the website is broken).

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

thesuperzapper · 2024-08-20T22:27:23Z

@james-jwu or @chensun can you please approve this critical update to the KFP docs?

It fixes two critical issues impacting users:

Fixes the "connect SDK outside cluster" script, which is no longer working on Kubeflow 1.9.0:
- https://deploy-preview-3795--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/core-functions/connect-api/#kubeflow-platform---outside-the-cluster
Improves the KFP V2 migration guide, as users are struggling to follow it right now:
- https://deploy-preview-3795--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/migration/

rimolive · 2024-08-21T13:31:02Z

/lgtm

juliusvonkohout · 2024-08-21T21:48:37Z

@juliusvonkohout we need to focus on getting this merged so the Dex script on the website actually works with Kubeflow 1.9.0. All previous users will be using the dex approach.

/lgtm
with the linking to Kubeflow platform

Can you confirm that the new script under "Kubeflow Platform - Outside the Cluster" works on your test 1.9.0 clusters?

there is positive user feedback in kubeflow/manifests#2844 (comment) but i did not have the time to test it myself yet

In terms of future updates like including the m2m stuff on this page, let's do that in a follow-up PR, so we don't block the fix to the dex script.

Yes that is fine.

PS: I think it's more user friendly to include all the information directly on the website, so we can add explanations and comments, and not break if you update your test script.

PSS: once we merge this PR, please update your test login script to the new one from this PR, because its currently disabled on your repo (hence why the script on the website is broken).

Please comment in the follow up kubeflow/manifests#2854. Note this does not yet contain your changes here.

thesuperzapper · 2024-08-22T15:45:12Z

Can @andreyvelich or @terrytangyuan please approve?

We need a root approver for this because we are also changing content from the Katib section (to fix broken links).

We have 2 LGTMs already, and given the very important changes (including fixes the KFP "out of cluster" script for 1.9.0), I think we should merge ASAP.

andreyvelich

/assign @kubeflow/wg-pipeline-leads @james-jwu @zijianjoy
Please check this PR, so we can merge it

andreyvelich · 2024-08-22T15:47:09Z

/cc @connor-mccarthy

james-jwu · 2024-08-27T15:42:06Z

/lgtm
/approve

google-oss-prow · 2024-08-27T15:42:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: james-jwu

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:

~~content/en/docs/components/katib/OWNERS~~ [james-jwu]
~~content/en/docs/components/pipelines/OWNERS~~ [james-jwu]

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

google-oss-prow bot requested review from chensun and hbelmiro July 7, 2024 01:44

google-oss-prow bot added the size/XXL label Jul 7, 2024

google-oss-prow bot assigned hbelmiro Jul 7, 2024

diegolovison reviewed Jul 8, 2024

View reviewed changes

rimolive reviewed Jul 8, 2024

View reviewed changes

content/en/docs/components/pipelines/user-guides/core-functions/cli.md Show resolved Hide resolved

rimolive reviewed Jul 8, 2024

View reviewed changes

content/en/docs/components/pipelines/user-guides/core-functions/connect-api.md Show resolved Hide resolved

rimolive reviewed Jul 8, 2024

View reviewed changes

content/en/docs/components/pipelines/user-guides/core-functions/platform-specific-features.md Show resolved Hide resolved

rimolive reviewed Jul 8, 2024

View reviewed changes

content/en/docs/components/pipelines/user-guides/migration.md Outdated Show resolved Hide resolved

hbelmiro reviewed Jul 9, 2024

View reviewed changes

thesuperzapper force-pushed the improve-kfp-v2-user-guides branch from 4ee4c51 to e740394 Compare August 19, 2024 18:20

thesuperzapper mentioned this pull request Aug 19, 2024

400: Bad request; unsupported request method. Can't connect the Pipelines SDK to Kubeflow Pipelines with Kind for Kubeflow 1.9 kubeflow/manifests#2844

Closed

7 tasks

thesuperzapper added 4 commits August 19, 2024 19:51

Improve the KFP / User Guides / Core Functions docs

2b5fda8

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

Small updates 1

e23f902

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

Fix KFPClientManager() for Kubeflow 1.9.0

9fd04b5

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

Fix checklinks errors

0264512

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

thesuperzapper force-pushed the improve-kfp-v2-user-guides branch from f0ddc5b to 0264512 Compare August 20, 2024 02:54

juliusvonkohout reviewed Aug 20, 2024

View reviewed changes

Make "Connect the SDK to the API" more clear

95d20ea

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

google-oss-prow bot assigned rimolive Aug 21, 2024

google-oss-prow bot added the lgtm label Aug 21, 2024

google-oss-prow bot assigned juliusvonkohout Aug 21, 2024

andreyvelich reviewed Aug 22, 2024

View reviewed changes

google-oss-prow bot requested a review from connor-mccarthy August 22, 2024 15:47

google-oss-prow bot assigned james-jwu Aug 27, 2024

google-oss-prow bot added the approved label Aug 27, 2024

google-oss-prow bot merged commit d3ca1b1 into kubeflow:master Aug 27, 2024
6 checks passed

thesuperzapper deleted the improve-kfp-v2-user-guides branch August 27, 2024 16:30


		In the following example, we use the `DockerRunner` type. Runner types are covered in more detail below.
		Local execution comes with several limitations:

	Local execution comes with several limitations:
	It comes with several limitations:


		In general, platform-specific plugin libraries provide functions that act on tasks similarly to [task-level configuration methods][task-level-config-methods] provided by the KFP SDK directly. Platform-specific plugin libraries may also provide pre-baked components.
		<!-- TODO: add docs on how to create a platform-specific authoring library -->

Improve the KFP / User Guides / Core Functions docs #3795

Improve the KFP / User Guides / Core Functions docs #3795

Conversation

thesuperzapper commented Jul 7, 2024 • edited Loading

thesuperzapper commented Jul 7, 2024

hbelmiro commented Jul 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegolovison commented Jul 8, 2024

Choose a reason for hiding this comment

rimolive commented Jul 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thesuperzapper commented Aug 19, 2024

thesuperzapper commented Aug 19, 2024

thesuperzapper commented Aug 19, 2024

Choose a reason for hiding this comment

thesuperzapper Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

juliusvonkohout commented Aug 20, 2024 • edited Loading

juliusvonkohout commented Aug 20, 2024 • edited Loading

juliusvonkohout commented Aug 20, 2024 • edited Loading

thesuperzapper commented Aug 20, 2024

thesuperzapper commented Aug 20, 2024

rimolive commented Aug 21, 2024

juliusvonkohout commented Aug 21, 2024 • edited Loading

thesuperzapper commented Aug 22, 2024

andreyvelich left a comment

Choose a reason for hiding this comment

andreyvelich commented Aug 22, 2024

james-jwu commented Aug 27, 2024

google-oss-prow bot commented Aug 27, 2024

thesuperzapper commented Jul 7, 2024 •

edited

Loading

thesuperzapper Aug 20, 2024 •

edited

Loading

juliusvonkohout commented Aug 20, 2024 •

edited

Loading

juliusvonkohout commented Aug 20, 2024 •

edited

Loading

juliusvonkohout commented Aug 20, 2024 •

edited

Loading

juliusvonkohout commented Aug 21, 2024 •

edited

Loading