Property with empty list 4 #512

lucemia · 2015-01-08T16:41:19Z

for issue #403

Previous PR
#404

gcloud/datastore/connection.py

dhermes · 2015-01-08T18:21:49Z

I like this change.
What is the difficulty in getting this to work for PropertyFilter?
Please run tox locally before submitting changes.
I want to let @tseaver have a chance to see this before signing off.

tseaver · 2015-01-08T22:31:29Z

_set_protobuf_property looks like a win: I think we have a least a couple of versions of it lying around in the code. Once @dhermes' issues are addressed, LGTM.

lucemia · 2015-01-09T03:13:32Z

@dhermes
for Property with None value, my testing shows that the current behavior is the same as ndb. (both save a property will null value)
In that case we don't need to handle the None value.
I will updated the docstring for save_entity

ndb

from google.appengine.ext import ndb

class Kind(ndb.Model):
    foo = ndb.StringProperty()
    bar = ndb.StringProperty()
    bar1 = ndb.StringProperty(repeated=True)


a = Kind(bar=None, bar1=[])
a.put()

gcloud-python

from gcloud import datastore
from gcloud.credentials import *
from gcloud.datastore.key import *


client_email = "[email protected]"
private_key_path = "path/to/key.p12"
SCOPE = ('https://www.googleapis.com/auth/datastore ',
         'https://www.googleapis.com/auth/userinfo.email')
credentials = get_for_service_account_p12(client_email, private_key_path, SCOPE)

key_pb = Key('Kind', 1234, dataset_id="tagtooadex2").to_protobuf()

conn = datastore.connection.Connection(credentials)

result = conn.save_entity("tagtooadex2", key_pb, {
    'foo': u'Foo',
    'bar': None,
    'bar1': []
})

dhermes · 2015-01-09T06:21:43Z

@lucemia I was curious to see what actually is stored, which requires inspecting the retrieved protobuf.

After storing

>>> from gcloud import datastore
>>> from gcloud.datastore.entity import Entity
>>> from gcloud.datastore.key import Key
>>>
>>> datastore.set_default_connection()
>>> dataset_id = 'datasetfoo'
>>> datastore.set_default_dataset_id(dataset_id)
>>>
>>> key = Key('Kind', 1234)
>>> entity = Entity(key=key)
>>> entity['foo'] = None
>>> dict(entity)
{'foo': None}
>>> entity.save()

retrieving the same entity shows that the property is set but Property.value is empty:

>>> entity_pb, = datastore.get_connection().lookup(
...     dataset_id=dataset_id,
...     key_pbs=[key.to_protobuf()],
... )
>>> prop_foo, = entity_pb.property
>>> prop_foo.name
u'foo'
>>> prop_foo_value = prop_foo.value
>>> prop_foo_value._fields
{}

This happens because in helpers._set_protobuf_value, Property.value is cleared but the property is still set on the Entity.

It's unclear what is the right thing to support, maybe going with ndb is fine? However, the note you introduce makes it seem the property will be dropped all together (as is done for []).

Cleanup:

>>> print entity_pb
key {
  partition_id {
    dataset_id: "s~datasetfoo"
  }
  path_element {
    kind: "Kind"
    id: 1234
  }
}
property {
  name: "foo"
  value {
  }
}

>>> entity.key.delete()

coveralls · 2015-01-09T07:22:45Z

Coverage remained the same when pulling 479ea6a on lucemia:property-with-empty-list-4 into 6bde37d on GoogleCloudPlatform:master.

lucemia · 2015-01-10T02:58:37Z

@dhermes

It's unclear what is the right thing to support, maybe going with ndb is fine? However, the note you introduce makes it seem the property will be dropped all together (as is done for []).

Currently I go with ndb way. I updated the note to only include empty list case.

What is the difficulty in getting this to work for PropertyFilter?

Store a Property whose value is empty list sounds normal and should not raise any exception (currently I am working with).
But create a PropertyFilter whose value is empty list sounds weird.
Which behavior (write a helper function or raise exception) is recommend for PropertyFilter case is unclear for me.

ps. Rebase is welcome

In ndb, pass an empty list will cause a special FalseNode state.

s~tagtooadex2> Kind.query(Kind.bar.IN([]))
Query(kind='Kind', filters=<google.appengine.ext.ndb.query.FalseNode object at 0x104ab5210>)

s~tagtooadex2> Kind.query(Kind.bar.IN(['a']))
Query(kind='Kind', filters=FilterNode('bar', '=', 'a'))

and raise exception while query.

s~tagtooadex2> Kind.query(Kind.bar.IN([])).fetch()
WARNING:root:initial generator _run_to_list(query.py:956) raised BadQueryError(Cannot convert FalseNode to predicate)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/utils.py", line 142, in positional_wrapper
    return wrapped(*args, **kwds)
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line 1187, in fetch
    return self.fetch_async(limit, **q_options).get_result()
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/tasklets.py", line 325, in get_result
    self.check_success()
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/tasklets.py", line 371, in _help_tasklet_along
    value = gen.send(val)
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line 961, in _run_to_list
    dsquery = self._get_query(conn)
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line 908, in _get_query
    filters = filters._to_filter()
  File "/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line 425, in _to_filter
    'Cannot convert FalseNode to predicate')
BadQueryError: Cannot convert FalseNode to predicate

dhermes · 2015-01-10T18:33:49Z

I see, I was thinking the issue was the code, but you're saying the expected behavior is a bit subtle.

Also, what do you mean by "Rebase is welcome"?

lucemia · 2015-01-11T01:04:11Z

@dhermes

Also, what do you mean by "Rebase is welcome"?

Last pull request you ask about re-writeing history. I mean feel free to do so if necessary :)

…helper function.

coveralls · 2015-01-12T02:48:19Z

Coverage remained the same when pulling a250cb2 on lucemia:property-with-empty-list-4 into 0525530 on GoogleCloudPlatform:master.

lucemia · 2015-01-12T23:22:11Z

I have squash the commit

Ignore Entity property with empty list

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

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

Source-Link: googleapis/synthtool@f15cc72 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:bc5eed3804aec2f05fad42aacf973821d9500c174015341f721a984a0825b6fd

* feat: add SPOT to Preemptibility enum PiperOrigin-RevId: 503019826 Source-Link: googleapis/googleapis@77cd8f1 Source-Link: googleapis/googleapis-gen@a3b02db Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYTNiMDJkYmIzMzljZjhkYWU1ZDYxMTQ1MDI5MmI3ODIwZjEyMDA3NSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * docs: Add documentation for enums fix: Add context manager return types chore: Update gapic-generator-python to v1.8.1 PiperOrigin-RevId: 503210727 Source-Link: googleapis/googleapis@a391fd1 Source-Link: googleapis/googleapis-gen@0080f83 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMDA4MGY4MzBkZWMzN2MzMzg0MTU3MDgyYmNlMjc5ZTM3MDc5ZWE1OCJ9 * 🦉 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>

#512)

* chore(samples): Remove samples Samples migrated to https://github.com/GoogleCloudPlatform/python-docs-samples/tree/main/monitoring GoogleCloudPlatform/python-docs-samples#8472 * Add README with link to new samples location

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

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

….0 (#512)

Source-Link: googleapis/synthtool@993985f Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:1894490910e891a385484514b22eb5133578897eb5b3c380e6d8ad475c6647cd

- Pass the app_profile_id from the message - Update unittest Fixes internal bug #214449800

* Enable support and testing for python 3.11 * feat: Enable support and testing for python 3.11 * build: Add kokoro configuration for python 3.11 * build: Remove kokoro configuration for python 3.11 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * add unit_test_python_versions arg * 🦉 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> Co-authored-by: Anthonios Partheniou <[email protected]>

* feat: add new types QueryMode, QueryPlan, ResultSetStats feat: add QueryMode field to RunQueryRequest feat: add ResultSetStats field to RunQueryResponse feat: add QueryMode field to RunAggregationQueryRequest feat: add ResultSetStats field to RunAggregationQueryResponse PiperOrigin-RevId: 595774772 Source-Link: googleapis/googleapis@03e7ed4 Source-Link: googleapis/googleapis-gen@dc63e0d Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZGM2M2UwZGVhODQyM2MyMzBkNWZiMDkzN2FjYjNjOTg3MTljOTM5NSJ9 * 🦉 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>

…512) * chore(python): Use latest python runtime in prerelease_deps session Source-Link: googleapis/synthtool@14d8b28 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:5651442a6336971a2fb2df40fb56b3337df67cafa14c0809cc89cb34ccee1b8e * filter warnings from google-cloud-logging 1.x --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <[email protected]>

* chore: allow releases on previous majors * 🦉 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>

dhermes mentioned this pull request Jan 8, 2015

fix empty set #404

Closed

dhermes reviewed Jan 8, 2015
View reviewed changes

gcloud/datastore/connection.py Outdated

This comment was marked as spam.

Sign in to view

dhermes mentioned this pull request Jan 9, 2015

Storing an Entity as an indexed property value always fails #529

Closed

dhermes added the api: datastore Issues related to the Datastore API. label Jan 9, 2015

fix empty list issue googleapis#403 by add a _set_protobuf_property …

a250cb2

…helper function.

lucemia force-pushed the property-with-empty-list-4 branch from 479ea6a to a250cb2 Compare January 12, 2015 02:46

dhermes added a commit that referenced this pull request Jan 13, 2015

Merge pull request #512 from lucemia/property-with-empty-list-4

8a42ef5

Ignore Entity property with empty list

dhermes merged commit 8a42ef5 into googleapis:master Jan 13, 2015

This was referenced Jan 13, 2015

Sniff out and fix duplicated code replaced by _set_protobuf_property #540

Closed

raise unexpected exception while property value is [] #403

Closed

#514: rip out Connection cruft #550

Merged

parthea pushed a commit that referenced this pull request Aug 15, 2023

chore(main): release 1.20.1 (#512)

81f2811

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

chore(deps): update dependency google-cloud-resource-manager to v1.6.3 (

14260e7

#512)

parthea pushed a commit that referenced this pull request Oct 21, 2023

chore(main): release 3.12.0 (#512)

6d6dac0

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

chore(main): release 3.3.0 (#512)

cf612c8

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

parthea pushed a commit that referenced this pull request Aug 21, 2025

chore(deps): update dependency google-cloud-bigquery-storage to v2.16…

2b5228f

….0 (#512)

parthea pushed a commit that referenced this pull request Sep 16, 2025

chore(deps): update dependency google-cloud-bigquery-storage to v2.16…

f4563fe

….0 (#512)

parthea pushed a commit that referenced this pull request Nov 22, 2025

fix: Pass app_profile_id when building updated request (#512)

52da520

- Pass the app_profile_id from the message - Update unittest Fixes internal bug #214449800

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Property with empty list 4 #512

Property with empty list 4 #512

Uh oh!

lucemia commented Jan 8, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Jan 8, 2015

Uh oh!

tseaver commented Jan 8, 2015

Uh oh!

lucemia commented Jan 9, 2015

Uh oh!

dhermes commented Jan 9, 2015

Uh oh!

coveralls commented Jan 9, 2015

Uh oh!

lucemia commented Jan 10, 2015

Uh oh!

dhermes commented Jan 10, 2015

Uh oh!

lucemia commented Jan 11, 2015

Uh oh!

coveralls commented Jan 12, 2015

Uh oh!

lucemia commented Jan 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Property with empty list 4 #512

Property with empty list 4 #512

Uh oh!

Conversation

lucemia commented Jan 8, 2015

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Jan 8, 2015

Uh oh!

tseaver commented Jan 8, 2015

Uh oh!

lucemia commented Jan 9, 2015

ndb

gcloud-python

Uh oh!

dhermes commented Jan 9, 2015

Uh oh!

coveralls commented Jan 9, 2015

Uh oh!

lucemia commented Jan 10, 2015

Uh oh!

dhermes commented Jan 10, 2015

Uh oh!

lucemia commented Jan 11, 2015

Uh oh!

coveralls commented Jan 12, 2015

Uh oh!

lucemia commented Jan 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants