-
Notifications
You must be signed in to change notification settings - Fork 525
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
[issue 486]Adding Request parameter 'master_timeout' is missing in the page of 'CAT Segments' API #522
[issue 486]Adding Request parameter 'master_timeout' is missing in the page of 'CAT Segments' API #522
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,7 @@ In addition to the [common URL parameters]({{site.url}}{{site.baseurl}}/opensear | |
Parameter | Type | Description | ||
:--- | :--- | :--- | ||
bytes | Byte size | Specify the units for byte size. For example, `7kb` or `6gb`. For more information, see [Supported units]({{site.url}}{{site.baseurl}}/opensearch/units/).. | ||
master_timeout | Time | The amount of time to wait for a connection to the master node. Default is 30 seconds. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the unit for this setting? Is it in seconds or do I need to specify the units like for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good advice 😄. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I don't think the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ashutoshcipher: Could you change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will do that |
||
|
||
|
||
## Response | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was changed in 2.x to a inclusive naming and I believe deprecated, @tlfeng care to point to the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock Yes, the situation and plan for the parameter
master_timeout
is described in the issue opensearch-project/OpenSearch#2928 .To summary,
In version >=2.0 & <3.0: A new parameter
cluster_manager_timeout
is added to perform the same function.In version >=3.0 & <4.0:
master_timeout
will be deprecated, and using it will get a deprecation warning notice.In version >= 4.0,
master_timeout
will be unsupported.I think the modification of the document can be:
master_timeout
tocluster_manager_timeout
in this line.master_timeout
as a alternative name for this parameter, but mark it as deprecated. (Although no deprecation warning will be shown in version 2.0, but we can still mark it as deprecated in the doc to encourage using the inclusive name.)@Naarcha-AWS @alicejw-aws May need your guide on how to deal with the change of the parameter name. We need a unified format to describe the name deprecation, because
master_timeout
parameter is used in 60 REST APIs, and we have to apply the parameter name change to many other pages.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can deal with various master deprecations separately. Let's merge this change since current docs are for 1.x, and follow general guidance for this for 2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, make sense, I forgot the current docs are for version 1.x. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually don't know if that's the case, I see a mix of things. @aetter maybe can help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tlfeng,
I'll flag this to get updated with the others when we publish for 2.0.
We will also explain the nomenclature change overall in the 2.0 release notes to announce the deprecated parameters throughout the APIs (master->cluster manager).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dblock: Andrew is no longer working for OpenSearch. The current version of the docs is 1.3, so we are ahead to merge this change now before we make a new branch for 2.0.
I'm with @tlfeng suggestion to handle master nomenclature changes as separate issues as we identify them, particularly when it comes to the deprecation of specific parameters in favor of new ones. As @alicejw-aws described, we do have a PR open for 2.0 that address the master nomenclature change (https://github.com/opensearch-project/documentation-website/pull/513/files). This includes that
master_timeout
parameter. We'll be merging this change in once we have a 2.0.0 RC (Release Candidate) branch up, which should be sometime this week.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alicejw-aws and @Naarcha-AWS Thanks for your reply! 😁 I got your plan for the nomenclature change in the doc. Then this PR looks good.
Please let me know when you composing the release note about the breaking change, in case there are anything missed about the REST API and Setting usages with new name. I will also add more detail in the issue #450 for the nomenclature change in these days.