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

Version types EXTERNAL & EXTERNAL_GTE test for version equality in read operation & disallow them in the Update API #5929

Closed

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Apr 24, 2014

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change VersionType.EXTERNAL & VersionType.EXTERNAL_GTE to behave the same as VersionType.INTERNAL for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of EXTERNAL & EXTERNAL_GTE in the update api raise a validation error as it make cause data to
be lost.

Closes #5663 , closes #5661

…n read operation & disallow them in the Update API

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change `VersionType.EXTERNAL` & `VersionType.EXTERNAL_GTE` to behave the same as `VersionType.INTERNAL` for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of `EXTERNAL` & `EXTERNAL_GTE` in the update api raise a validation error as it make cause data to
be lost.

Closes elastic#5663 , closes elastic#5661
@@ -165,6 +165,14 @@ including:
Support `_source` to return the full updated
source.

`version` & `version_type`:: the Update API uses the Elasticsearch's versioning
Copy link
Contributor

Choose a reason for hiding this comment

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

this should maybe have a since tag to make sure folks know when it was added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was already there, though not documented. Added in 1.0.0 beta

@s1monw
Copy link
Contributor

s1monw commented Apr 24, 2014

I left some comments, looks good but it seems like it has lots of unrelated changes?

@s1monw s1monw added the blocker label Apr 24, 2014
@bleskes bleskes removed the review label Apr 24, 2014
@s1monw
Copy link
Contributor

s1monw commented Apr 25, 2014

ok good! I think this is good to go! LGTM

@bleskes bleskes closed this in 051beb5 Apr 25, 2014
bleskes added a commit that referenced this pull request Apr 25, 2014
…n read operation & disallow them in the Update API

Separate version check logic for reads and writes for all version types, which allows different behavior in these cases.
Change `VersionType.EXTERNAL` & `VersionType.EXTERNAL_GTE` to behave the same as `VersionType.INTERNAL` for read operations.
The previous behavior was fit for writes but is useless in reads.

This commit also makes the usage of `EXTERNAL` & `EXTERNAL_GTE` in the update api raise a validation error as it make cause data to
be lost.

Closes #5663 , Closes #5661, Closes #5929
@clintongormley clintongormley added the :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. label Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker >breaking :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v1.2.0 v2.0.0-beta1
Projects
None yet
4 participants