Fix #141: remove spurious *args / **kwargs from API methods.

gcloud/datastore/key.py

@@ -11,6 +11,18 @@ class Key(object):
   """
 
   def __init__(self, dataset=None, namespace=None, path=None):
+    """Constructor / initializer for a key.
+
+    :type dataset: :class:`gcloud.datastore.dataset.Dataset`
+    :param dataset: A dataset instance for the key.
+
+    :type namespace: :class:`str`
+    :param namespace: A namespace identifier for the key.
+
+    :type path: sequence of dicts
+    :param path: Each dict must have keys 'kind' (a string) and optionally
+    'name' (a string) or 'id' (an integer).
+    """
     self._dataset = dataset
     self._namespace = namespace
     self._path = path or [{'kind': ''}]
@@ -21,13 +33,30 @@ def _clone(self):
     We make a shallow copy of the :class:`gcloud.datastore.dataset.Dataset`
     because it holds a reference an authenticated connection,
     which we don't want to lose.
+
+    :rtype: :class:`gcloud.datastore.key.Key`
+    :returns: a new `Key` instance
     """
     clone = copy.deepcopy(self)
     clone._dataset = self._dataset  # Make a shallow copy of the Dataset.
     return clone
 
   @classmethod
   def from_protobuf(cls, pb, dataset=None):
+    """Factory method for creating a key based on a protobuf.
+
+    The protobuf should be one returned from the Cloud Datastore Protobuf API.
+
+    :type pb: :class:`gcloud.datastore.datastore_v1_pb2.Key`
+    :param pb: The Protobuf representing the key.
+
+    :type dataset: :class:`gcloud.datastore.dataset.Dataset`
+    :param dataset: A dataset instance.  If not passed, defaults to an
+    instance whose ID is derived from pb.
+
+    :rtype: :class:`gcloud.datastore.key.Key`
+    :returns: a new `Key` instance
+    """
     path = []
     for element in pb.path_element:
       element_dict = {'kind': element.kind}
@@ -42,10 +71,18 @@ def from_protobuf(cls, pb, dataset=None):
 
     if not dataset:
       dataset = Dataset(id=pb.partition_id.dataset_id)
+      namespace = pb.partition_id.namespace
+    else:
+      namespace = None
 
-    return cls(path=path, dataset=dataset)
+    return cls(dataset, namespace, path)
 
   def to_protobuf(self):
+    """Return a protobuf corresponding to the key.
+
+    :rtype: :class:`gcloud.datastore.datastore_v1_pb2.Key`
+    :returns: The Protobuf representing the key.
+    """
     key = datastore_pb.Key()
 
     # Technically a dataset is required to do anything with the key,
@@ -77,6 +114,23 @@ def to_protobuf(self):
 
   @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 `__init__()`.
+
+    :rtype: :class:`gcloud.datastore.key.Key`
+    :returns: a new `Key` instance
+    """
+    if len(args) % 2:
+        raise ValueError('Must pass an even number of args.')
+
     path = []
     items = iter(args)
 
@@ -92,9 +146,25 @@ def from_path(cls, *args, **kwargs):
     return cls(**kwargs)
 
   def is_partial(self):
+    """Boolean test: is the key fully mapped onto a backend entity?
+
+    :rtype: :class:`bool`
+    :returns: True if the last element of the key's path does not have an 'id'
+    or a 'name'.
+    """
     return (self.id_or_name() is None)
 
   def dataset(self, dataset=None):
+    """Setter / getter.
+
+    :type dataset: :class:`gcloud.datastore.dataset.Dataset`
+    :param dataset: A dataset instance for the key.
+
+    :rtype: :class:`Key` (for setter); or
+            :class:`gcloud.datastore.dataset.Dataset` (for getter)
+    :returns: a new key, cloned from self., with the given dataset (setter);
+             or self's dataset (getter).
+    """
     if dataset:
       clone = self._clone()
       clone._dataset = dataset
@@ -103,6 +173,15 @@ def dataset(self, dataset=None):
       return self._dataset
 
   def namespace(self, namespace=None):
+    """Setter / getter.
+
+    :type namespace: :class:`str`
+    :param namespace: A namespace identifier for the key.
+
+    :rtype: :class:`Key` (for setter); or :class:`str` (for getter)
+    :returns: a new key, cloned from self., with the given namespace (setter);
+             or self's namespace (getter).
+    """
     if namespace:
       clone = self._clone()
       clone._namespace = namespace
@@ -111,6 +190,16 @@ def namespace(self, namespace=None):
       return self._namespace
 
   def path(self, path=None):
+    """Setter / getter.
+
+    :type path: sequence of dicts
+    :param path: Each dict must have keys 'kind' (a string) and optionally
+    'name' (a string) or 'id' (an integer).
+
+    :rtype: :class:`Key` (for setter); or :class:`str` (for getter)
+    :returns: a new key, cloned from self., with the given path (setter);
+             or self's path (getter).
+    """
     if path:
       clone = self._clone()
       clone._path = path
@@ -119,6 +208,15 @@ def path(self, path=None):
       return self._path
 
   def kind(self, kind=None):
+    """Setter / getter.  Based on the last element of path.
+
+    :type kind: :class:`str`
+    :param kind: The new kind for the key.
+
+    :rtype: :class:`Key` (for setter); or :class:`str` (for getter)
+    :returns: a new key, cloned from self., with the given kind (setter);
+             or self's kind (getter).
+    """
     if kind:
       clone = self._clone()
       clone._path[-1]['kind'] = kind
@@ -127,6 +225,15 @@ def kind(self, kind=None):
       return self._path[-1]['kind']
 
   def id(self, id=None):
+    """Setter / getter.  Based on the last element of path.
+
+    :type kind: :class:`str`
+    :param kind: The new kind for the key.
+
+    :rtype: :class:`Key` (for setter); or :class:`int` (for getter)
+    :returns: a new key, cloned from self., with the given id (setter);
+             or self's id (getter).
+    """
     if id:
       clone = self._clone()
       clone._path[-1]['id'] = id
@@ -135,6 +242,15 @@ def id(self, id=None):
       return self._path[-1].get('id')
 
   def name(self, name=None):
+    """Setter / getter.  Based on the last element of path.
+
+    :type kind: :class:`str`
+    :param kind: The new name for the key.
+
+    :rtype: :class:`Key` (for setter); or :class:`str` (for getter)
+    :returns: a new key, cloned from self., with the given name (setter);
+             or self's name (getter).
+    """
     if name:
       clone = self._clone()
       clone._path[-1]['name'] = name
@@ -143,11 +259,24 @@ def name(self, name=None):
       return self._path[-1].get('name')
 
   def id_or_name(self):
+    """Getter.  Based on the last element of path.
+
+    :rtype: :class:`int` (if 'id' is set); or :class:`str` (the 'name')
+    :returns: True if the last element of the key's path has either an 'id'
+    or a 'name'.
+    """
     return self.id() or self.name()
 
   def parent(self):#pragma NO COVER
-    # See https://github.com/GoogleCloudPlatform/gcloud-python/issues/135
-    raise NotImplementedError
+    """Getter:  return a new key for the next highest element in path.
+
+    :rtype: :class:`gcloud.datastore.key.Key`
+    :returns: a new `Key` instance, whose path consists of all but the last
+    element of self's path.  If self has only one path element, return None.
+    """
+    if len(self._path) <= 1:
+        return None
+    return self.path(self.path()[:-1])
 
   def __repr__(self): #pragma NO COVER
     return '<Key%s>' % self.path()

gcloud/datastore/test_key.py

@@ -80,12 +80,20 @@ def test_from_protobuf_w_dataset_in_pb(self):
         key = self._getTargetClass().from_protobuf(pb)
         self.assertEqual(key.dataset().id(), _DATASET)
 
-    def test_from_protobuf_w_namespace_in_pb(self):
+    def test_from_protobuf_w_namespace_in_pb_wo_dataset_passed(self):
         _NAMESPACE = 'NAMESPACE'
         pb = self._makePB(namespace=_NAMESPACE)
         key = self._getTargetClass().from_protobuf(pb)
-        # See: https://github.com/GoogleCloudPlatform/gcloud-python/issues/133
-        # self.assertEqual(key.namespace(), _NAMESPACE)  XXX
+        self.assertEqual(key.namespace(), _NAMESPACE)
+
+    def test_from_protobuf_w_namespace_in_pb_w_dataset_passed(self):
+        from gcloud.datastore.dataset import Dataset
+        _DATASET = 'DATASET'
+        _NAMESPACE = 'NAMESPACE'
+        dataset = Dataset(_DATASET)
+        pb = self._makePB(namespace=_NAMESPACE)
+        key = self._getTargetClass().from_protobuf(pb, dataset)
+        self.assertEqual(key.namespace(), None)
 
     def test_from_protobuf_w_path_in_pb(self):
         _DATASET = 'DATASET'
@@ -164,12 +172,11 @@ def test_from_path_empty(self):
         self.assertEqual(key.path(), [{'kind': ''}])
 
     def test_from_path_single_element(self):
-        # See https://github.com/GoogleCloudPlatform/gcloud-python/issues/134
-        key = self._getTargetClass().from_path('abc')
-        self.assertEqual(key.dataset(), None)
-        self.assertEqual(key.namespace(), None)
-        self.assertEqual(key.kind(), '') # XXX s.b. 'abc'?
-        self.assertEqual(key.path(), [{'kind': ''}]) # XXX s.b. 'abc'?
+        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')
@@ -339,6 +346,14 @@ def test_id_or_name_w_name_only(self):
         key = self._makeOne(path=_PATH)
         self.assertEqual(key.id_or_name(), _NAME)
 
-    def _ugh(self):
-        protokey = key.to_protobuf()
-        self.assertEqual(protokey.partition_id.dataset_id, _DATASET)
+    def test_parent_default(self):
+        key = self._makeOne()
+        self.assertEqual(key.parent(), None)
+
+    def test_parent_explicit_top_level(self):
+        key = self._getTargetClass().from_path('abc', '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'}])

gcloud/storage/bucket.py

@@ -1,3 +1,5 @@
+import os
+
 from gcloud.storage import exceptions
 from gcloud.storage.acl import BucketACL
 from gcloud.storage.acl import DefaultObjectACL
@@ -230,9 +232,56 @@ def upload_file(self, filename, key=None):
                 to the root of the bucket
                 with the same name as on your local file system.
     """
+    if key is None:
+        key = os.path.basename(filename)
     key = self.new_key(key)
     return key.set_contents_from_filename(filename)
 
+  def upload_file_object(self, fh, key=None):
+    # TODO: What do we do about overwriting data?
+    """Shortcut method to upload a file into this bucket.
+
+    Use this method to quickly put a local file in Cloud Storage.
+
+    For example::
+
+      >>> from gcloud import storage
+      >>> connection = storage.get_connection(project, email, key_path)
+      >>> bucket = connection.get_bucket('my-bucket')
+      >>> bucket.upload_file(open('~/my-file.txt'), 'remote-text-file.txt')
+      >>> print bucket.get_all_keys()
+      [<Key: my-bucket, remote-text-file.txt>]
+
+    If you don't provide a key value,
+    we will try to upload the file using the local filename
+    as the key
+    (**not** the complete path)::
+
+      >>> from gcloud import storage
+      >>> connection = storage.get_connection(project, email, key_path)
+      >>> bucket = connection.get_bucket('my-bucket')
+      >>> bucket.upload_file(open('~/my-file.txt'))
+      >>> print bucket.get_all_keys()
+      [<Key: my-bucket, my-file.txt>]
+
+    :type fh: file
+    :param fh: A file handle open for reading.
+
+    :type key: string or :class:`gcloud.storage.key.Key`
+    :param key: The key (either an object or a remote path)
+                of where to put the file.
+
+                If this is blank,
+                we will try to upload the file
+                to the root of the bucket
+                with the same name as on your local file system.
+    """
+    if key:
+      key = self.new_key(key)
+    else:
+      key = self.new_key(os.path.basename(fh.name))
+    return key.set_contents_from_file(fh)
+
   def has_metadata(self, field=None):
     """Check if metadata is available locally.
 
@@ -443,7 +492,9 @@ def save_acl(self, acl=None):
     if acl is None:
       return self
 
-    return self.patch_metadata({'acl': list(acl)})
+    self.patch_metadata({'acl': list(acl)})
+    self.reload_acl()
+    return self
 
   def clear_acl(self):
     """Remove all ACL rules from the bucket.
@@ -517,12 +568,15 @@ def save_default_object_acl(self, acl=None):
                 and save that.
     """
 
-    acl = acl or self.default_object_acl
+    if acl is None:
+        acl = self.default_object_acl
 
     if acl is None:
       return self
 
-    return self.patch_metadata({'defaultObjectAcl': list(acl)})
+    self.patch_metadata({'defaultObjectAcl': list(acl)})
+    self.reload_default_object_acl()
+    return self
 
   def clear_default_object_acl(self):
     """Remove the Default Object ACL from this bucket."""