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

Simplify underscore requirements in parameters #26886

Closed
ZLightning opened this issue Oct 4, 2017 · 7 comments

Comments

@ZLightning
Copy link

@ZLightning ZLightning commented Oct 4, 2017

Describe the feature: When updating a document through the update API, we have to specify parameters like retry_on_conflict without a leading _ in query-string parameters, but on the bulk API, we have to include the parameter as _retry_on_conflict in the action/meta line. Remembering when to use _ can be confusing to the user. This is even more true when using a library like elasticsearch.js where you have to specify parameters such as index, type, and id in one case and _index, _type, _id in another. Elasticsearch should accept parameters in the action/meta line with or without the leading _, and it seems this is already true of some parameters like version/_version and routing/_routing.

@clintongormley

This comment has been minimized.

Copy link
Member

@clintongormley clintongormley commented Oct 6, 2017

Discussed in FixItFriday. We agree that we should make this consistent with where these parameters are used in other APIs, eg:

  • _index, _type, and _id would keep their underscores, because that is how the fields are named
  • params like version and retry_on_conflict would be without underscores as that is how they are used in other APIs
  • _source would be with underscore also because that is how it is used in other APIs

This should be deprecated for as long as possible before making the breaking change.

@PnPie

This comment has been minimized.

Copy link
Contributor

@PnPie PnPie commented Oct 7, 2017

Hello,
We are going to remove all the leading underscore in Bulk API for the parameters like version, routing etc. ? or we just add support for the two format like _version/version. The latter is what we do currently, and not only in Bulk API, and also in for example Multi Get API, the body of More Like This Query and Term Vectors

@clintongormley

This comment has been minimized.

Copy link
Member

@clintongormley clintongormley commented Oct 9, 2017

We are going to remove all the leading underscore in Bulk API for the parameters like version, routing etc

Yes. We make it consistent with how these parameters are used elsewhere (obviously with a deprecation period).

not only in Bulk API, and also in for example Multi Get API, the body of More Like This Query and Term Vectors

thanks for pointing those out. we should do the same in these APIs

@mayya-sharipova mayya-sharipova self-assigned this Oct 11, 2017
@sumanthrao

This comment has been minimized.

Copy link

@sumanthrao sumanthrao commented Oct 17, 2017

I would like to work on this if no one is working.

@orinn248

This comment has been minimized.

Copy link

@orinn248 orinn248 commented Oct 17, 2017

I'm looking into this as well

@mayya-sharipova

This comment has been minimized.

Copy link
Contributor

@mayya-sharipova mayya-sharipova commented Oct 18, 2017

@sumanthrao @orinn248 I have already finished the implementation on this, will create a PR soon

@jasontedor jasontedor removed the help wanted label Oct 18, 2017
@jasontedor

This comment has been minimized.

Copy link
Member

@jasontedor jasontedor commented Oct 18, 2017

Thanks @mayya-sharipova, I’m removing the adoptme label.

mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this issue Nov 7, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery were
changed

Closes elastic#26886
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this issue Nov 7, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery were changed

Closes elastic#26886
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this issue Nov 7, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores
In 6.x these parameters are deprecated and produce
deprecated warnings.

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery
were changed

Closes elastic#26886
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this issue Nov 17, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores
Throw an error if older versions of parameters are used

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery
were changed

Closes elastic#26886
mayya-sharipova added a commit to mayya-sharipova/elasticsearch that referenced this issue Nov 17, 2017
…pe of

requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores
Throw an error if older versions of parameters are used

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery
were changed

Related to elastic#27040
Closes elastic#26886
mayya-sharipova added a commit that referenced this issue Nov 17, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores
Throw an error if older versions of parameters are used

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery
were changed

Closes #26886
mayya-sharipova added a commit that referenced this issue Nov 17, 2017
Stardardize underscore requirements in parameters across different type of
requests:
_index, _type, _source, _id keep their underscores
params like version and retry_on_conflict will be without underscores
Log a warning if older versions of these parameters are used

BulkRequest, MultiGetRequest, TermVectorcRequest, MoreLikeThisQuery
were changed

Closes #26886
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.