-
Notifications
You must be signed in to change notification settings - Fork 37
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 Get Workflow API to retrieve a stored template by workflow id #263
Conversation
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Is this going to conflict with feature branch? |
Overall looks good, had two comments though on the file naming. Originally when i wrote the status API my plan was that both this and get status will be under the GET mechanism because we are doing a GET operation on a workflow ID. AD does the same thing for GET detector and get detector profile. the status here is another api path param so it would be under the same rest action. I am okay doing it either way though and changing the getworkflow to getworkflowstatus and this as getworkflowtemplate. Additionally I originally had issues including the template in the get status API response because we couldn't read the user field. I was wondering if you the user field here gets read correctly? https://github.com/opensearch-project/flow-framework/blob/main/src/main/java/org/opensearch/flowframework/model/WorkflowState.java#L108 |
Signed-off-by: Joshua Palis <[email protected]>
@dbwiddis I dont think it should, the only common files that this modifies are the FlowFrameworkPlugin and the associated tests. I can add the backport featue/agen_framework label to this PR as well |
Signed-off-by: Joshua Palis <[email protected]>
Discussed offline with @amitgalitz . We've decided to instead just rename the Get Workflow State API implementation and keep this and the Get Workflow API separate (separate handlers, separate request response classes, separate transport action handlers). I have updated the The name |
Signed-off-by: Joshua Palis <[email protected]>
@dbwiddis with the Workflow State API name changes, this will conflict heavily with the feature branch, removing the backport |
I see we are returning a field:
based on your PR description, should we maybe not return this as its coming from our parsing of the workflow? or is this an optional field we currently accept in our workflow create request body. Additionally are we returning UI_metadata here if its present? |
@amitgalitz user_inputs is an optional field that we're accepting in our workflow create body, it should be returned. As for the UI metadata, this will be returned if it is present |
Closing this PR in favor of : #273 |
Description
Adds a Get Workflow API that retrieves a workflow from the Global Context index
Creating a workflow :
Using the API :
Issues Resolved
#170
Part of #88
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.