Key id / name should fail if set to 0 / ''

[dhermes]

In Key.id(), the check

 def id(self, id_to_set=None):
 ...
 if id_to_set:
 clone = self._clone()

also ignores False-y objects other than None.

This means it ignores the integer 0, which would seem to be a valid ID. However, the backend actually rejects this, so maybe this bug only requires fixing the docstring?

>>> from gcloud import datastore
>>> cnxn = datastore.get_connection()
>>> import os
>>> dataset = cnxn.dataset(os.getenv('GCLOUD_TESTS_DATASET_ID'))
>>> entity = dataset.entity('foo')
>>> entity.key()._path
[{'kind': 'foo'}]
>>> entity.key()._path[0]['id'] = 0
>>> entity
<Entity[{'kind': 'foo', 'id': 0}] {}>
>>> entity['bar'] = u'baz'
>>> entity.save()
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File "gcloud/datastore/entity.py", line 246, in save
 exclude_from_indexes=self.exclude_from_indexes())
 File "gcloud/datastore/connection.py", line 477, in save_entity
 result = self.commit(dataset_id, mutation)
 File "gcloud/datastore/connection.py", line 377, in commit
 datastore_pb.CommitResponse)
 File "gcloud/datastore/connection.py", line 95, in _rpc
 data=request_pb.SerializeToString())
 File "gcloud/datastore/connection.py", line 73, in _request
 raise Exception('Request failed. Error was: %s' % content)
Exception: Request failed. Error was: Key path id is invalid. Must not be zero.

/cc @pcostell

[pcostell]

Datastore in the near future will treat {'kind': 'foo', 'id': 0} == {'kind': 'foo'} if id is 0 or name is set and it is the empty string. Note that means setting the id = 0 is actually a request to automatically allocate the id (assuming you don't put it in the update section of the mutation).

[dhermes]

So my vote is to raise an exception if a user passes 0 to id() or '' to name() so they don't assume those are allowed as actual IDs/names. If they did assume that and get an auto-allocated ID, they'd think the library was lying to them, right?

[pcostell]

Makes sense to me.

[dhermes]

Deferring this to #532