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

Allow the deprecation warning of the REST API request parameter 'master_timeout' for the requests sent by High Level REST Client #2912

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Apr 15, 2022

Description

The REST API request parameter 'master_timeout' is deprecated in issue #2511.
The PR allows the specific deprecation warning when Requests are sent by High Level REST Client.

In High Level REST Client, parameter 'master_timeout' is added to every applicable REST API call,
see org.opensearch.client.RequestConverters.Params.withMasterTimeout(TimeValue).
To keep the compatibility of Rest Client 2.x with server 1.x, the parameter 'master_timeout' is preserved.
The deprecated parameter is not actively used by the user, so allow the deprecation warning by default.

  • Allow the deprecation warning of the REST API request parameter 'master_timeout' for the requests sent by High Level REST Client.
  • Add unit test
  • Make the variable of the deprecation message public to be used in different packages.

Issues Resolved

A part of #2511

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.

@tlfeng tlfeng requested a review from a team as a code owner April 15, 2022 09:15
…eter 'master_timeout' to HTTP response header

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng force-pushed the skip-master-timeout-warning-header branch from c7fa494 to 5f5bcba Compare April 15, 2022 09:18
Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng added v2.0.0 Version 2.0.0 backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch labels Apr 15, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure c7fa494c5e30ddeea632c658a7eab8daa2d72fce
Log 4506

Reports 4506

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e9f775d
Log 4507

Reports 4507

@tlfeng tlfeng marked this pull request as draft April 15, 2022 12:01
Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng force-pushed the skip-master-timeout-warning-header branch from 309e20f to b0d588e Compare April 15, 2022 12:19
@tlfeng tlfeng changed the title Skip adding deprecation warning message of the REST API request parameter 'master_timeout' to HTTP response header Allow the deprecation warning of the REST API request parameter 'master_timeout' for the requests sent by High Level REST Client Apr 15, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 309e20fbfab796c1f0aceea515a959888bf8c2f5
Log 4508

Reports 4508

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure b0d588e
Log 4509

Reports 4509

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e3d14d2
Log 4511

Reports 4511

@tlfeng tlfeng removed backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch labels Apr 15, 2022
@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 15, 2022

Only allow the deprecation warning in HLRC has got a defect that a deprecation warning will still be logged in file of the server (logs directory). It will confuse the user, because they didn't use the parameter actively. The problem will be addressed in PR #2920 instead.

@tlfeng tlfeng closed this Apr 15, 2022
@tlfeng tlfeng deleted the skip-master-timeout-warning-header branch April 26, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants