[Backport 2.x] Prevent message collection from being updated after message count has been received (#2180) #3035
Conversation
… been received (opensearch-project#2180) Also adds mechanism to detect if messages were missed so tests can be updated to appropriate counts. Signed-off-by: Peter Nied <[email protected]> (cherry picked from commit ba9d82e)
DarshitChanpura
requested review from
cliu123,
cwperks,
davidlago,
peternied,
RyanL1997,
stephen-crawford,
reta and
willyborankin
as code owners
|
Build is broken due to opensearch-project/OpenSearch#8782 |
|
Ci should be fixed once #3036 is merged. |
| } catch (final IOException e) { | ||
| throw ExceptionsHelper.convertToOpenSearchException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks weirdly formatted here, but that is not the case.
Codecov Report
@@ Coverage Diff @@
## 2.x #3035 +/- ##
============================================
- Coverage 62.31% 59.79% -2.53%
+ Complexity 3312 3204 -108
============================================
Files 264 264
Lines 19468 19471 +3
Branches 3326 3326
============================================
- Hits 12132 11642 -490
- Misses 5710 6215 +505
+ Partials 1626 1614 -12
|
|
Looks like there's still a failing test. I can help take a look. |
|
@DarshitChanpura FYI the assertion for the failing test was changed on main to only check for 1 message: https://github.com/opensearch-project/security/blob/main/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java#L145C47-L145C47 The test can be updated in this backport with the same assertion. Edit: The assertion for that changed was changed on this PR Second Edit: I had to set the expected number of messages to 4 to get this more stable. The assertion was previously at 4. There are retries happening that are making the test hard to follow - I'm seeing what is best now to make this the most stable. |
|
@DarshitChanpura I see why the assertion on main was set to I think we can disable automatic retries on 2.x and set the assertion in BasicAuditlogTests.testSSLPlainText to check for 1 message |
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Darshit Chanpura <[email protected]>
Backports ba9d82e from #2180
Manual backport is required because of:
Check List
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.