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

[SPEC] Change type of wait_for_active_shards from number to string #20186

Merged
merged 1 commit into from Aug 29, 2016

Conversation

gmarz
Copy link
Contributor

@gmarz gmarz commented Aug 26, 2016

Fixed all (most) instances where wait_for_active_shards was incorrectly typed as a string instead of number.

@gmarz gmarz added the :Core/Infra/REST API REST infrastructure and utilities label Aug 26, 2016
@abeyad
Copy link

abeyad commented Aug 26, 2016

@gmarz the original was correct, it is a "string" type because you can pass in to the query param all, default, or a string representation of an integer ("1", "2", etc) that will get parsed as a number. This type is a "string" to allow for all and default to be specified.

@clintongormley
Copy link

It looks like only rest-api-spec/src/main/resources/rest-api-spec/api/cluster.health.json needs to be changed to string

@abeyad
Copy link

abeyad commented Aug 29, 2016

@clintongormley actually that wait_for_active_shards in the cluster health APIs is different and is used only as an integer. So right now cluster.health.json is correct. But I do think we should consider changing it for consistency sake and also allow all and default (though in this case, default should be set to 0 instead of 1).

@gmarz
Copy link
Contributor Author

gmarz commented Aug 29, 2016

But I do think we should consider changing it for consistency sake and also allow all and default

+1, definitely agree,

@gmarz gmarz force-pushed the spec/wait_for_active_shards branch from 7ff55dc to 84f05cd Compare August 29, 2016 13:31
@gmarz
Copy link
Contributor Author

gmarz commented Aug 29, 2016

@abeyad I've revised this PR and changed the type for cluster.health.json only.

@clintongormley
Copy link

LGTM

@gmarz gmarz merged commit 2363c7d into elastic:master Aug 29, 2016
@gmarz gmarz deleted the spec/wait_for_active_shards branch August 29, 2016 14:19
@gmarz gmarz changed the title [SPEC] Fix type for wait_for_active_shards (string => number) [SPEC] Change type of wait_for_active_shards from number to string Aug 29, 2016
@abeyad
Copy link

abeyad commented Aug 29, 2016

I opened #20216 to discuss a proposal for the cluster health API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants