-
Notifications
You must be signed in to change notification settings - Fork 480
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
Add documentation for optional param for get workflow step API #6736
Add documentation for optional param for get workflow step API #6736
Conversation
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
6f3fc8c
to
4dc2a6d
Compare
@@ -30,18 +30,21 @@ OpenSearch validates workflows by using the validation template that lists the r | |||
} | |||
``` | |||
|
|||
The Get Workflow Steps API retrieves this file. | |||
The Get Workflow Steps API retrieves [this](https://github.com/opensearch-project/flow-framework/blob/2.x/src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java#L120-L229) enum. |
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.
The line numbers on this link may change in the future. It's also not very explanatory. Should be something like, "Returns a list of Workflow Steps including their required inputs, outputs, default timeout value, ...." etc.
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.
Changed the description. Also, added link of 2.13
release branch to avoid changes later.
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
@owaiskazi19 Thank you for providing the PR! I reviewed and pushed my edits. We don't usually direct users to GitHub code because this is user documentation. I removed the link for now because I think the steps are self-explanatory but if you think we should list the response fields, then we have to create a response fields table on this page and port all the fields with descriptions here. Let me know what you think. |
Looks good |
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.
@owaiskazi19 @kolchfa-aws Please see my comments and changes and let me know if you have any questions. Thanks!
{: .warning} | ||
|
||
OpenSearch validates workflows by using the validation template that lists the required inputs, generated outputs, and required plugins for all steps. For example, for the `register_remote_model` step, the validation template appears as follows: | ||
This API returns a list of workflow steps, including their required inputs, outputs, default timeout value, and required plugins For example, for the `register_remote_model` step, the Get Workflow Steps API returns the following information: |
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.
Should "timeout value" be "timeout values" (plural)?
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.
@kolchfa-aws @natebower we have just one timeout value.
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.
@owaiskazi19 I think it's fine to keep it plural since "steps" is plural. So there are timeouts for a number of steps (even though each has only one timeout), if that makes sense.
|
||
| Parameter | Data type | Description | | ||
| :--- | :--- | :--- | | ||
| `workflow_step` | String | The step name of the step to retrieve. Specify multiple step names as a comma-separated list. For example, `create_connector,delete_model,deploy_model`. | |
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 the first instance of "step" be removed? Ignore if essential to the meaning.
``` | ||
{% include copy-curl.html %} | ||
|
||
|
||
#### Example response | ||
|
||
OpenSearch responds with the validation template containing the steps. The order of fields in the returned steps may not exactly match the original JSON but will function identically. | ||
OpenSearch responds with the workflow steps. The order of fields in the returned steps may not exactly match the original JSON but will function identically. |
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.
Should a noun follow JSON?
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.
I don't think so, I think it's understood that JSON means the "document in JSON format".
Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: kolchfa-aws <105444904+kolchfa-aws@users.noreply.github.com>
Signed-off-by: Fanit Kolchina <kolchfa@amazon.com>
…-website into optional-param
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
…earch-project#6736) * Added documentaion for optional param for get workflow step API Signed-off-by: Owais Kazi <owaiskazi19@gmail.com> * Addressed PR comment Signed-off-by: Owais Kazi <owaiskazi19@gmail.com> * Doc review Signed-off-by: Fanit Kolchina <kolchfa@amazon.com> * Apply suggestions from code review Co-authored-by: Nathan Bower <nbower@amazon.com> Signed-off-by: kolchfa-aws <105444904+kolchfa-aws@users.noreply.github.com> --------- Signed-off-by: Owais Kazi <owaiskazi19@gmail.com> Signed-off-by: Fanit Kolchina <kolchfa@amazon.com> Signed-off-by: kolchfa-aws <105444904+kolchfa-aws@users.noreply.github.com> Co-authored-by: Fanit Kolchina <kolchfa@amazon.com> Co-authored-by: kolchfa-aws <105444904+kolchfa-aws@users.noreply.github.com> Co-authored-by: Nathan Bower <nbower@amazon.com>
Description
opensearch-project/flow-framework#538 added an optional
workflow_step
param to the fetch specific workflow steps in the workflow steps APIIssues Resolved
Part of opensearch-project/flow-framework#541
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.