Skip to content

Address first part of 451: Remove Key.from_path #456

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.

Sign up for GitHub

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

Merged
dhermes merged 1 commit into from
Dec 30, 2014
Merged

Address first part of 451: Remove Key.from_path #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions gcloud/datastore/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def get_entity(key):
:param key: The name of the item to retrieve.

:rtype: :class:`gcloud.datastore.entity.Entity` or ``None``
:return: The requested entity, or ``None`` if there was no match found.
:returns: The requested entity, or ``None`` if there was no match found.
"""
return _require_dataset().get_entity(key)

Expand All @@ -157,7 +157,7 @@ def get_entities(keys):
:param keys: The name of the item to retrieve.

:rtype: list of :class:`gcloud.datastore.entity.Entity`
:return: The requested entities.
:returns: The requested entities.
"""
return _require_dataset().get_entities(keys)

Expand All @@ -172,6 +172,6 @@ def allocate_ids(incomplete_key, num_ids):
:param num_ids: The number of IDs to allocate.

:rtype: list of :class:`gcloud.datastore.key.Key`
:return: The (complete) keys allocated with `incomplete_key` as root.
:returns: The (complete) keys allocated with `incomplete_key` as root.
"""
return _require_dataset().allocate_ids(incomplete_key, num_ids)
28 changes: 9 additions & 19 deletions gcloud/datastore/dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from gcloud.datastore.entity import Entity
from gcloud.datastore.query import Query
from gcloud.datastore.transaction import Transaction
from gcloud.datastore.key import Key


class Dataset(object):
Expand Down Expand Up @@ -118,26 +117,17 @@ def transaction(self, *args, **kwargs):
kwargs['dataset'] = self
return Transaction(*args, **kwargs)

def get_entity(self, key_or_path):
def get_entity(self, key):
"""Retrieves entity from the dataset, along with its attributes.

:type key_or_path: :class:`gcloud.datastore.key.Key` or path
:param key_or_path: The name of the item to retrieve or sequence
of even length, where the first of each pair
is a string representing the 'kind' of the
path element, and the second of the pair is
either a string (for the path element's name)
or an integer (for its id).
:type key: :class:`gcloud.datastore.key.Key` or path
:param key: The key of the entity to be retrieved.

:rtype: :class:`gcloud.datastore.entity.Entity` or ``None``
:return: The requested entity, or ``None`` if there was no match found.
:rtype: :class:`gcloud.datastore.entity.Entity` or `NoneType`
:returns: The requested entity, or ``None`` if there was no
match found.
"""

if isinstance(key_or_path, Key):
entities = self.get_entities([key_or_path])
else:
key = Key.from_path(*key_or_path)
entities = self.get_entities([key])
entities = self.get_entities([key])

if entities:
return entities[0]
Expand All @@ -159,7 +149,7 @@ def get_entities(self, keys, missing=None, deferred=None):
Use only as a keyword param.

:rtype: list of :class:`gcloud.datastore.entity.Entity`
:return: The requested entities.
:returns: The requested entities.
"""
entity_pbs = self.connection().lookup(
dataset_id=self.id(),
Expand Down Expand Up @@ -193,7 +183,7 @@ def allocate_ids(self, incomplete_key, num_ids):
:param num_ids: The number of IDs to allocate.

:rtype: list of :class:`gcloud.datastore.key.Key`
:return: The (complete) keys allocated with `incomplete_key` as root.
:returns: The (complete) keys allocated with `incomplete_key` as root.
:raises: `ValueError` if `incomplete_key` is not a partial key.
"""
if not incomplete_key.is_partial():
Expand Down
37 changes: 0 additions & 37 deletions gcloud/datastore/key.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
"""Create / interact with gcloud datastore keys."""

import copy
from itertools import izip

import six

from gcloud.datastore import datastore_v1_pb2 as datastore_pb

Expand Down Expand Up @@ -90,40 +87,6 @@ def to_protobuf(self):

return key

@classmethod
def from_path(cls, *args, **kwargs):
"""Factory method for creating a key based on a path.

:type args: :class:`tuple`
:param args: sequence of even length, where the first of each pair is a
string representing the 'kind' of the path element, and
the second of the pair is either a string (for the path
element's name) or an integer (for its id).

:type kwargs: :class:`dict`
:param kwargs: Other named parameters which can be passed to
:func:`Key.__init__`.

:rtype: :class:`gcloud.datastore.key.Key`
:returns: a new :class:`Key` instance
"""
if len(args) % 2:
raise ValueError('Must pass an even number of args.')

path = []
items = iter(args)

for kind, id_or_name in izip(items, items):
entry = {'kind': kind}
if isinstance(id_or_name, six.string_types):
entry['name'] = id_or_name
else:
entry['id'] = id_or_name
path.append(entry)

kwargs['path'] = path
return cls(**kwargs)

def is_partial(self):
"""Boolean test: is the key fully mapped onto a backend entity?

Expand Down
19 changes: 4 additions & 15 deletions gcloud/datastore/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,22 +172,15 @@ def ancestor(self, ancestor):
This will return a clone of the current :class:`Query` filtered
by the ancestor provided. For example::

>>> parent_key = Key.from_path('Person', '1')
>>> parent_key = Key(path=[{'kind': 'Person', 'name': '1'}])
>>> query = dataset.query('Person')
>>> filtered_query = query.ancestor(parent_key)

If you don't have a :class:`gcloud.datastore.key.Key` but just
know the path, you can provide that as well::

>>> query = dataset.query('Person')
>>> filtered_query = query.ancestor(['Person', '1'])

Each call to ``.ancestor()`` returns a cloned :class:`Query`,
however a query may only have one ancestor at a time.

:type ancestor: :class:`gcloud.datastore.key.Key` or list
:param ancestor: Either a Key or a path of the form
``['Kind', 'id or name', 'Kind', 'id or name', ...]``.
:type ancestor: :class:`gcloud.datastore.key.Key`
:param ancestor: A Key to an entity

:rtype: :class:`Query`
:returns: A Query filtered by the ancestor provided.
Expand All @@ -211,13 +204,9 @@ def ancestor(self, ancestor):
if not ancestor:
return clone

# If a list was provided, turn it into a Key.
if isinstance(ancestor, list):
ancestor = Key.from_path(*ancestor)

# If we don't have a Key value by now, something is wrong.
if not isinstance(ancestor, Key):
raise TypeError('Expected list or Key, got %s.' % type(ancestor))
raise TypeError('Expected Key, got %s.' % type(ancestor))

# Get the composite filter and add a new property filter.
composite_filter = clone._pb.filter.composite_filter
Expand Down
33 changes: 0 additions & 33 deletions gcloud/datastore/test_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,39 +109,6 @@ def test_get_entity_hit(self):
self.assertEqual(list(result), ['foo'])
self.assertEqual(result['foo'], 'Foo')

def test_get_entity_path(self):
from gcloud.datastore.connection import datastore_pb
DATASET_ID = 'DATASET'
KIND = 'Kind'
ID = 1234
PATH = [{'kind': KIND, 'id': ID}]
entity_pb = datastore_pb.Entity()
entity_pb.key.partition_id.dataset_id = DATASET_ID
path_element = entity_pb.key.path_element.add()
path_element.kind = KIND
path_element.id = ID
prop = entity_pb.property.add()
prop.name = 'foo'
prop.value.string_value = 'Foo'
connection = _Connection(entity_pb)
dataset = self._makeOne(DATASET_ID, connection)
result = dataset.get_entity([KIND, ID])
key = result.key()
self.assertEqual(key._dataset_id, DATASET_ID)
self.assertEqual(key.path(), PATH)
self.assertEqual(list(result), ['foo'])
self.assertEqual(result['foo'], 'Foo')

def test_get_entity_odd_nonetype(self):
DATASET_ID = 'DATASET'
KIND = 'Kind'
connection = _Connection()
dataset = self._makeOne(DATASET_ID, connection)
with self.assertRaises(ValueError):
dataset.get_entity([KIND])
with self.assertRaises(TypeError):
dataset.get_entity(None)

def test_get_entities_miss(self):
from gcloud.datastore.key import Key
DATASET_ID = 'DATASET'
Expand Down
43 changes: 7 additions & 36 deletions gcloud/datastore/test_key.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,39 +102,6 @@ def test_to_protobuf_w_explicit_path(self):
self.assertEqual(elems[2].name, '')
self.assertEqual(elems[2].id, 0)

def test_from_path_empty(self):
key = self._getTargetClass().from_path()
self.assertEqual(key._dataset_id, None)
self.assertEqual(key.namespace(), None)
self.assertEqual(key.kind(), '')
self.assertEqual(key.path(), [{'kind': ''}])

def test_from_path_single_element(self):
self.assertRaises(ValueError, self._getTargetClass().from_path, 'abc')

def test_from_path_three_elements(self):
self.assertRaises(ValueError, self._getTargetClass().from_path,
'abc', 'def', 'ghi')

def test_from_path_two_elements_second_string(self):
key = self._getTargetClass().from_path('abc', 'def')
self.assertEqual(key.kind(), 'abc')
self.assertEqual(key.path(), [{'kind': 'abc', 'name': 'def'}])

def test_from_path_two_elements_second_int(self):
key = self._getTargetClass().from_path('abc', 123)
self.assertEqual(key.kind(), 'abc')
self.assertEqual(key.path(), [{'kind': 'abc', 'id': 123}])

def test_from_path_nested(self):
key = self._getTargetClass().from_path('abc', 'def', 'ghi', 123)
self.assertEqual(key.kind(), 'ghi')
expected_path = [
{'kind': 'abc', 'name': 'def'},
{'kind': 'ghi', 'id': 123},
]
self.assertEqual(key.path(), expected_path)

def test_is_partial_no_name_or_id(self):
key = self._makeOne()
self.assertTrue(key.is_partial())
Expand Down Expand Up @@ -282,9 +249,13 @@ def test_parent_default(self):
self.assertEqual(key.parent(), None)

def test_parent_explicit_top_level(self):
key = self._getTargetClass().from_path('abc', 'def')
key = self._makeOne(path=[{'kind': 'abc', 'name': 'def'}])
self.assertEqual(key.parent(), None)

def test_parent_explicit_nested(self):
key = self._getTargetClass().from_path('abc', 'def', 'ghi', 123)
self.assertEqual(key.parent().path(), [{'kind': 'abc', 'name': 'def'}])
parent_part = {'kind': 'abc', 'name': 'def'}
key = self._makeOne(path=[
parent_part,
{'kind': 'ghi', 'id': 123},
])
self.assertEqual(key.parent().path(), [parent_part])
25 changes: 7 additions & 18 deletions gcloud/datastore/test_query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,10 @@ def test_filter_w_whitespace_property_name(self):
self.assertEqual(p_pb.value.string_value, u'John')
self.assertEqual(p_pb.operator, datastore_pb.PropertyFilter.EQUAL)

def test_ancestor_w_non_key_non_list(self):
def test_ancestor_w_non_key(self):
query = self._makeOne()
self.assertRaises(TypeError, query.ancestor, object())
self.assertRaises(TypeError, query.ancestor, ['KIND', 'NAME'])

def test_ancestor_wo_existing_ancestor_query_w_key_and_propfilter(self):
from gcloud.datastore.key import Key
Expand Down Expand Up @@ -208,39 +209,27 @@ def test_ancestor_wo_existing_ancestor_query_w_key(self):
self.assertEqual(p_pb.property.name, '__key__')
self.assertEqual(p_pb.value.key_value, key.to_protobuf())

def test_ancestor_wo_existing_ancestor_query_w_list(self):
def test_ancestor_clears_existing_ancestor_query_w_only(self):
from gcloud.datastore.key import Key
_KIND = 'KIND'
_ID = 123
key = Key(path=[{'kind': _KIND, 'id': _ID}])
query = self._makeOne()
after = query.ancestor([_KIND, _ID])
self.assertFalse(after is query)
self.assertTrue(isinstance(after, self._getTargetClass()))
q_pb = after.to_protobuf()
self.assertEqual(q_pb.filter.composite_filter.operator, 1) # AND
f_pb, = list(q_pb.filter.composite_filter.filter)
p_pb = f_pb.property_filter
self.assertEqual(p_pb.property.name, '__key__')
self.assertEqual(p_pb.value.key_value, key.to_protobuf())

def test_ancestor_clears_existing_ancestor_query_w_only(self):
_KIND = 'KIND'
_ID = 123
query = self._makeOne()
between = query.ancestor([_KIND, _ID])
between = query.ancestor(key)
after = between.ancestor(None)
self.assertFalse(after is query)
self.assertTrue(isinstance(after, self._getTargetClass()))
q_pb = after.to_protobuf()
self.assertEqual(list(q_pb.filter.composite_filter.filter), [])

def test_ancestor_clears_existing_ancestor_query_w_others(self):
from gcloud.datastore.key import Key
_KIND = 'KIND'
_ID = 123
_NAME = u'NAME'
key = Key(path=[{'kind': _KIND, 'id': _ID}])
query = self._makeOne().filter('name', '=', _NAME)
between = query.ancestor([_KIND, _ID])
between = query.ancestor(key)
after = between.ancestor(None)
self.assertFalse(after is query)
self.assertTrue(isinstance(after, self._getTargetClass()))
Expand Down
8 changes: 4 additions & 4 deletions pylintrc_default
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# PyLint config for 'gcloud' *library* code.
# PyLint config for 'gcloud' *library* code.
#
# NOTES:
#
Expand Down Expand Up @@ -65,14 +65,14 @@ ignore =
# DEFAULT: disable=
# RATIONALE:
# - maybe-no-member: bi-modal functions confuse pylint type inference.
# - no-member: indirections in protobuf-generated code
# - no-member: indirections in protobuf-generated code
# - protected-access: helpers use '_foo' of classes from generated code.
# - redefined-builtin: use of 'id', 'type', 'filter' args in API-bound funcs;
# use of 'NotImplemented' to map HTTP response code.
# - similarities: 'Bucket' and 'Key' define 'metageneration' and 'owner' with
# identical implementation but different docstrings.
# - star-args: standard Python idioms for varargs:
# ancestor = Key.from_path(*ancestor)
# ancestor = Query().filter(*order_props)
disable =
maybe-no-member,
no-member,
Expand Down Expand Up @@ -201,7 +201,7 @@ max-module-lines=1500

# Good variable names which should always be accepted, separated by a comma
# DEFAULT: good-names=i,j,k,ex,Run,_
# RATIONALE: 'pb' and 'id' have well-understood meainings in the code.
# RATIONALE: 'pb' and 'id' have well-understood meainings in the code.
good-names = i, j, k, ex, Run, _,
pb,
id,
Expand Down
4 changes: 2 additions & 2 deletions regression/datastore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def test_empty_kind(self):
class TestDatastoreSaveKeys(TestDatastore):

def test_save_key_self_reference(self):
key = datastore.key.Key.from_path('Person', 'name')
key = datastore.key.Key(path=[{'kind': 'Person', 'name': 'name'}])
entity = datastore.entity.Entity(kind=None).key(key)
entity['fullName'] = u'Full name'
entity['linkedTo'] = key # Self reference.
Expand Down Expand Up @@ -337,7 +337,7 @@ def test_query_group_by(self):
class TestDatastoreTransaction(TestDatastore):

def test_transaction(self):
key = datastore.key.Key.from_path('Company', 'Google')
key = datastore.key.Key(path=[{'kind': 'Company', 'name': 'Google'}])
entity = datastore.entity.Entity(kind=None).key(key)
entity['url'] = u'www.google.com'

Expand Down