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

Fix and include unauthenticated user web server test #273

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Fix and include unauthenticated user web server test #273

merged 1 commit into from
Jan 23, 2023

Conversation

Tjofil
Copy link
Contributor

@Tjofil Tjofil commented Jan 19, 2023

Signed-off-by: Filip Drobnjakovic drobnjakovicfilip@gmail.com

Is your feature request related to a problem? Please provide an existing Issue # , or describe.
Issue #167

Describe the solution you are proposing
Implementation of the proposed solution from the issue (#167) comment (by me). Please raise any additional concerns.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • 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: Filip Drobnjakovic <drobnjakovicfilip@gmail.com>
@Tjofil Tjofil requested a review from a team January 19, 2023 16:29
@kkhatua kkhatua requested review from kiranprakash154, sgup432, sruti1312, kaushalmahi12 and khushbr and removed request for a team January 19, 2023 17:06
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2023

Codecov Report

Merging #273 (3a216c9) into main (e0d6a0c) will increase coverage by 0.35%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #273      +/-   ##
============================================
+ Coverage     71.57%   71.92%   +0.35%     
- Complexity     2989     2997       +8     
============================================
  Files           380      380              
  Lines         18984    18984              
  Branches       1463     1463              
============================================
+ Hits          13587    13654      +67     
+ Misses         4803     4727      -76     
- Partials        594      603       +9     
Impacted Files Coverage Δ
...nceanalyzer/rca/samplers/MetricsDBFileSampler.java 75.00% <0.00%> (-9.10%) ⬇️
...nalyzer/rca/net/handler/PublishRequestHandler.java 85.71% <0.00%> (-2.05%) ⬇️
...nsearch/performanceanalyzer/rca/RcaController.java 79.29% <0.00%> (-1.33%) ⬇️
...ceanalyzer/rca/store/rca/hotshard/HotShardRca.java 86.13% <0.00%> (-1.00%) ⬇️
...performanceanalyzer/collectors/StatsCollector.java 92.59% <0.00%> (-0.75%) ⬇️
...rch/performanceanalyzer/config/PluginSettings.java 30.17% <0.00%> (+1.18%) ⬆️
...ensearch/performanceanalyzer/hwnet/NetworkE2E.java 79.27% <0.00%> (+1.80%) ⬆️
.../org/opensearch/performanceanalyzer/core/Util.java 70.83% <0.00%> (+8.33%) ⬆️
...formanceanalyzer/PerformanceAnalyzerWebServer.java 68.65% <0.00%> (+52.23%) ⬆️
...ensearch/performanceanalyzer/CertificateUtils.java 84.74% <0.00%> (+61.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kkhatua kkhatua self-requested a review January 20, 2023 02:04
Copy link
Member

@kkhatua kkhatua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find on the JDK version change causing the handshake behaviour. Thanks for the fix !

@Tjofil Tjofil closed this by deleting the head repository Jan 23, 2023
@Tjofil
Copy link
Contributor Author

Tjofil commented Jan 23, 2023

Accidentally closed. Reopening.

@Tjofil Tjofil reopened this Jan 23, 2023
@kiranprakash154 kiranprakash154 merged commit b74944a into opensearch-project:main Jan 23, 2023
@kiranprakash154
Copy link
Contributor

kiranprakash154 commented Jan 24, 2023

Hi @Tjofil, Thanks for the fix !
can you backport this to older versions of OS ?
(cc: @kkhatua)

@Tjofil
Copy link
Contributor Author

Tjofil commented Jan 24, 2023

Hey @kiranprakash154, I have no label assignment access on github to invoke the automatic bot backport PR, shall I do it by hand then ? And which branches beside 2.x should I backport to ?

Edit: Did it manually 270 and 273 for 2.x

Tjofil added a commit to Tjofil/performance-analyzer-rca that referenced this pull request Jan 24, 2023
Signed-off-by: Filip Drobnjakovic <drobnjakovicfilip@gmail.com>
Tjofil added a commit to Tjofil/performance-analyzer-rca that referenced this pull request Jan 24, 2023
Signed-off-by: Filip Drobnjakovic <drobnjakovicfilip@gmail.com>
@Tjofil Tjofil mentioned this pull request Jan 24, 2023
5 tasks
khushbr pushed a commit that referenced this pull request Jan 26, 2023
Signed-off-by: Filip Drobnjakovic <drobnjakovicfilip@gmail.com>

Signed-off-by: Filip Drobnjakovic <drobnjakovicfilip@gmail.com>
@opensearch-trigger-bot
Copy link

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-273-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b74944a27a6028daacdb1951af941991d5f090c5
# Push it to GitHub
git push --set-upstream origin backport/backport-273-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-273-to-1.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants