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

Introduce RestUtils#getMasterNodeTimeout #107986

Conversation

DaveCTurner
Copy link
Contributor

Many APIs accept a ?master_timeout parameter, but reading this
parameter requires a little unnecessary boilerplate to specify the
literal parameter name and default value. Moreover, today's convention
is to construct a MasterNodeRequest and then read the default master
timeout from the freshly-created request. In practice this results in a
default of 30s, but we specify in the docs that this default is always
30s, and in principle one could create a transport request with a
different initial value which would deviate from the documented
behaviour.

This commit introduces a utility method for reading this parameter in a
fashion which is completely consistent with the documented behaviour.

Relates #107984

Many APIs accept a `?master_timeout` parameter, but reading this
parameter requires a little unnecessary boilerplate to specify the
literal parameter name and default value. Moreover, today's convention
is to construct a `MasterNodeRequest` and then read the default master
timeout from the freshly-created request. In practice this results in a
default of 30s, but we specify in the docs that this default is _always_
30s, and in principle one could create a transport request with a
different initial value which would deviate from the documented
behaviour.

This commit introduces a utility method for reading this parameter in a
fashion which is completely consistent with the documented behaviour.

Relates elastic#107984
@DaveCTurner DaveCTurner added >non-issue :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v8.15.0 labels Apr 28, 2024
@DaveCTurner DaveCTurner requested a review from ywangd April 28, 2024 10:53
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team label Apr 28, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@DaveCTurner DaveCTurner merged commit 30d31bf into elastic:main Apr 29, 2024
14 checks passed
@DaveCTurner DaveCTurner deleted the 2024/04/28/RestUtils-getMasterNodeTimeout branch April 29, 2024 07:03
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Nice refactor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >non-issue Team:Distributed Meta label for distributed team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants