Skip to content
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

Backport removal of runBlocking and Stats API fix #284

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

qreshi
Copy link
Contributor

@qreshi qreshi commented Jan 24, 2022

Issue #, if available:

Description of changes:
Backport #281 and #283 to 1.2

CheckList:
[x] Commits are signed per the DCO using --signoff

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.

* Remove runBlocking calls from MonitorRunner

Signed-off-by: Mohammad Qureshi <[email protected]>

* Add security tests with partial user permissions to check user context being correctly applied

Signed-off-by: Mohammad Qureshi <[email protected]>

* Fix ktlint style issues

Signed-off-by: Mohammad Qureshi <[email protected]>
…response (opensearch-project#283)

* Add back opendistro prefixed sweeper enabled to alerting stats response

Signed-off-by: Mohammad Qureshi <[email protected]>

* Rename stats response field variables used in tests

Signed-off-by: Mohammad Qureshi <[email protected]>
@qreshi qreshi force-pushed the run-blocking-and-stats-1.2 branch from 35a814f to 1c27c67 Compare January 25, 2022 21:47
@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2022

Codecov Report

Merging #284 (e53953f) into 1.2 (a4597de) will increase coverage by 0.17%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                1.2     #284      +/-   ##
============================================
+ Coverage     78.45%   78.63%   +0.17%     
- Complexity      211      214       +3     
============================================
  Files           173      173              
  Lines          6959     6964       +5     
  Branches        912      912              
============================================
+ Hits           5460     5476      +16     
+ Misses         1017     1003      -14     
- Partials        482      485       +3     
Impacted Files Coverage Δ
...in/kotlin/org/opensearch/alerting/MonitorRunner.kt 72.05% <100.00%> (ø)
...ing/core/action/node/ScheduledJobsStatsResponse.kt 65.38% <100.00%> (+1.38%) ⬆️
...pensearch/alerting/elasticapi/ElasticExtensions.kt 55.35% <100.00%> (+3.43%) ⬆️
...ain/kotlin/org/opensearch/alerting/AlertService.kt 78.53% <0.00%> (-0.49%) ⬇️
...e/action/node/ScheduledJobsStatsTransportAction.kt 75.51% <0.00%> (ø)
.../opensearch/alerting/core/schedule/JobScheduler.kt 76.38% <0.00%> (+6.94%) ⬆️
...rch/alerting/core/action/node/ScheduledJobStats.kt 66.66% <0.00%> (+11.11%) ⬆️
...arch/alerting/core/schedule/JobSchedulerMetrics.kt 72.22% <0.00%> (+16.66%) ⬆️

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 a4597de...e53953f. Read the comment docs.

@qreshi qreshi force-pushed the run-blocking-and-stats-1.2 branch 3 times, most recently from 8f3d2ae to dfd73ec Compare January 26, 2022 04:14
@qreshi
Copy link
Contributor Author

qreshi commented Jan 26, 2022

Judging from the GitHub Action run results from the original PR of this change on main, the security tests there were never actually run because it deemed that the security plugin couldn't be found (this was for the 1.3.0 Docker image, doesn't seem to be the case for the older versions since it was able to fail here).

Because of this, I ended up using this PR to debug the security tests written for this change since they weren't actually passing. I'm able to get a clean pass now but I'm going to force push this PR to not include the recent changes so I can submit the change as a follow-up PR from main -> 1.x -> etc.

@qreshi qreshi force-pushed the run-blocking-and-stats-1.2 branch from dfd73ec to 1c27c67 Compare January 26, 2022 18:40
@qreshi qreshi merged commit 19751c3 into opensearch-project:1.2 Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants