search

googleapis / google-cloud-go

Google Cloud Client Libraries for Go.
https://cloud.google.com/go/docs/reference
Apache License 2.0
3.73k stars 1.28k forks source link

datastore: versioned mutations #3756

Open hermanbanken opened 3 years ago

hermanbanken commented 3 years ago

Is your feature request related to a problem? Please describe. Looking through the internal code of the Datastore library, I notice that the LookupResult and Mutation both have properties with a 'version' field. The entities in the Datastore apparently have a version and it would be possible to create mutations that are rejected because the entity is outdated.

This kind of optimistic locking is precisely what I'd like to use. However, the current Datastore library does not expose this.

Describe the solution you'd like Just like the Datastore currently supports loading Key's into __key__ labeled fields (KeyLoader), the library could also support loading the version into __version__ labeled fields (VersionLoader). Then any update/mutation could reinstantiate this __version__ value into the Mutation.

Describe alternatives you've considered Alternatives like extending the user API via options would be troublesome and cumbersome for existing users of the library to implement. The __version__ addition would be fairly invisible for existing users. Through the documentation we can explain this feature.

crwilcox commented 3 years ago

Relevant gRPC type https://cloud.google.com/datastore/docs/reference/data/rpc/google.datastore.v1#google.datastore.v1.EntityResult

hermanbanken commented 3 years ago

I drafted how exposing the EntityResult.Version could look in a fork: https://github.com/googleapis/google-cloud-go/compare/50ca760f8a4415a78de2556fc545e340eae98715...hermanbanken:feature/datastore/version. This is not the whole proposal, but at the very least makes it possible for users to retrieve the entity version. The BaseVersion could then be set to the Mutation objects if desired.

It seems that Keys (through __key__) are already internally a bit bolted-on, and adding __version__ doesn't make it much nicer. In some functions the pb.Entity is used, which does contain Key but does not contain Version, so then the version needs to be a parameter and return value.

Maybe I'm completely doing this wrong. How do the architects of this API feel about this issue? @crwilcox @tritone?

24seconds commented 2 years ago

Hello, is there an any updates or progress on this? (design is finalized or something?)