[issue 486]Adding Request parameter 'master_timeout' is missing in the page of 'CAT Segments' API #522

ashutoshcipher · 2022-04-24T00:00:27Z

Description

Adding Request parameter 'master_timeout' is missing in the page of 'CAT Segments' API

Issues Resolved

issue 486 Adding Request parameter 'master_timeout' is missing in the page of 'CAT Segments' API

Check List

- 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.

Naarcha-AWS · 2022-04-25T17:21:06Z

Hey @ashutoshcipher. We recently added a DCO check that requires you to sign off your commits. For information on how to fix your commit, refer to this page.

…CAT Segments' API Signed-off-by: Ashutosh Gupta <ashugpt@amazon.com>

ashutoshcipher · 2022-04-25T18:25:05Z

Hey @ashutoshcipher. We recently added a DCO check that requires you to sign off your commits. For information on how to fix your commit, refer to this page.

@Naarcha-AWS - Thanks for reviewing. I have fixed signed off the commit as mentioned in doc.

alicejw1

LGTM

dblock · 2022-04-25T19:05:19Z

_opensearch/rest-api/cat/cat-segments.md

@@ -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.


This was changed in 2.x to a inclusive naming and I believe deprecated, @tlfeng care to point to the change?

@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:

Change master_timeout to cluster_manager_timeout in this line.

Add 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.

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.

Ah, make sense, I forgot the current docs are for version 1.x. 👍

I actually don't know if that's the case, I see a mix of things. @aetter maybe can help?

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).

@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.

@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.

dblock

I think units should be explained.

dblock · 2022-04-25T20:28:26Z

_opensearch/rest-api/cat/cat-segments.md

@@ -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.


What is the unit for this setting? Is it in seconds or do I need to specify the units like for bytes? The doc should clarify.

Good advice 😄.
The unit has to be specified, such as (1h stands for 1 hour, 2m stands for 2 minutes). But probably there should be an additional page to explain the units, and put a hyperlink near here.

@tlfeng and @dblock: Since your suggestion covers more than this instance of the master_timeout, would you be ok with us filing an issue to modify the master_timeout description to explain that units need to be specified?

Yeah, I don't think the Time unit have to be explained in this Pull Request, because it will be a long story.

Inclusive naming covered in a seperate PR.

Naarcha-AWS · 2022-05-25T20:44:40Z

_opensearch/rest-api/cat/cat-segments.md

@@ -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.


@ashutoshcipher: Could you change master_timeout to cluster_manager_timeout?

I will do that

alicejw1 · 2022-06-17T23:43:17Z

We have an updated PR here with the cluster_manager_timeout updated.
this PR we can archive with the info in the thread?
thanks

ashutoshcipher requested a review from a team as a code owner April 24, 2022 00:00

Adding Request parameter 'master_timeout' is missing in the page of '…

7461333

…CAT Segments' API Signed-off-by: Ashutosh Gupta <ashugpt@amazon.com>

ashutoshcipher force-pushed the BUG-486 branch from 9f5f5cc to 7461333 Compare April 25, 2022 18:22

Naarcha-AWS approved these changes Apr 25, 2022

View reviewed changes

alicejw1 self-requested a review April 25, 2022 18:34

alicejw1 approved these changes Apr 25, 2022

View reviewed changes

dblock reviewed Apr 25, 2022

View reviewed changes

dblock previously requested changes Apr 25, 2022

View reviewed changes

ashutoshcipher requested review from tlfeng and dblock April 29, 2022 01:04

Naarcha-AWS requested changes May 25, 2022

View reviewed changes

Naarcha-AWS self-assigned this May 25, 2022

Naarcha-AWS added the v2.0.0 label May 25, 2022

hdhalter mentioned this pull request Jun 17, 2022

small fix: update with 'cluster manager' terminology #690

Closed

alicejw1 mentioned this pull request Jun 17, 2022

for cluster manager term update #691

Merged

alicejw1 closed this Jun 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[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

ashutoshcipher commented Apr 24, 2022

Naarcha-AWS commented Apr 25, 2022

ashutoshcipher commented Apr 25, 2022

alicejw1 left a comment

dblock Apr 25, 2022

tlfeng Apr 25, 2022

dblock Apr 25, 2022

tlfeng Apr 25, 2022

dblock Apr 25, 2022

alicejw1 Apr 25, 2022 •

edited

Loading

Naarcha-AWS Apr 25, 2022 •

edited

Loading

tlfeng Apr 26, 2022

dblock left a comment

dblock Apr 25, 2022

tlfeng Apr 25, 2022

Naarcha-AWS Apr 25, 2022

tlfeng Apr 26, 2022

Naarcha-AWS May 25, 2022

ashutoshcipher May 26, 2022

alicejw1 commented Jun 17, 2022

[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

Conversation

ashutoshcipher commented Apr 24, 2022

Description

Issues Resolved

Check List

Naarcha-AWS commented Apr 25, 2022

ashutoshcipher commented Apr 25, 2022

alicejw1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alicejw1 Apr 25, 2022 • edited Loading

Choose a reason for hiding this comment

Naarcha-AWS Apr 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alicejw1 commented Jun 17, 2022

alicejw1 Apr 25, 2022 •

edited

Loading

Naarcha-AWS Apr 25, 2022 •

edited

Loading