Adding pylintrc config and custom runner and making it pass.

Some key things to notice:
- The tools (pep8 / pylint) disagree about continutation lines.
  This is because PEP8 is ambiguous about "multi-line constructs".
  One of the examples for bracket indentation in the PEP is

      my_list = [
          1, 2, 3,
          4, 5, 6,
          ]

  but the PEP doesn't explicity say whether or not

      my_list = [1, 2, 3,
                 4, 5, 6,
                 ]

  would be allowed. As it turns out, pep8 (the tool) considers
  the latter to be a continuation while pylint does not.
- I changed all indentation where this was a problem, but was
  intentionally inconsistent. This way we can see "all options"
  and discuss whichever is preferred.
- I made a _MetadataMixin helper class for methods shared between
  storage.Bucket and storage.Key. I did not refactor the affected
  tests but they do still give 100% coverage.
- To silence docstrings warnings I added many bogus docstrings
  without actually documenting the given objects.
- I factored out part of
      storage.connection.Connection.generate_signed_url
  into a _get_expiration_seconds() method and added tests.
  This method is untested (has pragma no cover) and is still very long.
  I refactored because pylint complained about too many locals.
- I made sure tox -e lint and tox -e cover passed.
- I increased the maximum number of arguments from 5 to 10 and
  the maximum public methods from 20 to 30. Offenders were:
    - storage.connection.Connection.make_request (6 arguments)
    - storage.connection.Connection.api_request (9 arguments)
    - storage.connection.Connection.generate_signed_url (6 arguments)
    - storage.bucket.Bucket (27 public methods) and
    - storage.key.Key (23 public methods)
  I think refactoring these should be considered (though is not
  mandatory). I certainly don't know a good solution for the class
  public methods.

Author: dhermes

gcloud/__init__.py

@@ -1 +1,4 @@
+"""Totally unnecessary docstring."""
+
+
 __version__ = '0.02.2'

gcloud/connection.py

@@ -1,3 +1,5 @@
+"""Need module docstring."""
+
 from pkg_resources import get_distribution
 
 import httplib2
@@ -27,10 +29,12 @@ def __init__(self, credentials=None):
         :type credentials: :class:`oauth2client.client.OAuth2Credentials`
         :param credentials: The OAuth2 Credentials to use for this connection.
         """
+        self._http = None
         self._credentials = credentials
 
     @property
     def credentials(self):
+        """Needs docstring."""
         return self._credentials
 
     @property
@@ -40,7 +44,7 @@ def http(self):
         :rtype: :class:`httplib2.Http`
         :returns: A Http object used to transport data.
         """
-        if not hasattr(self, '_http'):
+        if self._http is None:
             self._http = httplib2.Http()
             if self._credentials:
                 self._http = self._credentials.authorize(self._http)

gcloud/datastore/_helpers.py

@@ -88,32 +88,32 @@ def _get_value_from_protobuf(pb):
     :returns: The value provided by the Protobuf.
     """
 
+    result = None
     if pb.value.HasField('timestamp_microseconds_value'):
         microseconds = pb.value.timestamp_microseconds_value
         naive = (datetime.utcfromtimestamp(0) +
                  timedelta(microseconds=microseconds))
-        return naive.replace(tzinfo=pytz.utc)
+        result = naive.replace(tzinfo=pytz.utc)
 
     elif pb.value.HasField('key_value'):
-        return Key.from_protobuf(pb.value.key_value)
+        result = Key.from_protobuf(pb.value.key_value)
 
     elif pb.value.HasField('boolean_value'):
-        return pb.value.boolean_value
+        result = pb.value.boolean_value
 
     elif pb.value.HasField('double_value'):
-        return pb.value.double_value
+        result = pb.value.double_value
 
     elif pb.value.HasField('integer_value'):
-        return pb.value.integer_value
+        result = pb.value.integer_value
 
     elif pb.value.HasField('string_value'):
-        return pb.value.string_value
+        result = pb.value.string_value
 
     elif pb.value.HasField('entity_value'):
-        return Entity.from_protobuf(pb.value.entity_value)
+        result = Entity.from_protobuf(pb.value.entity_value)
 
-    else:
-        return None
+    return result
 
 
 def _set_protobuf_value(value_pb, val):
@@ -142,9 +142,9 @@ def _set_protobuf_value(value_pb, val):
         key = val.key()
         if key is not None:
             e_pb.key.CopyFrom(key.to_protobuf())
-        for k, v in val.items():
+        for item_key, value in val.iteritems():
             p_pb = e_pb.property.add()
-            p_pb.name = k
-            _set_protobuf_value(p_pb.value, v)
+            p_pb.name = item_key
+            _set_protobuf_value(p_pb.value, value)
     else:  # scalar, just assign
         setattr(value_pb, attr, val)

gcloud/datastore/connection.py

@@ -1,3 +1,5 @@
+"""Needs module docstring."""
+
 from gcloud import connection
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 from gcloud.datastore import _helpers
@@ -22,7 +24,7 @@ class Connection(connection.Connection):
     """A template for the URL of a particular API call."""
 
     def __init__(self, credentials=None):
-        self._credentials = credentials
+        super(Connection, self).__init__(credentials=credentials)
         self._current_transaction = None
 
     def _request(self, dataset_id, method, data):
@@ -59,6 +61,7 @@ def _request(self, dataset_id, method, data):
         return content
 
     def _rpc(self, dataset_id, method, request_pb, response_pb_cls):
+        """Needs docstring."""
         response = self._request(dataset_id=dataset_id, method=method,
                                  data=request_pb.SerializeToString())
         return response_pb_cls.FromString(response)
@@ -94,13 +97,15 @@ def build_api_url(cls, dataset_id, method, base_url=None,
             dataset_id=dataset_id, method=method)
 
     def transaction(self, transaction=connection.Connection._EMPTY):
+        """Needs docstring."""
         if transaction is self._EMPTY:
             return self._current_transaction
         else:
             self._current_transaction = transaction
             return self
 
     def mutation(self):
+        """Needs docstring."""
         if self.transaction():
             return self.transaction().mutation()
         else:
@@ -210,8 +215,7 @@ def run_query(self, dataset_id, query_pb, namespace=None):
         return ([e.entity for e in response.batch.entity_result],
                 response.batch.end_cursor,
                 response.batch.more_results,
-                response.batch.skipped_results,
-                )
+                response.batch.skipped_results)
 
     def lookup(self, dataset_id, key_pbs):
         """Lookup keys from a dataset in the Cloud Datastore.
@@ -278,6 +282,7 @@ def lookup(self, dataset_id, key_pbs):
         return results
 
     def commit(self, dataset_id, mutation_pb):
+        """Needs docstring."""
         request = datastore_pb.CommitRequest()
 
         if self.transaction():
@@ -365,4 +370,5 @@ def delete_entities(self, dataset_id, key_pbs):
             return self.commit(dataset_id, mutation)
 
     def delete_entity(self, dataset_id, key_pb):
+        """Needs docstring."""
         return self.delete_entities(dataset_id, [key_pb])

gcloud/datastore/dataset.py

@@ -1,3 +1,6 @@
+"""Needs module docstring."""
+
+
 class Dataset(object):
     """A dataset in the Cloud Datastore.
 
@@ -58,15 +61,21 @@ def id(self):
         return self._id
 
     def query(self, *args, **kwargs):
+        """Needs docstring."""
+        # This import is here to avoid circular references.
         from gcloud.datastore.query import Query
         kwargs['dataset'] = self
         return Query(*args, **kwargs)
 
     def entity(self, kind):
+        """Needs docstring."""
+        # This import is here to avoid circular references.
         from gcloud.datastore.entity import Entity
         return Entity(dataset=self, kind=kind)
 
     def transaction(self, *args, **kwargs):
+        """Needs docstring."""
+        # This import is here to avoid circular references.
         from gcloud.datastore.transaction import Transaction
         kwargs['dataset'] = self
         return Transaction(*args, **kwargs)
@@ -86,6 +95,7 @@ def get_entity(self, key):
             return entities[0]
 
     def get_entities(self, keys):
+        """Needs docstring."""
         # This import is here to avoid circular references.
         from gcloud.datastore.entity import Entity
 

gcloud/datastore/key.py

@@ -1,3 +1,5 @@
+"""Needs module docstring."""
+
 import copy
 from itertools import izip
 
@@ -157,8 +159,9 @@ def is_partial(self):
         :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)
+        return self.id_or_name() is None
 
+    # NOTE: This is identical to datastore.query.Query.dataset.
     def dataset(self, dataset=None):
         """Dataset setter / getter.
 
@@ -229,19 +232,19 @@ def kind(self, kind=None):
         elif self.path():
             return self._path[-1]['kind']
 
-    def id(self, id=None):
+    def id(self, id_to_set=None):
         """ID setter / getter.  Based on the last element of path.
 
-        :type kind: :class:`str`
-        :param kind: The new kind for the key.
+        :type id_to_set: :class:`int`
+        :param id_to_set: The new ID 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).
+                  or self's id (getter).
         """
-        if id:
+        if id_to_set:
             clone = self._clone()
-            clone._path[-1]['id'] = id
+            clone._path[-1]['id'] = id_to_set
             return clone
         elif self.path():
             return self._path[-1].get('id')

gcloud/datastore/query.py

@@ -1,3 +1,5 @@
+"""Needs module docstring."""
+
 import base64
 import copy
 
@@ -60,6 +62,7 @@ def __init__(self, kind=None, dataset=None):
             self._pb.kind.add().name = kind
 
     def _clone(self):
+        """Needs docstring."""
         clone = copy.deepcopy(self)
         clone._dataset = self._dataset  # Shallow copy the dataset.
         return clone
