-
Notifications
You must be signed in to change notification settings - Fork 36
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
Remove useCase and defaultParams field in WorkflowRequest #952
Conversation
Signed-off-by: Junwei Dai <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #952 +/- ##
=========================================
Coverage 78.07% 78.08%
+ Complexity 996 993 -3
=========================================
Files 99 99
Lines 4625 4617 -8
Branches 431 431
=========================================
- Hits 3611 3605 -6
+ Misses 834 832 -2
Partials 180 180 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Junwei Dai <[email protected]>
Signed-off-by: Junwei Dai <[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, but not sure if the removed tests are completely duplicated somewhere else or provide at least one interesting change worth testing (even if not the new params)
src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java
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.
Also removed the constructor that has never been used in our code base
Although this comment seems to be the "new thing" that was tested so probably good to remove.
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-952-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 219aec5c1b85b8aa3e5e15cdf0029c13666ae37e
# Push it to GitHub
git push --set-upstream origin backport/backport-952-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/flow-framework/backport-2.x Then, create a pull request where the |
…-project#952) * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Removed the constructor that has never been used and tested. Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 219aec5)
…-project#952) * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Removed the constructor that has never been used and tested. Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 219aec5)
…-project#952) * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Removed the constructor that has never been used and tested. Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 219aec5)
…est (#952) (#954) * Remove useCase and defaultParams field in WorkflowRequest (#952) * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Removed the constructor that has never been used and tested. Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 219aec5) * Remove useCase and defaultParams field in WorkflowRequest (#952) * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Remove useCase and defaultParams field in WorkflowRequest Signed-off-by: Junwei Dai <[email protected]> * Removed the constructor that has never been used and tested. Signed-off-by: Junwei Dai <[email protected]> --------- Signed-off-by: Junwei Dai <[email protected]> Co-authored-by: Junwei Dai <[email protected]> (cherry picked from commit 219aec5) --------- Signed-off-by: Junwei Dai <[email protected]>
Description
Removed
useCase
AnddefaultParams
since the WorkflowRequest class doesn't serialize the use case and default parameters over transport.Also removed the constructor that has never been used in our code base
Related Issues
Resolves #758
Check List
[ ] New functionality has been documented.[ ] API changes companion pull request created.--signoff
.[ ] Public documentation issue/PR created.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.