-
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
Adding multi node IT and update API bug fixes #416
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #416 +/- ##
============================================
+ Coverage 71.85% 71.90% +0.05%
- Complexity 614 622 +8
============================================
Files 79 79
Lines 3084 3136 +52
Branches 237 238 +1
============================================
+ Hits 2216 2255 +39
- Misses 763 774 +11
- Partials 105 107 +2 ☔ View full report in Codecov by Sentry. |
23c1897
to
df5cb26
Compare
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.
Initial pass
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.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.
LGTM with some naming nits, missing spaces and stray slashes.
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java
Outdated
Show resolved
Hide resolved
src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java
Outdated
Show resolved
Hide resolved
@dbwiddis @joshpalis @owaiskazi19 multi node tests might still be flaky here, but i think we should push and continue to work on fixing this with additional PRs. we should at least have this on every PR we submit |
803c6e9
to
c475303
Compare
Signed-off-by: Amit Galitzky <[email protected]>
Signed-off-by: Amit Galitzky <[email protected]>
Signed-off-by: Amit Galitzky <[email protected]>
c475303
to
e5f7fb7
Compare
* add multi node IT, fix update api bugs Signed-off-by: Amit Galitzky <[email protected]> * adding test and stashing context Signed-off-by: Amit Galitzky <[email protected]> * cleaning up comments and adding tests Signed-off-by: Amit Galitzky <[email protected]> --------- Signed-off-by: Amit Galitzky <[email protected]> (cherry picked from commit a6ac83a) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Adding multi node IT and update API bug fixes (#416) * add multi node IT, fix update api bugs * adding test and stashing context * cleaning up comments and adding tests --------- (cherry picked from commit a6ac83a) 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 multi node IT tests, removed deprovisioning from IT for now until other fixes merge.
Fixed 2 bugs for update API.
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.