Add synchronous execution option to workflow provisioning#990
Merged
dbwiddis merged 17 commits intoopensearch-project:mainfrom Jan 16, 2025
Merged
Add synchronous execution option to workflow provisioning#990dbwiddis merged 17 commits intoopensearch-project:mainfrom
dbwiddis merged 17 commits intoopensearch-project:mainfrom
Conversation
dbwiddis
reviewed
Jan 9, 2025
Member
dbwiddis
left a comment
There was a problem hiding this comment.
Generally looks good.
- You need to handle -1 time value; my recommendation is you use that for the default "async" rather than null
- You need to do stream version checks for the new (optional) workflow state in the response, and the new timeout parameter in the workflow request (unless you want to just keep it in the params map).
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
src/main/java/org/opensearch/flowframework/transport/WorkflowResponse.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java
Outdated
Show resolved
Hide resolved
dbwiddis
reviewed
Jan 9, 2025
src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
dbwiddis
reviewed
Jan 13, 2025
src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java
Outdated
Show resolved
Hide resolved
added 13 commits
January 15, 2025 10:53
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com> # Conflicts: # src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com> # Conflicts: # src/test/java/org/opensearch/flowframework/workflow/DeleteConnectorStepTests.java
Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
d6c0c53 to
18a1dbb
Compare
dbwiddis
approved these changes
Jan 15, 2025
Member
dbwiddis
left a comment
There was a problem hiding this comment.
LGTM with a few suggestions.
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/util/WorkflowTimeoutUtility.java
Show resolved
Hide resolved
added 2 commits
January 15, 2025 12:16
…, update error message Signed-off-by: Junwei Dai <junweid@amazon.com>
Signed-off-by: Junwei Dai <junweid@amazon.com>
joshpalis
reviewed
Jan 15, 2025
Member
joshpalis
left a comment
There was a problem hiding this comment.
Overall looks good to me, great work implementing this feature @junweid62 . A few comments
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/WorkflowResponse.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Junwei Dai <junweid@amazon.com>
joshpalis
approved these changes
Jan 16, 2025
Contributor
|
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/flow-framework/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/flow-framework/backport-2.x
# Create a new branch
git switch --create backport/backport-990-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 33579a35a9d7a72cc0a2d44561b98bd64a79dd04
# Push it to GitHub
git push --set-upstream origin backport/backport-990-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/flow-framework/backport-2.xThen, create a pull request where the |
Member
|
@junweid62 can you please manually backport this? |
5 tasks
junweid62
added a commit
to junweid62/flow-framework
that referenced
this pull request
Jan 17, 2025
…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <junweid@amazon.com> * code refactor Signed-off-by: Junwei Dai <junweid@amazon.com> * add change log Signed-off-by: Junwei Dai <junweid@amazon.com> * refactor code based on comment Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <junweid@amazon.com> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <junweid@amazon.com> * remove unsued common value Signed-off-by: Junwei Dai <junweid@amazon.com> * add reprovision sync execution Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * addressed some comments Signed-off-by: Junwei Dai <junweid@amazon.com> --------- Signed-off-by: Junwei Dai <junweid@amazon.com> Co-authored-by: Junwei Dai <junweid@amazon.com> (cherry picked from commit 33579a3)
junweid62
added a commit
to junweid62/flow-framework
that referenced
this pull request
Jan 17, 2025
…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <junweid@amazon.com> * code refactor Signed-off-by: Junwei Dai <junweid@amazon.com> * add change log Signed-off-by: Junwei Dai <junweid@amazon.com> * refactor code based on comment Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <junweid@amazon.com> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <junweid@amazon.com> * remove unsued common value Signed-off-by: Junwei Dai <junweid@amazon.com> * add reprovision sync execution Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * addressed some comments Signed-off-by: Junwei Dai <junweid@amazon.com> --------- Signed-off-by: Junwei Dai <junweid@amazon.com> Co-authored-by: Junwei Dai <junweid@amazon.com>
dbwiddis
pushed a commit
that referenced
this pull request
Jan 22, 2025
…ing(#990) (#1009) Add synchronous execution option to workflow provisioning (#990) * Add synchronous execution option to workflow provisioning * code refactor * add change log * refactor code based on comment * fix spotless check * Limit workflow timeout to a range of 1 to 300 seconds * Limit workflow timeout to a range of 1 to 300 seconds * Limit workflow timeout to non-negative * Add synchronous execution to reprovision * remove unsued common value * add reprovision sync execution * fix test for WorkflowTimeoutUtilityTests * fix test name for WorkflowTimeoutUtilityTests * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message * fix spotless check * addressed some comments --------- Signed-off-by: Junwei Dai <junweid@amazon.com> Co-authored-by: Junwei Dai <junweid@amazon.com>
junweid62
added a commit
to junweid62/flow-framework
that referenced
this pull request
Jan 23, 2025
…-project#990) * Add synchronous execution option to workflow provisioning Signed-off-by: Junwei Dai <junweid@amazon.com> * code refactor Signed-off-by: Junwei Dai <junweid@amazon.com> * add change log Signed-off-by: Junwei Dai <junweid@amazon.com> * refactor code based on comment Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to a range of 1 to 300 seconds Signed-off-by: Junwei Dai <junweid@amazon.com> * Limit workflow timeout to non-negative Signed-off-by: Junwei Dai <junweid@amazon.com> * Add synchronous execution to reprovision Signed-off-by: Junwei Dai <junweid@amazon.com> * remove unsued common value Signed-off-by: Junwei Dai <junweid@amazon.com> * add reprovision sync execution Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * fix test name for WorkflowTimeoutUtilityTests Signed-off-by: Junwei Dai <junweid@amazon.com> * Add comments to explain AtomicBoolean usage in WorkflowTimeoutUtility, update error message Signed-off-by: Junwei Dai <junweid@amazon.com> * fix spotless check Signed-off-by: Junwei Dai <junweid@amazon.com> * addressed some comments Signed-off-by: Junwei Dai <junweid@amazon.com> --------- Signed-off-by: Junwei Dai <junweid@amazon.com> Co-authored-by: Junwei Dai <junweid@amazon.com> Signed-off-by: Junwei Dai <junweid@amazon.com>
4 tasks
Merged
1 task
1 task
This was referenced Jan 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jan 14 Revision
Added synchronous execution option to reprovision
Description
This PR introduces a new
wait_for_completion_timeoutfeature to the Provision Workflow API in the OpenSearch Flow Framework. The feature allows users to control whether the API call waits for the entire workflow provisioning process to complete before returning a response.What’s Changed:
Success Response:
TimeOut Response:
Areas of Concern:
I have a few parts of the implementation that I believe can be further improved, particularly in ProvisionWorkflowTransportAction. Some of the logic feels a bit verbose and might not be the most efficient way to handle the timeout and synchronous execution. I’d appreciate the feedback from reviewers.
Related Issues
Resolves #967
Check List
--signoff.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.