-
Notifications
You must be signed in to change notification settings - Fork 777
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
Updated the Pipelines API reference docs #1424
Conversation
Hi @OfficePop. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign |
/assign @IronPan @IronPan Please would you have a look at this PR too, since you've been working with @OfficePop on the API reference. |
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.
Well done @OfficePop it's great to see the API docs receiving this attention! I've a few suggestions, some of which will need input from @IronPan.
</tr> | ||
<tr> | ||
<td><a href="#operation--apis-v1beta1-jobs--id--disable-post">POST /apis/v1beta1/jobs/{id}/disable</a></td> | ||
<td></td> | ||
<td><p>Disable a job.</p> |
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.
Can we add a bit more info here and below, about the significance of disabling or enabling a job?
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.
Is a kubeflow job analogous to a kubernetes job? I don't see anything describing them in the kubeflow documentation.
If they are the same, perhaps something like "Activate a job that performs a given series of runs."
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.
Updated as follows:
Disable - Stops a job and all its associated runs. The job is not deleted.
Enable - Restarts a job that was previously stopped. All runs associated with the job will continue.
@@ -300,29 +319,54 @@ <h3 id="tag-PipelineService" class="swagger-summary-tag">Tag: PipelineService</h | |||
</thead> | |||
<tbody> | |||
<tr> | |||
<td><a href="#operation--apis-v1beta1-pipelines-get">GET /apis/v1beta1/pipelines</a></td> | |||
<td><a href="#operation--apis-v1beta1-pipeline_versions-get">GET /apis/v1beta1/pipeline_versions</a></td> |
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.
Yaayyy pipeline versions! I believe this is a new feature. @IronPan do you know which version of Kubeflow will include Pipelines versions?
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.
it ideally will be the next kf release
</tr> | ||
<tr> | ||
<td><a href="#operation--apis-v1beta1-pipelines--id--templates-get">GET /apis/v1beta1/pipelines/{id}/templates</a></td> | ||
<td><p>Get a YAML template for the selected pipeline.</p> |
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.
Can we give a bit of information about what a template is? Also, does this end point list all templates rather than returning a specific one?
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.
Updated as follows:
"Returns a single YAML template that contains the description, parameters, and metadata associated with the pipeline provided."
Tried to describe what is in it since I am not sure what you would actually use the template for. My guess might be portability for custom pipelines between different KF instances (Or other ML platforms?) but I can't verify that just now.
<div class="json-inner-schema"> | ||
|
||
</div> | ||
</dd> | ||
<dt data-property-name="description"> | ||
<span class=" |
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.
s/id/ID
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.
Also, how about replacing "not empty" with "present" in both cases?
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.
Added for the second comment, unsure what is intended by the first.
content/docs/pipelines/reference/api/kubeflow-pipeline-api-spec.html
Outdated
Show resolved
Hide resolved
@@ -300,29 +319,54 @@ <h3 id="tag-PipelineService" class="swagger-summary-tag">Tag: PipelineService</h | |||
</thead> | |||
<tbody> | |||
<tr> | |||
<td><a href="#operation--apis-v1beta1-pipelines-get">GET /apis/v1beta1/pipelines</a></td> | |||
<td><a href="#operation--apis-v1beta1-pipeline_versions-get">GET /apis/v1beta1/pipeline_versions</a></td> |
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.
it ideally will be the next kf release
Re-generated API spec to account for update to API version, also included other requested changes from PR.
Thanks for all your work on this @OfficePop! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sarahmaddox 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 |
Numerous updates to the swagger documentation for the Pipeline API spec. I am responsible for adding descriptions to the endpoints and filter.proto links per issue 1257 but there have also been several new endpoints added to the pipeline service tag (no descriptions for those yet, will include in a future PR).
Not sure why the version number is regressed in line 177, but that's consistent with the current swagger def in the pipelines repo.
This change is