-
Notifications
You must be signed in to change notification settings - Fork 918
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
[Deprecation] Deprecate master_timeout in favor of cluster_manager_timeout #1788
Conversation
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.
Looking good! Two things for further investigation:
- I'm suspicious of changes from
"osd"
to"kb"
- I want to make sure we understand/document the removal of
"include_type_name"
or{type}
as a param. I'm guessing this was another change to the js client since the last time these were updated? But worth confirming and including in the commit info.
"osd", | ||
"kb", |
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.
My guess is that these are regressions? Naively, I'd think osd
is the correct value.
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 got replaced on running the spec generation script with latest OpenSearch "main" branch. Is there a way I can verify the correct value?
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.
kb
is the right value. When we forked and did renaming we just went ahead and just renamed a lot of references. This kb
is related to bytes
so it we seem we had an existing bug.
@@ -1,10 +1,10 @@ | |||
{ | |||
"indices.create": { | |||
"url_params": { | |||
"include_type_name": "__flag__", |
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 change may be fine, but seems unrelated to the other changes.
fd64e78
to
9cd2e3d
Compare
@@ -5,7 +5,7 @@ | |||
"bytes": [ | |||
"b", | |||
"k", | |||
"osd", |
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.
Ha, whoopsie this happened in our renaming when we turned kb
to osd
I think this is a good change though. |
…meout As part of the meta issue opensearch-project/OpenSearch#2589 to track the plan and progress of applying inclusive naming across OpenSearch Repositories. Issue - opensearch-project#1685 Signed-off-by: Manasvini B Suryanarayana <[email protected]>
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.
Sounds like all these changes are good - I'd just recommend also including the non-naming changes in the commit message just so it's obvious they were intentional.
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.
LGTM! Will mention what @joshuarrrr recommended on merge.
…meout (#1788) As part of the meta issue opensearch-project/OpenSearch#2589 to track the plan and progress of applying inclusive naming across OpenSearch Repositories. Includes updates from generation with the removal of type. Issues: #1685 #1290 Signed-off-by: Manasvini B Suryanarayana <[email protected]> (cherry picked from commit c659831)
…meout (#1788) (#1794) As part of the meta issue opensearch-project/OpenSearch#2589 to track the plan and progress of applying inclusive naming across OpenSearch Repositories. Includes updates from generation with the removal of type. Issues: #1685 #1290 Signed-off-by: Manasvini B Suryanarayana <[email protected]> (cherry picked from commit c659831) Co-authored-by: Manasvini B Suryanarayana <[email protected]>
Signed-off-by: manasvinibs [email protected]
Description
As part of the meta issue opensearch-project/OpenSearch#2589 to track the plan and progress of applying inclusive naming across OpenSearch Repositories.
In this change we are generating the json files for the _cat API 's specs by running following sample cmd:
node scripts/spec_to_console.js -g "../OpenSearch/rest-api-spec/src/main/resources/rest-api-spec/api/cat.cluster_manager.json" -d "src/plugins/console/server/lib/spec_definitions/json/generated"
Issues Resolved
#1685
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr