Property with empty list 4

[lucemia]

for issue #403

Previous PR
#404

[dhermes]

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

[tseaver]

_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]

@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 = "foo@mail.com"
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]

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

After storing

>>>fromgcloudimportdatastore>>>fromgcloud.datastore.entityimportEntity>>>fromgcloud.datastore.keyimportKey>>>>>>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.nameu'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:

>>>printentity_pbkey {
partition_id {
dataset_id: "s~datasetfoo"
}
path_element {
kind: "Kind"id: 1234
}
}
property {
name: "foo"value {
}
}
>>>entity.key.delete()

[coveralls]

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

[lucemia]

@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.FalseNodeobjectat0x104ab5210>)
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:initialgenerator_run_to_list(query.py:956) raisedBadQueryError(CannotconvertFalseNodetopredicate)
Traceback (mostrecentcalllast):
File"<console>", line1, in<module>File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/utils.py", line142, inpositional_wrapperreturnwrapped(*args, **kwds)
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line1187, infetchreturnself.fetch_async(limit, **q_options).get_result()
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/tasklets.py", line325, inget_resultself.check_success()
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/tasklets.py", line371, in_help_tasklet_alongvalue=gen.send(val)
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line961, in_run_to_listdsquery=self._get_query(conn)
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line908, in_get_queryfilters=filters._to_filter()
File"/Users/david/google-cloud-sdk/platform/google_appengine/google/appengine/ext/ndb/query.py", line425, in_to_filter'Cannot convert FalseNode to predicate')
BadQueryError: CannotconvertFalseNodetopredicate

[dhermes]

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]

@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 :)

[coveralls]

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

[lucemia]

I have squash the commit