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

Adding api.delete() and removing Key.delete(). #527

Merged
merged 2 commits into from
Jan 12, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 9, 2015

Also:

  • Changing api.get() back to accept only keys and
    returns only a list (a lot of headache for not much
    gain).
  • Factored out behavior to extract shared dataset_id from
    a set of keys into _get_dataset_id_from_keys().
  • Updated docstrings and other tests that rely on changed /
    removed methods.

See #518 for some context.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c5a634f on dhermes:add-delete-method into 0525530 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 9, 2015

@tseaver I took a stab at testing / integrating the implementation from your comment.

If you already wrote this up, feel free to close.

Also, viewing _get_dataset_id_from_keys in light of #447, we should remove dataset_id from the Batch constructor and instead rely on Batch.put(), Batch.add_auto_id_entity(), and Batch.delete() to set / check the dataset ID. (This also means Batch.begin() has to move outside of __enter__.)

@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Jan 9, 2015
@tseaver
Copy link
Contributor

tseaver commented Jan 12, 2015

LGTM.

Also:
- Changing api.get() back to accept only `keys` and
  returns only a list (a lot of headache for not much
  gain).
- Factored out behavior to extract shared dataset_id from
  a set of keys into _get_dataset_id_from_keys().
- Updated docstrings and other tests that rely on changed /
  removed methods.

See googleapis#518 for some context.
@tseaver
Copy link
Contributor

tseaver commented Jan 12, 2015

Merge broken?

======================================================================
ERROR: test_delete_existing_batch (gcloud.datastore.test_api.Test_delete_function)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/GoogleCloudPlatform/gcloud-python/gcloud/datastore/test_api.py", line 322, in test_delete_existing_batch
connection._deleted,
AttributeError: '_Connection' object has no attribute '_deleted'
======================================================================
ERROR: test_delete_implicit_connection (gcloud.datastore.test_api.Test_delete_function)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/GoogleCloudPlatform/gcloud-python/gcloud/datastore/test_api.py", line 350, in test_delete_implicit_connection
connection._deleted,
AttributeError: '_Connection' object has no attribute '_deleted'

Behavior of test_batch._Connection changed in googleapis#524.
@dhermes
Copy link
Contributor Author

dhermes commented Jan 12, 2015

No, merge was fine, just changes to test_batch._Connection from #524 broke my tests (rightfully so).

I fixed in 2908d6f.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2908d6f on dhermes:add-delete-method into 51c1069 on GoogleCloudPlatform:master.

dhermes added a commit that referenced this pull request Jan 12, 2015
Adding api.delete() and removing Key.delete().
@dhermes dhermes merged commit dcb73d3 into googleapis:master Jan 12, 2015
@dhermes dhermes deleted the add-delete-method branch January 12, 2015 22:04
parthea pushed a commit that referenced this pull request Aug 15, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/909573ce9da2819eeb835909c795d29aea5c724e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ddf4551385d566771dc713090feb7b4c1164fb8a698fe52bbe7670b24236565b
parthea pushed a commit that referenced this pull request Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/92006bb3cdc84677aa93c7f5235424ec2b157146
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:2e247c7bf5154df7f98cce087a20ca7605e236340c7d6d1a14447e5c06791bd6
parthea pushed a commit that referenced this pull request Oct 21, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
parthea pushed a commit that referenced this pull request Oct 21, 2023
* docs: Fix formatting of request arg in docstring

chore: Update gapic-generator-python to v1.9.1
PiperOrigin-RevId: 518604533

Source-Link: googleapis/googleapis@8a085ae

Source-Link: googleapis/googleapis-gen@b2ab4b0
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYjJhYjRiMGEwYWUyOTA3ZTgxMmMyMDkxOThhNzRlMDg5OGFmY2IwNCJ9

* 🦉 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 22, 2023
* chore: Update gapic-generator-python to v1.11.3

PiperOrigin-RevId: 546899192

Source-Link: googleapis/googleapis@e6b1691

Source-Link: googleapis/googleapis-gen@0b3917c
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMGIzOTE3YzQyMWNiZGE3ZmNiNjcwOTJlMTZjMzNmM2VhNDZmNGJjNyJ9

* 🦉 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>
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.

3 participants