Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Accept request parameters in RestGetRollupAction and fix flakey tests #353

Merged

Conversation

qreshi
Copy link
Contributor

@qreshi qreshi commented Dec 10, 2020

Issue #, if available:

Description of changes:

  • RestGetRollupAction was not taking in parameters to be passed into GetRollupsRequest, meaning only the default values could be used
  • Due to the issue above, the integration test for getting all rollups would sometimes fail if the total number of rollup jobs at the time of the test exceeded 20 as this was the default response size
  • Fixed a flakey test for the stop API that led to intermittent version_conflict_exceptions since the rollup job was being updated when being disabled in between the time that the stop API was retrieving and updating the document

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@qreshi qreshi added bug Something isn't working maintenance improves code quality, but not the product labels Dec 10, 2020
@codecov
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #353 (012b208) into master (e721e59) will increase coverage by 0.40%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #353      +/-   ##
============================================
+ Coverage     75.68%   76.08%   +0.40%     
- Complexity     1374     1387      +13     
============================================
  Files           183      183              
  Lines          7011     7022      +11     
  Branches       1143     1143              
============================================
+ Hits           5306     5343      +37     
+ Misses         1096     1052      -44     
- Partials        609      627      +18     
Impacted Files Coverage Δ Complexity Δ
...xmanagement/rollup/action/get/GetRollupsRequest.kt 100.00% <100.00%> (ø) 10.00 <1.00> (ø)
...nagement/rollup/resthandler/RestGetRollupAction.kt 95.65% <100.00%> (+3.98%) 4.00 <0.00> (ø)
...statemanagement/model/destination/CustomWebhook.kt 65.21% <0.00%> (-28.99%) 12.00% <0.00%> (-2.00%)
...indexmanagement/rollup/model/RollupFieldMapping.kt 83.33% <0.00%> (+8.33%) 7.00% <0.00%> (+1.00%)
...nt/indexstatemanagement/model/destination/Slack.kt 54.54% <0.00%> (+13.63%) 4.00% <0.00%> (+2.00%)
...xmanagement/rollup/actionfilter/FieldCapsFilter.kt 75.00% <0.00%> (+30.88%) 27.00% <0.00%> (+12.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e721e59...fd7cfc8. Read the comment docs.

@qreshi qreshi merged commit 5b40097 into opendistro-for-elasticsearch:master Dec 10, 2020
@qreshi qreshi deleted the rest-get-rollup-fix branch December 10, 2020 03:42
@qreshi qreshi removed the maintenance improves code quality, but not the product label Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants