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

Fix #484: Remove dataset from entity #488

Merged
merged 1 commit into from
Jan 6, 2015

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jan 3, 2015

Also:

  • Dropping dataset and kind from Entity constructor.
  • Dropping Entity.from_path factory.
  • Removing Connection.dataset() quasi-factory.
  • helpers.entity_from_protobuf() no longer accepts / needs dataset.
  • Making Entity.key a data attribute instead of a getter/setter method.
  • Removing Entity.dataset() (since no longer stored).
  • Making Entity.kind() and Entity.exclude_from_indexes() into @property's.
  • Removing Entity._must_dataset (no longer needed).
  • Adding optional connection argument to Entity.save() and Entity.reload().
  • Making Entity.save() and Entity.reload() return nothing.
  • Updating regression/datastore.py for changes.

Fix #484

NOTE: Has #483 as diffbase.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0dbf10c on dhermes:remove-dataset-from-entity into b79d862 on GoogleCloudPlatform:master.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

@tseaver I am re-working this now, having just merged #483.

Can you take a stab at #485?

Also:
- Dropping dataset and kind from Entity constructor.
- Dropping Entity.from_path factory.
- Removing Connection.dataset().
- helpers.entity_from_protobuf() no longer requires dataset.
- Making Entity.key a data attribute instead of a getter/setter method.
- Removing Entity.dataset() (since no longer stored).
- Making Entity.kind() and Entity.exclude_from_indexes() @Property's.
- Removing Entity._must_dataset (no longer needed).
- Adding optional connection argument to Entity.save() and Entity.reload().
- Making Entity.save() and Entity.reload() return nothing.
- Updating regression/datastore.py for changes.
@dhermes dhermes force-pushed the remove-dataset-from-entity branch from 0dbf10c to 1dab892 Compare January 6, 2015 21:42
@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

@tseaver Rebase completed. PTAL.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1dab892 on dhermes:remove-dataset-from-entity into a02a73e on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

Looks OK. No impact on regression tests?

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

The impact is the code looks nicer every time we throw something away 👍

LGTY?

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

Yes -- I just didn't see any changes in the regression side.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

They're there, at the very bottom.

dhermes added a commit that referenced this pull request Jan 6, 2015
@dhermes dhermes merged commit 22c8b19 into googleapis:master Jan 6, 2015
@dhermes dhermes deleted the remove-dataset-from-entity branch January 6, 2015 22:00
@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

We have regression failures right now.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

Where? If you're referring to build 808, that was a transitive failure (we should add retries so flakes stop occurring so often).

UPDATE: Don't see any failures on PR builds: https://travis-ci.org/GoogleCloudPlatform/gcloud-python/pull_requests (latest is 819)

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

Like so:

======================================================================
FAIL: test_empty_kind (datastore.TestDatastoreSave)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tseaver/projects/agendaless/Google/src/gcloud-python/regression/datastore.py", line 141, in test_empty_kind
    self.assertEqual(posts, [])
AssertionError: Lists differ: [<Entity[{'kind': u'Post', 'id': 123456789[522 chars]se}>] != []

First list contains 2 additional elements.
First extra element 0:
<Entity[{'kind': u'Post', 'id': 123456789L}] {u'title': u'How to make the perfect pizza in your grill', u'publishedAt': datetime.datetime(2001, 1, 1, 0, 0, tzinfo=<UTC>), u'wordCount': 400L, u'author': u'Silvano', u'rating': 5.0, u'tags': [u'pizza', u'grill'], u'isDraft': False}>

+ []
- [<Entity[{'kind': u'Post', 'id': 123456789L}] {u'title': u'How to make the perfect pizza in your grill', u'publishedAt': datetime.datetime(2001, 1, 1, 0, 0, tzinfo=<UTC>), u'wordCount': 400L, u'author': u'Silvano', u'rating': 5.0, u'tags': [u'pizza', u'grill'], u'isDraft': False}>,
-  <Entity[{'kind': u'Post', 'id': 5216192952270848L}] {u'title': u'How to make the perfect homemade pasta', u'publishedAt': datetime.datetime(2001, 1, 1, 0, 0, tzinfo=<UTC>), u'wordCount': 450L, u'author': u'Silva

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

The test is test_empty_kind, so you have some entities from a previously failed test that didn't get deleted.

I made regression/clear_datastore.py for cases like that but filed #453 when I realized I couldn't keep up with it as the API changed massively.

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

Ugh:

$ .tox/regression/bin/python regression/clear_datastore.py 
This command will remove all entities for the following kinds:
- Character
- Company
- Kind
- Person
- Post
Is this OK [y/n]? y
Traceback (most recent call last):
File "regression/clear_datastore.py", line 107, in <module>
    remove_all_entities()
File "regression/clear_datastore.py", line 103, in remove_all_entities
    remove_kind(dataset, kind)
File "regression/clear_datastore.py", line 66, in remove_kind
    with dataset.transaction():
AttributeError: 'Dataset' object has no attribute 'transaction'

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

Yeah, I will fix the utility after we get stable again. In the meantime:

from gcloud import datastore
from gcloud.datastore.query import Query

datastore.set_default_connection()
datastore.set_default_dataset(dataset_id='foo')

q = Query(kind='Post')

for post in q.fetch():
    post.key.delete()

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

That works, but test_empty_kind still fails for me.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

It could be that the tests don't run in a deterministic way? But each case cleans up after itself transactionally so it shouldn't matter.

It should only fail if there are non-deleted keys. Are you using the same dataset ID for tests and in the snippet above? Do you have other failures occurring before that test gets run (hence failed deletes)?

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

No other failures; I'm running by doing:

$ .tox/regression/bin/python
>>> import regression.datastore # to set up the connection / dataset
>>> from gcloud.datastore.query import Query
>>> q = Query(kind='Post')
>>> list(q.fetch())
[]

but the regression test still fails.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

Can you print the failure message? (Is it same as above?)

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

The 'id' fields in the keys are different; everything else is the same.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

I can't replicate locally or on Travis. Are you running from master (22c8b19) or elsewhere? Is it possible that Transaction is swallowing some failures and some self.case_entities_to_delete.append statements aren't being executed? (Potentially related to #457.)

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

Uhoh: list(query.fetch()) returned empty, while list(query.fetch(limit=100)) returned 3 items.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

It's good to have found that though.

@tseaver
Copy link
Contributor

tseaver commented Jan 6, 2015

#491

So, now I've deleted them, and my regression tests are passing.

@dhermes
Copy link
Contributor Author

dhermes commented Jan 6, 2015

w00t.

Gonna supply some code in your issue which illustrates this regression.

@dhermes dhermes added the api: datastore Issues related to the Datastore API. label Dec 31, 2015
tswast added a commit to tswast/google-cloud-python that referenced this pull request Mar 26, 2019
* Generate protos for Models API.

The Models API is a component of the bigquery_v2 interface.
It is not available as a gRPC API, but it does provide
protocol buffers. This commit adds those protocol buffers
to the client so that they can be used to avoid much manual
work to create resource classes that can be serialized to/from
JSON.
atulep pushed a commit that referenced this pull request Apr 3, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
atulep pushed a commit that referenced this pull request Apr 6, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
atulep pushed a commit that referenced this pull request Apr 6, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
atulep pushed a commit that referenced this pull request Apr 18, 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 Aug 15, 2023
* feat: ResponseMessage proto contains channel information
docs: updated go library package

PiperOrigin-RevId: 501638939

Source-Link: googleapis/googleapis@529c07a

Source-Link: googleapis/googleapis-gen@6dc5598
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNmRjNTU5OGZhY2VjY2UyZTUyMDRjMzVhY2JlMmU0NjBmOWVjYTcxYyJ9

* 🦉 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>
vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
* feat: added HUMAN_INTERVENTION_NEEDED type in ConversationEvent
feat: added SetSuggestionFeatureConfig and ClearSuggestionFeatureConfig apis
feat: added AGENT_FACING_SMART_REPLY type in KnowledgeType
feat: added GcsDestination.
docs: added explanation for uri fields in resources
docs: added explanation for SuggestionResult
docs: added explanation for language code in session

PiperOrigin-RevId: 435479473

Source-Link: googleapis/googleapis@d9ca515

Source-Link: googleapis/googleapis-gen@358c4b9
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMzU4YzRiOWIxZGYxNDliYzUwNjIwYmMzODExMDA0ZjkxM2Q0MzIzNiJ9

* 🦉 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 Sep 22, 2023
Source-Link: https://togithub.com/googleapis/synthtool/commit/0ddbff8012e47cde4462fe3f9feab01fbc4cdfd6
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bced5ca77c4dda0fd2f5d845d4035fc3c5d3d6b81f245246a36aee114970082b
parthea pushed a commit that referenced this pull request Sep 22, 2023
…p/templates/python_library/.kokoro (#488)

Source-Link: https://togithub.com/googleapis/synthtool/commit/bb171351c3946d3c3c32e60f5f18cee8c464ec51
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f62c53736eccb0c4934a3ea9316e0d57696bb49c1a7c86c726e9bb8a2f87dadf
parthea pushed a commit that referenced this pull request Oct 21, 2023
Source-Link: googleapis/synthtool@56da63e
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:993a058718e84a82fda04c3177e58f0a43281a996c7c395e0a56ccc4d6d210d7
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 pushed a commit that referenced this pull request Oct 21, 2023
Source-Link: googleapis/synthtool@909573c
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:ddf4551385d566771dc713090feb7b4c1164fb8a698fe52bbe7670b24236565b

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
Source-Link: https://togithub.com/googleapis/synthtool/commit/26c7505b2f76981ec1707b851e1595c8c06e90fc
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:f946c75373c2b0040e8e318c5e85d0cf46bc6e61d0a01f3ef94d8de974ac6790
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.

Remove dataset from Entity constructor and allow a key
3 participants