-
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
Fixing model group parsing and restoring context #695
Fixing model group parsing and restoring context #695
Conversation
Signed-off-by: Amit Galitzky <[email protected]>
looking at failing tests |
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 context restores LGTM.
Potential NPE you need to address, though.
src/main/java/org/opensearch/flowframework/workflow/RegisterModelGroupStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Show resolved
Hide resolved
What is the type of the ( |
Signed-off-by: Amit Galitzky <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #695 +/- ##
============================================
- Coverage 73.78% 73.74% -0.04%
Complexity 722 722
============================================
Files 82 82
Lines 3662 3668 +6
Branches 312 315 +3
============================================
+ Hits 2702 2705 +3
- Misses 818 820 +2
- Partials 142 143 +1 ☔ View full report in Codecov by Sentry. |
src/main/java/org/opensearch/flowframework/workflow/RegisterModelGroupStep.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Amit Galitzky <[email protected]>
* adding context.restore when missed and fixing model group parsing Signed-off-by: Amit Galitzky <[email protected]> * adding change log, fix casting Signed-off-by: Amit Galitzky <[email protected]> * changed to contains key Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]> (cherry picked from commit 28ea3dd) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* adding context.restore when missed and fixing model group parsing Signed-off-by: Amit Galitzky <[email protected]> * adding change log, fix casting Signed-off-by: Amit Galitzky <[email protected]> * changed to contains key Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]> (cherry picked from commit 28ea3dd) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixing model group parsing and restoring context (#695) * adding context.restore when missed and fixing model group parsing * adding change log, fix casting * changed to contains key --------- (cherry picked from commit 28ea3dd) Signed-off-by: Amit Galitzky <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Fixing model group parsing and restoring context (#695) * adding context.restore when missed and fixing model group parsing * adding change log, fix casting * changed to contains key --------- (cherry picked from commit 28ea3dd) Signed-off-by: Amit Galitzky <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Adding restore.context() in multiple place it isn't restored after stashing.
Fixing model group access_mode parsing, this was always failing but never tested with private on security enabled domain
Issues Resolved
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.