search

kubeflow / website

Kubeflow's public website
Creative Commons Attribution 4.0 International
145 stars 751 forks source link

Restructured Kubeflow Pipelines docs #3737

Closed hbelmiro closed 1 week ago

hbelmiro commented 1 month ago

Resolves https://github.com/kubeflow/website/issues/3716.

This PR:

cc @StefanoFioravanzo @diegolovison @milosjava

StefanoFioravanzo commented 1 month ago

@hbelmiro Awesome work!

I have just a few comments:

Feature Status Type
feat A Missing User guide
feat A Partial Reference
feat B Done User guide
feat B Missing Reference
... ... ...

WDYT?

hbelmiro commented 1 month ago

/hold for review

review-notebook-app[bot] commented 1 month ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

hbelmiro commented 1 month ago

@StefanoFioravanzo

  • Let's rename How-to/User guides to User guides

Done.

  • Thanks for moving content to Build a More Advanced ML Pipeline. I think it's a good temporary measure. For longer term, it seems like this is not exactly a "how to guide", but rather an "Advanced" quickstart. Each bullet in this guide could reference to its own how to guide, while this example serves as an advanced tutorial/quickstart. If you agree, we can have a separation issue describing the desired end state.
  • Now that we have a solid structure in place we should start building a "feature map" to understand what are the features that need documentation, the ones that are already documented but need improvement, and the ones that are green. I can see us doing this with a table like this

Sounds good. I created two issues for that:

Should we add them (and new ones that may arise) to https://github.com/kubeflow/website/issues/3712 even if they may not be completed on time for 1.9?

hbelmiro commented 1 month ago

/unhold ready for review

hbelmiro commented 1 month ago

@thesuperzapper @rimolive @HumairAK @milosjava Can you guys take another look?

hbelmiro commented 1 month ago

@thesuperzapper I addressed all your suggestions. Thank you.

rimolive commented 1 month ago

May I ask one more thing? This setion should be part of the v2 docs too if we can to make v2 the default structure.

https://www.kubeflow.org/docs/components/pipelines/v1/concepts/

hbelmiro commented 1 month ago

May I ask one more thing? This setion should be part of the v2 docs too if we can to make v2 the default structure.

https://www.kubeflow.org/docs/components/pipelines/v1/concepts/

@thesuperzapper suggested the same and I forgot that one. Thank you @rimolive.

hbelmiro commented 1 month ago

May I ask one more thing? This setion should be part of the v2 docs too if we can to make v2 the default structure.

https://www.kubeflow.org/docs/components/pipelines/v1/concepts/

Done @rimolive.

rimolive commented 4 weeks ago

/lgtm

hbelmiro commented 4 weeks ago

@thesuperzapper

I think a few more pages can be moved out of the legacy section because they also apply to V2 (and having them in V2 will encourage people to update them):

* `/pipelines/legacy-v1/reference/component-spec/` -> `/pipelines/reference/component-spec/`

  * The component spec is still the same, but we have no reference to it on the V2 section.
  * We can put it under the "Reference" section of V2.

* `/pipelines/legacy-v1/overview/interfaces/` -> `/pipelines/interfaces/`

  * It's nice to have an overview of the interfaces of KFP.
  * We can put it directly under "Getting Started" at the top level.

* `/pipelines/legacy-v1/overview/multi-user/` -> `/pipelines/operator-guides/multi-user/`

  * This will need some small updates, but is still mostly correct for v2.
  * We can put it under the "operator guides" section.

* `/pipelines/legacy-v1/overview/pipeline-root/` -> `/pipelines/concepts/pipeline-root/`

  * This is a very important concept (which is actually exclusive to V2).
  * I think it makes sense to put it under "Concepts", probably directly under "Pipeline", in the second position.

Moved, except /pipelines/legacy-v1/overview/multi-user/, which I created an issue to move in a follow PR.

There are also a few pages in the "Legacy (v1)" section which will need updating but are critical to move/copy (we should probably do them in a separate PR).

Here is the list of "Legacy (v1)" pages which we need to move to top of "user guides":

* `Introduction to the Pipelines SDK`

  * Once updated, I think this one can go at the top of the "User Guides" section.

* `Install the Kubeflow Pipelines SDK`

  * THis is relly just explaining how to pip install `kfp` and use a conda env, we can update it and put it second under the "User Guides" section.

Here is the list of "Legacy (v1)" pages which we need to move to the "user guides" under "core functions":

* `Pipeline Parameters`

* `Visualize Results in the Pipelines UI`

  * This ones is critical, because it currently includes V2 docs under a separate heading, despite living in the V1 section.

Here is the list of "Legacy (v1)" pages which we need to move to the "user guides" under "create components":

* `Building Components`

* `Best Practices for Designing Components`

Let's do that in a follow PR too. Issue: https://github.com/kubeflow/website/issues/3750.

hbelmiro commented 4 weeks ago

@thesuperzapper

@hbelmiro I am pretty much happy with this now, just a small number of comments left.

We also need to create an issue about populating the "Install Kubeflow Pipelines" page rather than redirecting to the V1 docs.

We already have one.

Also, I see a random diff in content/en/docs/images/kubeflow-getting-started-diagram.svg, which is not part of the KFP docs, so please remove it.

Removed. Thank you.

hbelmiro commented 3 weeks ago

@StefanoFioravanzo @rimolive @milosjava @thesuperzapper @HumairAK I think this is ready to get merged. Can you guys take a final look?

rimolive commented 3 weeks ago

I agree. Since we had a recent issue with the DCO plugin, can you force push to make it check again?

hbelmiro commented 3 weeks ago

All green @rimolive.

rimolive commented 3 weeks ago

Thank you!

cc @StefanoFioravanzo @milosjava @thesuperzapper @HumairAK

HumairAK commented 3 weeks ago

I agree with you @thesuperzapper, but at the risk of this pr going on ad infinitum, do you feel it is at a point where we can merge this and create follow up issues to address?

I will make a separate PR to add detailed instructions on how to leverage pipeline root and path prefixes once the UI portion is merged in.

hbelmiro commented 3 weeks ago

I can't comment on them, because you are not actually updating the file, but I will list the links here:

* `[Kubeflow Pipelines SDK v2](https://www.kubeflow.org/docs/components/pipelines/sdk-v2/)` just remove this link (keep the text only)

* `[Kubeflow Pipelines deployment guide](/docs/components/pipelines/installation/)` should be updated to `/docs/components/pipelines/operator-guides/installation/`

* `[building pipelines](https://www.kubeflow.org/docs/components/pipelines/sdk-v2/build-pipeline/#build-your-pipeline)` should be replaced with `/docs/components/pipelines/user-guides/core-functions/build-advanced-pipeline/`

@thesuperzapper I updated the link to Kubeflow Pipelines deployment guide and created an issue for sdk-v2, since it will require more than just remove the link.

I will make a separate PR to add detailed instructions on how to leverage pipeline root and path prefixes once the UI portion is merged in.

@HumairAK I created an issue for that.

rimolive commented 3 weeks ago

@hbelmiro We talked in the last relese meeting that we should create follow-up issues for the missing changes in this PR and merge as-is. Is there a reason you marked the PR as draft?

thesuperzapper commented 2 weeks ago

@rimolive as I said in https://github.com/kubeflow/website/pull/3737#pullrequestreview-2108440509, we can merge once we fix the 404 errors, and @hbelmiro seems to be working on in his latest commits.

We also need to make a tracking issue for all the other issues we raised during the creation of this PR.

hbelmiro commented 2 weeks ago

@rimolive yeah, I was just fixing the 404s. This PR introduced them and if we had merged it with them we would be actually making the docs worse. Anyway, they are all fixed now. Tomorrow I'll create a new epic to track all the latest issues we found.

rimolive commented 2 weeks ago

@hbelmiro Open the follow-up issues and let's merge this PR. Currently this work is silo'd in your PR and only in you, having the other issues opened, we can span to multiple contributors and then work faster on restructuring the pipeline docs. You did a great job, but also we are 3 weeks away from final release and we need to start moving faster to meet the final release date.

hbelmiro commented 2 weeks ago

Ok @rimolive, let's get this merged. I added the issues to https://github.com/kubeflow/website/issues/3712.

@thesuperzapper what are you using to check 404s?

