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

Address sixth part of 451: Implement Key.compare_to_proto to check pb keys against existing. #462

Merged
merged 2 commits into from
Dec 31, 2014

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Dec 23, 2014

Addresses sixth part of #451.

NOTE: This has #461 as a diffbase.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 031b864 on dhermes:fix-451-part6 into b343acf on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor

tseaver commented Dec 30, 2014

031b864 comments inline on the changeset.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

RE: "Why not pass self._data". That was creep from the killed #445. I rebased and got rid of it.

RE: "Calling Entity.key() twice in Entity.save()". I was trying to use the public API as often as possible. I think we should remove the key() / _key distinction and just make key an attribute.

RE: "Replicating code from helpers.key_from_protobuf". See my comment in the parent issue.

RE: "Server completion of a partial key". ISTM the server will always complete with an ID, hence the name auto_id. @pcostell can you confirm?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1778430 on dhermes:fix-451-part6 into 8cb41f6 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Dec 30, 2014

As I look at this more I wonder why Connection.save_entity returns the whole Key pb when the backend assigns an auto ID. Why not just return the assigned ID? (Then this whole PR changes, compare_to_proto doesn't need to be implemented.)

In some sense, we want to check that the entire returned key matches but in another sense, we should trust that the backend has preserved everything except the missing final ID.

@tseaver WDYT?

@tseaver
Copy link
Contributor

tseaver commented Dec 31, 2014

I'd prefer not to return the protobuf, and I really don't like the "type variant" return signature. Could we have Connection.save_entity return a tuple, (assigned, new_id). E.g.:

assigned, new_id = connection.save_entity(ds_id, key_pb, properties)
if assigned:
    entity.key = entity.key.complete_key(new_id)

@dhermes
Copy link
Contributor Author

dhermes commented Dec 31, 2014

Good call. On it.

@tseaver
Copy link
Contributor

tseaver commented Dec 31, 2014

Travis failure: rename to completed_key was incomplete. :)

Making Connection.save_entity return the auto allocated ID
instead of the entire PB.
@dhermes
Copy link
Contributor Author

dhermes commented Dec 31, 2014

Removed Key.compare_to_proto and made Connection.save_entity have a consistent return type (if you consider None to be valid as type int).

@tseaver PTAL

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a257a9c on dhermes:fix-451-part6 into f0e26b8 on GoogleCloudPlatform:master.

tseaver added a commit that referenced this pull request Dec 31, 2014
Address sixth part of 451: Implement Key.compare_to_proto to check pb keys against existing.
@tseaver tseaver merged commit e86ba09 into googleapis:master Dec 31, 2014
@dhermes dhermes deleted the fix-451-part6 branch December 31, 2014 22:05
@pcostell
Copy link
Contributor

pcostell commented Jan 4, 2015

@dhermes the server will always complete keys when the entity is part of insert_with_auto_id. Otherwise, it will not complete it. If something bad happens the server will throw an error (rather than return an incomplete key).

@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Dec 31, 2015
atulep pushed a commit that referenced this pull request Apr 6, 2023
Co-authored-by: AJ Morozoff <amorozoff@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this pull request Apr 6, 2023
Co-authored-by: AJ Morozoff <amorozoff@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
atulep pushed a commit that referenced this pull request Apr 18, 2023
Co-authored-by: AJ Morozoff <amorozoff@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea added a commit that referenced this pull request Aug 15, 2023
* fix(deps): allow protobuf 3.19.5

* explicitly exclude protobuf 4.21.0
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
Source-Link: googleapis/synthtool@4760d8d
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f0e4b51deef56bed74d3e2359c583fc104a8d6367da3984fc5c66938db738828

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/30bd01b4ab78bf1b2a425816e15b3e7e090993dd
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:9bc5fa3b62b091f60614c08a7fb4fd1d3e1678e326f34dd66ce1eefb5dc3267b
parthea pushed a commit that referenced this pull request Sep 22, 2023
…[autoapprove] (#462)

Source-Link: googleapis/synthtool@1f37ce7
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:8e84e0e0d71a0d681668461bba02c9e1394c785f31a10ae3470660235b673086
parthea added a commit that referenced this pull request Sep 22, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
…eIamPolicies API (#462)

* feat: Add client library support for AssetService v1 BatchGetEffectiveIamPolicies API
Committer: haochunzhang@

PiperOrigin-RevId: 466134014

Source-Link: googleapis/googleapis@63c73fb

Source-Link: googleapis/googleapis-gen@2350945
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMjM1MDk0NWY3YTcwZWNhYWVjZjlhMWZkZDdkNmU3MGFjNTBlODYyZCJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Oct 21, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea added a commit that referenced this pull request Oct 22, 2023
Co-authored-by: AJ Morozoff <amorozoff@google.com>
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants