-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Restructure samples #1710
Restructure samples #1710
Conversation
Can you look at #1342 ? It's been pending for a lot of time. |
Besides reviewing that particular PR, could you review this PR? |
/lgtm |
/lgtm |
/approve |
Since this PR is large, consider adding a more verbose description to the PR so future readers can see the reasons for this change. |
Thanks for doing this work. I'd group the DSL samples together (e.g. in a DSL directory). |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777, neuromage 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 |
As the samples grow bigger, we need a hierarchical structure for all these samples under each of the categories. |
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
As discussed offline, we might consider moving the primitive samples like retry or volume one to core/ instead of contrib/ folder. |
should we also move following? basically the sample demonstrate the native functionalities should live in core/ and vendor specific live in contrib/ |
You are correct that these samples will be in the core directory in the future. However, I don't think it is a good idea to move the samples to the core when they are not covered by the sample tests. |
Moved these samples as well. |
/lgtm |
This PR aims to organize the samples into core set and contrib set.
Core samples: demonstrates the full KFP functionalities and covered by the sample test infra.
A selected set of these core samples will also be preloaded to the KFP during deployment and the tests for these samples will block the PRs. The core samples will also include intermediate samples that are more complex than basic samples such as flip coins but simpler than TFX samples. It serves to demonstrate a set of the outstanding features and offers users the next level KFP experience.
Contrib samples: these contrib samples are not tested by KFP and could potentially be moved the core samples if tests are covered and it demonstrates certain KFP functionality that fit. Another reason to put some samples in this directory is that some samples require certain platform support that is hard to support in our test infra.
Structure:
In the Core directory, each sample will be in a separate directory. In the Contrib directory, there is an intermediate directory for each contributor, e.g. ibm and arena, within which each sample is in a separate directory. An example of the resulting structure is as follows:
This change is