-
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
Adds REST APIs for creating and provisioning a workflow #63
Conversation
…ending global context index, state index implementations Signed-off-by: Joshua Palis <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #63 +/- ##
============================================
- Coverage 81.74% 81.68% -0.07%
- Complexity 211 285 +74
============================================
Files 21 30 +9
Lines 838 1119 +281
Branches 96 125 +29
============================================
+ Hits 685 914 +229
- Misses 119 160 +41
- Partials 34 45 +11
|
src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Outdated
Show resolved
Hide resolved
…nt param default value, setting workflow request to read/write optional string Signed-off-by: Joshua Palis <[email protected]>
…, added FixedExecutorBuilder thread pool for provisioning tasks, set up async workflow execution, added TODOs for state/GC index handling Signed-off-by: Joshua Palis <[email protected]>
…estResponse unit tests Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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, would approve except with the TODOs not sure this is done.
src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java
Outdated
Show resolved
Hide resolved
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.
Need to discuss more on the APIs. This would also require integ tests, let's create an issue for the same
src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestProvisionWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
…class Signed-off-by: Joshua Palis <[email protected]>
…e components, added updateTemplate(), indexExists() methods to handler and createIndex step respecitvely. Implemented CreateWorkflow/ProvisionWorkflow transport actions Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…or added methods in CreateIndexStep, GlobalContextHandler Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…ion. Still need to add tests for success case Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…g field via string replacement Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
…ibility version fields from int to text Signed-off-by: Joshua Palis <[email protected]>
…versions are of type text, added GC template document readers/writers, modified tests. Still need to add test cases for the new readers/writers Signed-off-by: Joshua Palis <[email protected]>
…e instead of toXContent() Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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.
Almost there. Looks good overall.
src/main/java/org/opensearch/flowframework/indices/GlobalContextHandler.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/CreateIndexStep.java
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
…o a common TemplateUtil class Signed-off-by: Joshua Palis <[email protected]>
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! Thanks for addressing the comments patiently
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, please create an issue for the TODO if there isn't one.
src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
Show resolved
Hide resolved
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/opensearch-ai-flow-framework/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/opensearch-ai-flow-framework/backport-2.x
# Create a new branch
git switch --create backport/backport-63-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 014475d7e2968b723c916fb5c134d257f0b11578
# Push it to GitHub
git push --set-upstream origin backport/backport-63-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/opensearch-ai-flow-framework/backport-2.x Then, create a pull request where the |
Seems the backport PR failed to get raised, @owaiskazi19 ill create a manual PR |
…roject#63) * Inital implementation, set up rest/transport actions, registration, pending global context index, state index implementations Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, seting params to snake case, removing redundant param default value, setting workflow request to read/write optional string Signed-off-by: Joshua Palis <[email protected]> * Introducing getExecutorBuilders extension point to FlowFramworkPlugin, added FixedExecutorBuilder thread pool for provisioning tasks, set up async workflow execution, added TODOs for state/GC index handling Signed-off-by: Joshua Palis <[email protected]> * updating unit tests for FlowFrameworkPluginTests, adding WorkflowRequestResponse unit tests Signed-off-by: Joshua Palis <[email protected]> * Adding validate/toXContent tests for workflow request/responses Signed-off-by: Joshua Palis <[email protected]> * Adding unit tests for create and provision rest actions Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 1). Moving common vlaues to CommonValue class Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 2), adding globalcontexthandler to create components, added updateTemplate(), indexExists() methods to handler and createIndex step respecitvely. Implemented CreateWorkflow/ProvisionWorkflow transport actions Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 3) Signed-off-by: Joshua Palis <[email protected]> * Removing TODOs for RestAction constructors, adding basic unit tests for added methods in CreateIndexStep, GlobalContextHandler Signed-off-by: Joshua Palis <[email protected]> * Adding CreateWorkflowTransportAction unit tests Signed-off-by: Joshua Palis <[email protected]> * Adding intial failure test case for the ProvisionWorkflowTransportAction. Still need to add tests for success case Signed-off-by: Joshua Palis <[email protected]> * Updating base URI namespace to workflow instead of workflows Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comment, updating invalid template config test, removing field via string replacement Signed-off-by: Joshua Palis <[email protected]> * Add success test case for ProvisionWorkflowTransportAction Signed-off-by: Joshua Palis <[email protected]> * Updating global context index mapping for template version and compatibility version fields from int to text Signed-off-by: Joshua Palis <[email protected]> * Fixing bugs, changed GC index mapping so that template/compatibility versions are of type text, added GC template document readers/writers, modified tests. Still need to add test cases for the new readers/writers Signed-off-by: Joshua Palis <[email protected]> * Updating GlobalContextHandler.updateTemplate() to use toDocumentSource instead of toXContent() Signed-off-by: Joshua Palis <[email protected]> * Replacing exceptions with FlowFrameworException Signed-off-by: Joshua Palis <[email protected]> * Resolving javadoc warnings Signed-off-by: Joshua Palis <[email protected]> * Cleaning up TODOs Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, moving some common template parsing methods to a common TemplateUtil class Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]> (cherry picked from commit 014475d)
#63) (#96) Adds REST APIs for creating and provisioning a workflow (#63) * Inital implementation, set up rest/transport actions, registration, pending global context index, state index implementations Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, seting params to snake case, removing redundant param default value, setting workflow request to read/write optional string Signed-off-by: Joshua Palis <[email protected]> * Introducing getExecutorBuilders extension point to FlowFramworkPlugin, added FixedExecutorBuilder thread pool for provisioning tasks, set up async workflow execution, added TODOs for state/GC index handling Signed-off-by: Joshua Palis <[email protected]> * updating unit tests for FlowFrameworkPluginTests, adding WorkflowRequestResponse unit tests Signed-off-by: Joshua Palis <[email protected]> * Adding validate/toXContent tests for workflow request/responses Signed-off-by: Joshua Palis <[email protected]> * Adding unit tests for create and provision rest actions Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 1). Moving common vlaues to CommonValue class Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 2), adding globalcontexthandler to create components, added updateTemplate(), indexExists() methods to handler and createIndex step respecitvely. Implemented CreateWorkflow/ProvisionWorkflow transport actions Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments (Part 3) Signed-off-by: Joshua Palis <[email protected]> * Removing TODOs for RestAction constructors, adding basic unit tests for added methods in CreateIndexStep, GlobalContextHandler Signed-off-by: Joshua Palis <[email protected]> * Adding CreateWorkflowTransportAction unit tests Signed-off-by: Joshua Palis <[email protected]> * Adding intial failure test case for the ProvisionWorkflowTransportAction. Still need to add tests for success case Signed-off-by: Joshua Palis <[email protected]> * Updating base URI namespace to workflow instead of workflows Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comment, updating invalid template config test, removing field via string replacement Signed-off-by: Joshua Palis <[email protected]> * Add success test case for ProvisionWorkflowTransportAction Signed-off-by: Joshua Palis <[email protected]> * Updating global context index mapping for template version and compatibility version fields from int to text Signed-off-by: Joshua Palis <[email protected]> * Fixing bugs, changed GC index mapping so that template/compatibility versions are of type text, added GC template document readers/writers, modified tests. Still need to add test cases for the new readers/writers Signed-off-by: Joshua Palis <[email protected]> * Updating GlobalContextHandler.updateTemplate() to use toDocumentSource instead of toXContent() Signed-off-by: Joshua Palis <[email protected]> * Replacing exceptions with FlowFrameworException Signed-off-by: Joshua Palis <[email protected]> * Resolving javadoc warnings Signed-off-by: Joshua Palis <[email protected]> * Cleaning up TODOs Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments, moving some common template parsing methods to a common TemplateUtil class Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]> (cherry picked from commit 014475d)
Description
Initial implementation, set up rest/transport actions, registration, pending state index implementations
This PR exposes two new APIs to
CreateWorkflow
(save) and toProvisionWorkflow
(execute). More details on the URI, payloads can be found on issue #49 .Additionally, this PR integrates PR #65,
In this current state, the rest and transport actions to facilitate these requests have been set up. The
CreateWorkflow
API will be the main entry point in which the plugin's system indices are created (Global Context
andState
). Once created. theCreateWorkflow
API will parse a use case template from the request content and either save to or update an existing entry within theGlobal Context
index, returning the document ID as theworkflow_id
. Any insert into theGlobal Context
entry will create a correspondingState
index entry mapped with theworkflow_id
, defaulted to aNOT_STARTED
state. Additionally, any update to a use case template will default its state back toNOT_STARTED
.The
Provision
API is responsible for executing the provisioning workflow for a use case template stored in theGlobal Context
. Theworkflow_id
is returned back to the user for use in querying provisioning status and the provision workflow is executed asynchronously on a separate thread, updating the state entry on success or failure.Using test template (invalid due to self-connecting edge):
Create API :
OpenSearch logs successful Global Context Creation :
Provision API :
OpenSearch logs process sorting error due to self connecting edge :
Once the workflow steps are polished, we can proceed with full end to end testing of a valid use case template
Issues Resolved
#49
#61
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.