@@ -308,11 +311,9 @@ def fetch(self, limit=None):
         if limit:
             clone = self.limit(limit)
 
-        (entity_pbs,
-         end_cursor,
-         more_results,
-         skipped_results) = self.dataset().connection().run_query(
+        query_results = self.dataset().connection().run_query(
             query_pb=clone.to_protobuf(), dataset_id=self.dataset().id())
+        entity_pbs, end_cursor = query_results[:2]
 
         self._cursor = end_cursor
         return [Entity.from_protobuf(entity, dataset=self.dataset())
@@ -372,14 +373,14 @@ def order(self, *properties):
         """
         clone = self._clone()
 
-        for p in properties:
+        for prop in properties:
             property_order = clone._pb.order.add()
 
-            if p.startswith('-'):
-                property_order.property.name = p[1:]
+            if prop.startswith('-'):
+                property_order.property.name = prop[1:]
                 property_order.direction = property_order.DESCENDING
             else:
-                property_order.property.name = p
+                property_order.property.name = prop
                 property_order.direction = property_order.ASCENDING
 
         return clone

gcloud/datastore/test___init__.py

@@ -3,7 +3,8 @@
 
 class Test_get_connection(unittest2.TestCase):
 
-    def _callFUT(self, client_email, private_key_path):
+    @staticmethod
+    def _callFUT(client_email, private_key_path):
         from gcloud.datastore import get_connection
         return get_connection(client_email, private_key_path)
 
@@ -25,16 +26,16 @@ def test_it(self):
                 found = self._callFUT(CLIENT_EMAIL, f.name)
         self.assertTrue(isinstance(found, Connection))
         self.assertTrue(found._credentials is client._signed)
-        self.assertEqual(client._called_with,
-                         {'service_account_name': CLIENT_EMAIL,
-                          'private_key': PRIVATE_KEY,
-                          'scope': SCOPE,
-                          })
+        expected_called_with = {'service_account_name': CLIENT_EMAIL,
+                                'private_key': PRIVATE_KEY,
+                                'scope': SCOPE}
+        self.assertEqual(client._called_with, expected_called_with)
 
 
 class Test_get_dataset(unittest2.TestCase):
 
-    def _callFUT(self, dataset_id, client_email, private_key_path):
+    @staticmethod
+    def _callFUT(dataset_id, client_email, private_key_path):
         from gcloud.datastore import get_dataset
         return get_dataset(dataset_id, client_email, private_key_path)
 
@@ -59,8 +60,7 @@ def test_it(self):
         self.assertTrue(isinstance(found, Dataset))
         self.assertTrue(isinstance(found.connection(), Connection))
         self.assertEqual(found.id(), DATASET_ID)
-        self.assertEqual(client._called_with,
-                         {'service_account_name': CLIENT_EMAIL,
-                          'private_key': PRIVATE_KEY,
-                          'scope': SCOPE,
-                          })
+        expected_called_with = {'service_account_name': CLIENT_EMAIL,
+                                'private_key': PRIVATE_KEY,
+                                'scope': SCOPE}
+        self.assertEqual(client._called_with, expected_called_with)

gcloud/datastore/test__helpers.py

@@ -3,7 +3,8 @@
 
 class Test__get_protobuf_attribute_and_value(unittest2.TestCase):
 
-    def _callFUT(self, val):
+    @staticmethod
+    def _callFUT(val):
         from gcloud.datastore._helpers import _get_protobuf_attribute_and_value
 
         return _get_protobuf_attribute_and_value(val)
@@ -96,12 +97,14 @@ def test_object(self):
 
 class Test__get_value_from_protobuf(unittest2.TestCase):
 
-    def _callFUT(self, pb):
+    @staticmethod
+    def _callFUT(pb):
         from gcloud.datastore._helpers import _get_value_from_protobuf
 
         return _get_value_from_protobuf(pb)
 
-    def _makePB(self, attr_name, value):
+    @staticmethod
+    def _makePB(attr_name, value):
         from gcloud.datastore.datastore_v1_pb2 import Property
 
         prop = Property()
@@ -170,17 +173,19 @@ def test_unknown(self):
         from gcloud.datastore.datastore_v1_pb2 import Property
 
         pb = Property()
-        self.assertEqual(self._callFUT(pb), None)  # XXX desirable?
+        self.assertEqual(self._callFUT(pb), None)
 
 
 class Test_set_protobuf_value(unittest2.TestCase):
 
-    def _callFUT(self, value_pb, val):
+    @staticmethod
+    def _callFUT(value_pb, val):
         from gcloud.datastore._helpers import _set_protobuf_value
 
         return _set_protobuf_value(value_pb, val)
 
-    def _makePB(self):
+    @staticmethod
+    def _makePB():
         from gcloud.datastore.datastore_v1_pb2 import Value
 
         return Value()