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

Remove unnecessary stashContext usage in AbstractApiAction #4668

Closed

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Aug 21, 2024

Description

In anticipation of asking plugins to remove direct usages of threadContext.stashContext, I am looking at the Security repos usages. I found this usage inside AbstractApiAction that reads from the ThreadContext, stashes the context and then repopulates the context with original values. The stashContext appears to be unnecessary because it goes back to repopulating with original values.

I'm opening a PR to discuss the removal of this usage.

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)

Maintenance

Issues Resolved

Related to opensearch-project/opensearch-plugins#238

Testing

CI Checks

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • 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.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.23%. Comparing base (d76bbfb) to head (dbc0dd9).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4668      +/-   ##
==========================================
- Coverage   65.29%   65.23%   -0.06%     
==========================================
  Files         317      317              
  Lines       22312    22312              
  Branches     3589     3591       +2     
==========================================
- Hits        14569    14556      -13     
- Misses       5951     5957       +6     
- Partials     1792     1799       +7     
Files Coverage Δ
...arch/security/dlic/rest/api/AbstractApiAction.java 82.40% <ø> (-0.55%) ⬇️

... and 6 files with indirect coverage changes

@cwperks cwperks closed this Aug 22, 2024
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.

2 participants