thesuperzapper commented 2 weeks ago

@rimolive First I really want to stress that 404s are NOT something we can fix later, having a 404 effectively gives away the SEO for that page. Google will de-index any page that becomes a 404, however, a redirect will give all of that SEO to whatever the new page is.

Second, it's by the nature of significant refactors that they happen all at the same time (in one PR, with one person taking charge), It's just not possible to do any other way. I understand that it blocks other updates to the content, but that was the decision the pipelines people made when they decided to refactor this section again.

@hbelmiro the redirect testing can be done by going to the demo version of the site for this PR, and manually pasting the target of the redirect after the site URL.

Note, even if there aren't any pages that reference a specific redirect, it doesn't mean that that redirect isn't used by an external website (e.g. Google or other docs) pointing at the original page.

Also, for those 2 wildcard redirects, it's just safer if we redirect them to the specific root pages for the sections that they redirect to (E.g. redirecting the concepts overview page rather than using :splat)

rimolive commented 2 weeks ago

/lgtm

thesuperzapper commented 2 weeks ago

@hbelmiro once you fix the redirects I listed in https://github.com/kubeflow/website/pull/3737#pullrequestreview-2111822549, we can merge.

StefanoFioravanzo commented 2 weeks ago

@hbelmiro do you think you can address these redirects quickly to unblock this PR? I have never looked into how this indexing works, but I trust what Mathew is saying.

In the context of unblocking subsequent work and merging this one as quickly as possible, please let us know if you can look in to adding the redirect soon, or if you need any help doing that

hbelmiro commented 2 weeks ago

@hbelmiro do you think you can address these redirects quickly to unblock this PR? I have never looked into how this indexing works, but I trust what Mathew is saying.

In the context of unblocking subsequent work and merging this one as quickly as possible, please let us know if you can look in to adding the redirect soon, or if you need any help doing that

@StefanoFioravanzo I can do it today.

hbelmiro commented 2 weeks ago

@rimolive @StefanoFioravanzo @thesuperzapper 404s from https://github.com/kubeflow/website/pull/3737#pullrequestreview-2111822549 fixed.

rimolive commented 2 weeks ago

@thesuperzapper @StefanoFioravanzo Can you please verify if there are other 404s remaining?

hbelmiro commented 2 weeks ago

These we'll fix in a follow PR:

thesuperzapper commented 2 weeks ago

@hbelmiro it was very hard to review because the ordering was random, so I sorted them alphabetically and went through each one to ensure its source and target were correct and that no pages were missing.

Here is the fixed copy, please update the PR to replace the redirects you added to end of the _redirects file as follows: (NOTE: leave the redirects you added in the previous sections, as they are still needed).

# Restructured the pipeline docs (https://github.com/kubeflow/website/issues/3716)
/docs/components/pipelines/overview/quickstart/                             /docs/components/pipelines/overview/
/docs/components/pipelines/v1/                                              /docs/components/pipelines/legacy-v1/
/docs/components/pipelines/v1/concepts/                                     /docs/components/pipelines/concepts/
/docs/components/pipelines/v1/concepts/component/                           /docs/components/pipelines/concepts/component/
/docs/components/pipelines/v1/concepts/experiment/                          /docs/components/pipelines/concepts/experiment/
/docs/components/pipelines/v1/concepts/graph/                               /docs/components/pipelines/concepts/graph/
/docs/components/pipelines/v1/concepts/metadata/                            /docs/components/pipelines/concepts/metadata/
/docs/components/pipelines/v1/concepts/output-artifact/                     /docs/components/pipelines/concepts/output-artifact/
/docs/components/pipelines/v1/concepts/pipeline/                            /docs/components/pipelines/concepts/pipeline/
/docs/components/pipelines/v1/concepts/run-trigger/                         /docs/components/pipelines/concepts/run-trigger/
/docs/components/pipelines/v1/concepts/run/                                 /docs/components/pipelines/concepts/run/
/docs/components/pipelines/v1/concepts/step/                                /docs/components/pipelines/concepts/step/
/docs/components/pipelines/v1/installation/                                 /docs/components/pipelines/legacy-v1/installation/
/docs/components/pipelines/v1/installation/choose-executor/                 /docs/components/pipelines/legacy-v1/installation/choose-executor/
/docs/components/pipelines/v1/installation/compatibility-matrix/            /docs/components/pipelines/legacy-v1/installation/compatibility-matrix/
/docs/components/pipelines/v1/installation/localcluster-deployment/         /docs/components/pipelines/legacy-v1/installation/localcluster-deployment/
/docs/components/pipelines/v1/installation/overview/                        /docs/components/pipelines/legacy-v1/installation/overview/
/docs/components/pipelines/v1/installation/standalone-deployment/           /docs/components/pipelines/legacy-v1/installation/standalone-deployment/
/docs/components/pipelines/v1/installation/upgrade/                         /docs/components/pipelines/legacy-v1/installation/upgrade/
/docs/components/pipelines/v1/introduction/                                 /docs/components/pipelines/legacy-v1/introduction/
/docs/components/pipelines/v1/overview/                                     /docs/components/pipelines/legacy-v1/overview/
/docs/components/pipelines/v1/overview/caching/                             /docs/components/pipelines/legacy-v1/overview/caching/
/docs/components/pipelines/v1/overview/interfaces/                          /docs/components/pipelines/interfaces/
/docs/components/pipelines/v1/overview/multi-user/                          /docs/components/pipelines/legacy-v1/overview/multi-user/
/docs/components/pipelines/v1/overview/pipeline-root/                       /docs/components/pipelines/concepts/pipeline-root/
/docs/components/pipelines/v1/overview/quickstart/                          /docs/components/pipelines/legacy-v1/overview/quickstart/
/docs/components/pipelines/v1/reference/                                    /docs/components/pipelines/legacy-v1/reference/
/docs/components/pipelines/v1/reference/api/kubeflow-pipeline-api-spec/     /docs/components/pipelines/legacy-v1/reference/api/kubeflow-pipeline-api-spec/
/docs/components/pipelines/v1/reference/component-spec/                     /docs/components/pipelines/reference/component-spec/
/docs/components/pipelines/v1/reference/sdk/                                /docs/components/pipelines/legacy-v1/reference/sdk/
/docs/components/pipelines/v1/sdk/                                          /docs/components/pipelines/legacy-v1/sdk/
/docs/components/pipelines/v1/sdk/best-practices/                           /docs/components/pipelines/legacy-v1/sdk/best-practices/
/docs/components/pipelines/v1/sdk/build-pipeline/                           /docs/components/pipelines/legacy-v1/sdk/build-pipeline/
/docs/components/pipelines/v1/sdk/component-development/                    /docs/components/pipelines/legacy-v1/sdk/component-development/
/docs/components/pipelines/v1/sdk/connect-api/                              /docs/components/pipelines/user-guides/core-functions/connect-api/
/docs/components/pipelines/v1/sdk/dsl-recursion/                            /docs/components/pipelines/legacy-v1/sdk/dsl-recursion/
/docs/components/pipelines/v1/sdk/enviroment_variables/                     /docs/components/pipelines/legacy-v1/sdk/enviroment_variables/
/docs/components/pipelines/v1/sdk/gcp/                                      /docs/components/pipelines/legacy-v1/sdk/gcp/
/docs/components/pipelines/v1/sdk/install-sdk/                              /docs/components/pipelines/legacy-v1/sdk/install-sdk/
/docs/components/pipelines/v1/sdk/manipulate-resources/                     /docs/components/pipelines/legacy-v1/sdk/manipulate-resources/
/docs/components/pipelines/v1/sdk/output-viewer/                            /docs/components/pipelines/legacy-v1/sdk/output-viewer/
/docs/components/pipelines/v1/sdk/parameters/                               /docs/components/pipelines/legacy-v1/sdk/parameters/
/docs/components/pipelines/v1/sdk/pipelines-with-tekton/                    /docs/components/pipelines/legacy-v1/sdk/pipelines-with-tekton/
/docs/components/pipelines/v1/sdk/python-based-visualizations/              /docs/components/pipelines/legacy-v1/sdk/python-based-visualizations/
/docs/components/pipelines/v1/sdk/python-function-components/               /docs/components/pipelines/legacy-v1/sdk/python-function-components/
/docs/components/pipelines/v1/sdk/sdk-overview/                             /docs/components/pipelines/legacy-v1/sdk/sdk-overview/
/docs/components/pipelines/v1/sdk/static-type-checking/                     /docs/components/pipelines/legacy-v1/sdk/static-type-checking/
/docs/components/pipelines/v1/troubleshooting/                              /docs/components/pipelines/legacy-v1/troubleshooting/
/docs/components/pipelines/v1/tutorials/                                    /docs/components/pipelines/legacy-v1/tutorials/
/docs/components/pipelines/v1/tutorials/api-pipelines/                      /docs/components/pipelines/legacy-v1/tutorials/api-pipelines/
/docs/components/pipelines/v1/tutorials/benchmark-examples/                 /docs/components/pipelines/legacy-v1/tutorials/benchmark-examples/
/docs/components/pipelines/v1/tutorials/build-pipeline/                     /docs/components/pipelines/legacy-v1/tutorials/build-pipeline/
/docs/components/pipelines/v1/tutorials/cloud-tutorials/                    /docs/components/pipelines/legacy-v1/tutorials/cloud-tutorials/
/docs/components/pipelines/v1/tutorials/sdk-examples/                       /docs/components/pipelines/legacy-v1/tutorials/sdk-examples/
/docs/components/pipelines/v2/                                              /docs/components/pipelines/
/docs/components/pipelines/v2/administration/                               /docs/components/pipelines/operator-guides/
/docs/components/pipelines/v2/administration/server-config/                 /docs/components/pipelines/operator-guides/server-config/
/docs/components/pipelines/v2/caching/                                      /docs/components/pipelines/user-guides/core-functions/caching/
/docs/components/pipelines/v2/cli/                                          /docs/components/pipelines/user-guides/core-functions/cli/
/docs/components/pipelines/v2/community-and-support/                        /docs/components/pipelines/reference/community-and-support/
/docs/components/pipelines/v2/compile-a-pipeline/                           /docs/components/pipelines/user-guides/core-functions/compile-a-pipeline/
/docs/components/pipelines/v2/components/                                   /docs/components/pipelines/user-guides/components/
/docs/components/pipelines/v2/components/additional-functionality/          /docs/components/pipelines/user-guides/components/additional-functionality/
/docs/components/pipelines/v2/components/container-components/              /docs/components/pipelines/user-guides/components/container-components/
/docs/components/pipelines/v2/components/containerized-python-components/   /docs/components/pipelines/user-guides/components/containerized-python-components/
/docs/components/pipelines/v2/components/importer-component/                /docs/components/pipelines/user-guides/components/importer-component/
/docs/components/pipelines/v2/components/lightweight-python-components/     /docs/components/pipelines/user-guides/components/lightweight-python-components/
/docs/components/pipelines/v2/data-types/                                   /docs/components/pipelines/user-guides/data-handling/data-types/
/docs/components/pipelines/v2/data-types/artifacts/                         /docs/components/pipelines/user-guides/data-handling/artifacts/
/docs/components/pipelines/v2/data-types/parameters/                        /docs/components/pipelines/user-guides/data-handling/parameters/
/docs/components/pipelines/v2/hello-world/                                  /docs/components/pipelines/getting-started/
/docs/components/pipelines/v2/installation/                                 /docs/components/pipelines/operator-guides/installation/
/docs/components/pipelines/v2/installation/quickstart/                      /docs/components/pipelines/operator-guides/installation/
/docs/components/pipelines/v2/introduction/                                 /docs/components/pipelines/overview/
/docs/components/pipelines/v2/load-and-share-components/                    /docs/components/pipelines/user-guides/components/load-and-share-components/
/docs/components/pipelines/v2/local-execution/                              /docs/components/pipelines/user-guides/core-functions/execute-kfp-pipelines-locally/
/docs/components/pipelines/v2/migration/                                    /docs/components/pipelines/user-guides/migration/
/docs/components/pipelines/v2/pipelines/                                    /docs/components/pipelines/user-guides/core-functions/
/docs/components/pipelines/v2/pipelines/control-flow/                       /docs/components/pipelines/user-guides/core-functions/control-flow/
/docs/components/pipelines/v2/pipelines/pipeline-basics/                    /docs/components/pipelines/user-guides/components/compose-components-into-pipelines/
/docs/components/pipelines/v2/platform-specific-features/                   /docs/components/pipelines/user-guides/core-functions/platform-specific-features/
/docs/components/pipelines/v2/reference/                                    /docs/components/pipelines/reference/
/docs/components/pipelines/v2/reference/api/kubeflow-pipeline-api-spec/     /docs/components/pipelines/reference/api/kubeflow-pipeline-api-spec/
/docs/components/pipelines/v2/reference/sdk/                                /docs/components/pipelines/reference/sdk/
/docs/components/pipelines/v2/run-a-pipeline/                               /docs/components/pipelines/user-guides/core-functions/run-a-pipeline/
/docs/components/pipelines/v2/version-compatibility/                        /docs/components/pipelines/reference/version-compatibility/

After that I can LGTM.

hbelmiro commented 2 weeks ago

@thesuperzapper done.

thesuperzapper commented 2 weeks ago

@hbelmiro thanks for your work on this, I think its ok to merge now.

/lgtm

Note, we will still get 404 for all the pre-v1/v2 split pages, but since we didn't want to make a new "component" path (e.g. rename the root prefix from /docs/components/pipelines/, there is no avoiding this.

thesuperzapper commented 2 weeks ago

@james-jwu or @zijianjoy can you please approve this significant refactor of the Kubeflow Pipelines docs if you agree?

We need a root approver because we are updating the redirects.

zijianjoy commented 2 weeks ago

It contains a huge change in website architecture, including @connor-mccarthy who originally introduced the current structure to take a look.

hbelmiro commented 2 weeks ago

@chensun @connor-mccarthy @zijianjoy This week's meeting has been canceled. We can't wait until the next one to have this merged. We have several other issues planned for the 1.9 KF release that depend on this PR. Can we get this merged or do you have any other suggestions?

connor-mccarthy commented 2 weeks ago

My 2c: with the removal of the quickstart page and the new structure, some very basic usage patterns (e.g., define and compile a component) are deeply nested on the documentation site. I would consider restoring a quickstart guide.

A typical and critical reason for reviewing the docs is to determine whether to use Pipelines or some other ML orchestration system. A key input to this choice is often the SDK authoring experience. With the current restructuring, "Run a pipeline" is quite deep in the tree.

image

I wont block on this, but want to raise it for consideration before merging.

thesuperzapper commented 2 weeks ago

@connor-mccarthy the "Getting Started" guide actually gives an example of running a pipeline: https://deploy-preview-3737--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/getting-started/

But it probably needs to be updated for "Kubeflow Platform" deployments, which need special steps to submit pipelines from outside the cluster, probably by linking to the "Connect the Pipelines SDK to Kubeflow Pipelines" guide: https://deploy-preview-3737--competent-brattain-de2d6d.netlify.app/docs/components/pipelines/user-guides/core-functions/connect-api/

However, I agree that we should link users from the "Getting Started" page to the new "User Guides" section, and point them at the most important topics in a logical order that "reads like a story" for new users.

hbelmiro commented 1 week ago

@connor-mccarthy @thesuperzapper Thank you for the suggestions. I created a new issue and added it to the follow-up tasks we need to address.

StefanoFioravanzo commented 1 week ago

I agree on those points, especially providing links in the Getting Started section. As Helber mentioned, this is something we can work on right after merging this PR. @connor-mccarthy It's important we proceed quickly so that we can start working independently on several improvements, including what you suggested, in time for the Kubeflow 1.9 release.

If you don't have other concerns, could you approve? We will prfioritize addressing your concerns right after.

connor-mccarthy commented 1 week ago

/lgtm /approve

Thanks everyone for the effort and discussion on this!

hbelmiro commented 1 week ago

@zijianjoy we still need your approval to get this merged.

james-jwu commented 1 week ago

James Liu is OOO. I will approve on his behalf. /lgtm /approve

google-oss-prow[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: connor-mccarthy, james-jwu, milosjava

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: - ~~[OWNERS](https://github.com/kubeflow/website/blob/master/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