Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Jan 11, 2017

There are some parameters that are accepted by each and every api we expose. Those (pretty, source, error_trace and filter_path) are not explicitly listed in the spec of every api, rather whitelisted in clients test runners so that they are always accepted. The human flag has been treated up until now as a parameter that's accepted by only some stats and info api, but that doesn't reflect reality as es core treats it exactly like pretty (relevant especially now that we validate params and throw exception when we find one that is not supported). Furthermore, the human flag has effect on every api that outputs a date, time, percentage or byte size field. For instance the tasks api outputs a date field although they don't have the human flag explicitly listed in their spec. There are other similar cases. This commit removes the human flag from the rest spec and makes it an always accepted query_string param.

There are some parameters that are accepted by each and every api we expose. Those (pretty, source, error_trace and filter_path)  are not explicitly listed in the spec of every api, rather whitelisted in clients test runners so that they are always accepted. The `human` flag has been treated up until now as a parameter that's accepted by only some stats and info api, but that doesn't reflect reality as es core treats it exactly like `pretty` (relevant especially now that we validate params and throw exception when we find one that is not supported). Furthermore, the human flag has effect on every api that outputs a date, time, percentage or byte size field. For instance the tasks api outputs a date field although they don't have the human flag explicitly listed in their spec. There are other similar cases. This commit removes the human flag from the rest spec and makes it an always accepted query_string param.
@javanna
Copy link
Member Author

javanna commented Jan 11, 2017

@elastic/es-clients are we all good with this change?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Makes sense to me though this should probably wait on es-clients so it doesn't break them.

@javanna
Copy link
Member Author

javanna commented Jan 11, 2017

agreed, everybody has to upgrade their list of always accepted query_string params. When everyone has done that, we can safely push I think.

@polyfractal
Copy link
Contributor

++ on my end

@gmarz
Copy link
Contributor

gmarz commented Jan 11, 2017

++ good on our end as well. We'll need to make a minor adjustment in the .NET client, but it won't break us.

Related, it's probably a good idea to include these global params somewhere in the spec. I opened #11638 a while back, perhaps now is a good time to revisit it.

@karmi
Copy link
Contributor

karmi commented Jan 12, 2017

I already include the human URL parameter in a list of global ones.

Makes sense to me though this should probably wait on es-clients so it doesn't break them.

I think in the past we have agreed that core can (and sometimes must) move faster, so breaking the integrations is not a problem per se. We have only asked for a notification (like this one), so we know what's gonna happen.

@clintongormley
Copy link
Contributor

I think in the past we have agreed that core can (and sometimes must) move faster, so breaking the integrations is not a problem per se. We have only asked for a notification (like this one), so we know what's gonna happen.

+1 Go for it

@javanna javanna merged commit 7674de9 into elastic:master Jan 12, 2017
javanna added a commit to javanna/elasticsearch that referenced this pull request Jan 12, 2017
)

There are some parameters that are accepted by each and every api we expose. Those (pretty, source, error_trace and filter_path)  are not explicitly listed in the spec of every api, rather whitelisted in clients test runners so that they are always accepted. The `human` flag has been treated up until now as a parameter that's accepted by only some stats and info api, but that doesn't reflect reality as es core treats it exactly like `pretty` (relevant especially now that we validate params and throw exception when we find one that is not supported). Furthermore, the human flag has effect on every api that outputs a date, time, percentage or byte size field. For instance the tasks api outputs a date field although they don't have the human flag explicitly listed in their spec. There are other similar cases. This commit removes the human flag from the rest spec and makes it an always accepted query_string param.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants