From aaee90831b7c3b9dc8e3c7930ac864c39eaecd0e Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Wed, 24 Jun 2015 16:23:50 -0700 Subject: [PATCH] Adding Pylint plugin to check Sphinx docstrings. Also - Updated docstrings which don't pass / are invalid. - Integrated plugin with `tox lint` rule and `run_pylint`. For now, pulling directly from the mercurial source repository to get the `check_docs` pylint extension. The maintainter mentioned this will be released July 15-17, 2015. See: http://www.bitbucket.org/logilab/pylint/pull-request/143/ --- gcloud/connection.py | 12 +++++++++ gcloud/credentials.py | 11 +++----- gcloud/datastore/_implicit_environ.py | 4 +++ gcloud/datastore/batch.py | 28 +++++++++---------- gcloud/datastore/connection.py | 3 +++ gcloud/datastore/key.py | 39 ++++++++++++++------------- gcloud/iterator.py | 3 +++ gcloud/pubsub/message.py | 9 ++++--- gcloud/pubsub/topic.py | 4 +-- gcloud/storage/_helpers.py | 15 +++++++---- gcloud/storage/_implicit_environ.py | 4 +++ gcloud/storage/acl.py | 26 +++++++++--------- gcloud/storage/api.py | 3 +++ gcloud/storage/batch.py | 12 +++++++++ gcloud/storage/blob.py | 7 ++--- gcloud/storage/bucket.py | 3 --- pylintrc_default | 3 +++ run_pylint.py | 1 + tox.ini | 2 +- 19 files changed, 116 insertions(+), 73 deletions(-) diff --git a/gcloud/connection.py b/gcloud/connection.py index 392ed9347252..bc6505d45519 100644 --- a/gcloud/connection.py +++ b/gcloud/connection.py @@ -127,6 +127,12 @@ def from_service_account_json(cls, json_credentials_path, *args, **kwargs): other credentials information (downloaded from the Google APIs console). + :type args: tuple + :param args: Remaining positional arguments to pass to constructor. + + :type kwargs: dictionary + :param kwargs: Remaining keyword arguments to pass to constructor. + :rtype: :class:`gcloud.connection.Connection` :returns: The connection created with the retrieved JSON credentials. """ @@ -153,6 +159,12 @@ def from_service_account_p12(cls, client_email, private_key_path, given to you when you created the service account). This file must be in P12 format. + :type args: tuple + :param args: Remaining positional arguments to pass to constructor. + + :type kwargs: dictionary + :param kwargs: Remaining keyword arguments to pass to constructor. + :rtype: :class:`gcloud.connection.Connection` :returns: The connection created with the retrieved P12 credentials. """ diff --git a/gcloud/credentials.py b/gcloud/credentials.py index 483e58ecea5c..3fdcd39e2280 100644 --- a/gcloud/credentials.py +++ b/gcloud/credentials.py @@ -208,20 +208,17 @@ def _get_service_account_name(credentials): :param credentials: The credentials used to determine the service account name. - :type string_to_sign: string - :param string_to_sign: The string to be signed by the credentials. - - :rtype: bytes - :returns: Signed bytes produced by the credentials. + :rtype: string + :returns: Service account name associated with the credentials. :raises: :class:`ValueError` if the credentials are not a valid service - account type + account type. """ service_account_name = None if isinstance(credentials, client.SignedJwtAssertionCredentials): service_account_name = credentials.service_account_name elif isinstance(credentials, service_account._ServiceAccountCredentials): service_account_name = credentials._service_account_email - elif _GAECreds is not None and isinstance(credentials, _GAECreds): + elif isinstance(credentials, _GAECreds): service_account_name = app_identity.get_service_account_name() if service_account_name is None: diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py index fd5ba9e5b8f9..7dae6554cb67 100644 --- a/gcloud/datastore/_implicit_environ.py +++ b/gcloud/datastore/_implicit_environ.py @@ -151,6 +151,10 @@ class _DefaultsContainer(object): :type dataset_id: string :param dataset_id: Persistent implied dataset ID from environment. + + :type implicit: boolean + :param implicit: Boolean indicating if the container should allow + implicit properties. """ @_lazy_property_deco diff --git a/gcloud/datastore/batch.py b/gcloud/datastore/batch.py index 8c5d4d900ebb..8bc0084192bb 100644 --- a/gcloud/datastore/batch.py +++ b/gcloud/datastore/batch.py @@ -61,21 +61,19 @@ class Batch(object): >>> with Batch() as batch: ... do_some_work(batch) ... raise Exception() # rolls back - """ - _id = None # "protected" attribute, always None for non-transactions - def __init__(self, dataset_id=None, connection=None): - """Construct a batch. + :type dataset_id: :class:`str`. + :param dataset_id: The ID of the dataset. - :type dataset_id: :class:`str`. - :param dataset_id: The ID of the dataset. + :type connection: :class:`gcloud.datastore.connection.Connection` + :param connection: The connection used to connect to datastore. - :type connection: :class:`gcloud.datastore.connection.Connection` - :param connection: The connection used to connect to datastore. + :raises: :class:`ValueError` if either a connection or dataset ID + are not set. + """ + _id = None # "protected" attribute, always None for non-transactions - :raises: :class:`ValueError` if either a connection or dataset ID - are not set. - """ + def __init__(self, dataset_id=None, connection=None): self._connection = (connection or _implicit_environ.get_default_connection()) self._dataset_id = (dataset_id or @@ -254,14 +252,14 @@ def _assign_entity_to_mutation(mutation_pb, entity, auto_id_entities): Helper method for ``Batch.put``. :type mutation_pb: :class:`gcloud.datastore._datastore_v1_pb2.Mutation` - :param mutation_pb; the Mutation protobuf for the batch / transaction. + :param mutation_pb: The Mutation protobuf for the batch / transaction. :type entity: :class:`gcloud.datastore.entity.Entity` - :param entity; the entity being updated within the batch / transaction. + :param entity: The entity being updated within the batch / transaction. :type auto_id_entities: list of :class:`gcloud.datastore.entity.Entity` - :param auto_id_entities: entiites with partial keys, to be fixed up - during commit. + :param auto_id_entities: Entities with partial keys, to be fixed up + during commit. """ auto_id = entity.key.is_partial diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py index 64e48f3b9bd0..576deb50c8b1 100644 --- a/gcloud/datastore/connection.py +++ b/gcloud/datastore/connection.py @@ -37,6 +37,9 @@ class Connection(connection.Connection): :type credentials: :class:`oauth2client.client.OAuth2Credentials` :param credentials: The OAuth2 Credentials to use for this connection. + :type http: :class:`httplib2.Http` or class that defines ``request()``. + :param http: An optional HTTP object to make requests. + :type api_base_url: string :param api_base_url: The base of the API call URL. Defaults to the value from :mod:`gcloud.connection`. diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py index f075ce0187f7..d1064bb9017f 100644 --- a/gcloud/datastore/key.py +++ b/gcloud/datastore/key.py @@ -43,29 +43,22 @@ class Key(object): >>> Key('Parent', 'foo', 'Child') - .. automethod:: __init__ - """ - - def __init__(self, *path_args, **kwargs): - """Constructor / initializer for a key. + :type path_args: tuple of string and integer + :param path_args: May represent a partial (odd length) or full (even + length) key path. - :type path_args: tuple of string and integer - :param path_args: May represent a partial (odd length) or full (even - length) key path. + :type kwargs: dictionary + :param kwargs: Keyword arguments to be passed in. - :type namespace: string - :param namespace: A namespace identifier for the key. Can only be - passed as a keyword argument. + Accepted keyword arguments are + * namespace (string): A namespace identifier for the key. + * dataset_id (string): The dataset ID associated with the key. + * parent (:class:`gcloud.datastore.key.Key`): The parent of the key. - :type dataset_id: string - :param dataset_id: The dataset ID associated with the key. Required, - unless the implicit dataset ID has been set. Can - only be passed as a keyword argument. + The dataset ID argument is required unless it has been set implicitly. + """ - :type parent: :class:`gcloud.datastore.key.Key` - :param parent: The parent of the key. Can only be passed as a - keyword argument. - """ + def __init__(self, *path_args, **kwargs): self._flat_path = path_args parent = self._parent = kwargs.get('parent') self._namespace = kwargs.get('namespace') @@ -393,6 +386,14 @@ def _validate_dataset_id(dataset_id, parent): If ``dataset_id`` is unset, attempt to infer the ID from the environment. + :type dataset_id: string + :param dataset_id: A dataset ID. + + :type parent: :class:`gcloud.datastore.key.Key` or ``NoneType`` + :param parent: The parent of the key or ``None``. + + :rtype: string + :returns: The ``dataset_id`` passed in, or implied from the environment. :raises: :class:`ValueError` if ``dataset_id`` is ``None`` and no dataset can be inferred. """ diff --git a/gcloud/iterator.py b/gcloud/iterator.py index 0ec3082cfbf3..1adf9201ef43 100644 --- a/gcloud/iterator.py +++ b/gcloud/iterator.py @@ -53,6 +53,9 @@ class Iterator(object): :type path: string :param path: The path to query for the list of items. + + :type extra_params: dict or None + :param extra_params: Extra query string parameters for the API call. """ PAGE_TOKEN = 'pageToken' diff --git a/gcloud/pubsub/message.py b/gcloud/pubsub/message.py index b01c0e9c29b4..aff2da167cf5 100644 --- a/gcloud/pubsub/message.py +++ b/gcloud/pubsub/message.py @@ -28,14 +28,15 @@ class Message(object): See: https://cloud.google.com/pubsub/reference/rest/google/pubsub/v1beta2/PubsubMessage - :type name: bytes - :param name: the payload of the message + :type data: bytes + :param data: the payload of the message :type message_id: string :param message_id: An ID assigned to the message by the API. - :type attrs: dict or None - :param attrs: Extra metadata associated by the publisher with the message. + :type attributes: dict or None + :param attributes: Extra metadata associated by the publisher with the + message. """ def __init__(self, data, message_id, attributes=None): self.data = data diff --git a/gcloud/pubsub/topic.py b/gcloud/pubsub/topic.py index 9a0af105747d..9cde5cb33db0 100644 --- a/gcloud/pubsub/topic.py +++ b/gcloud/pubsub/topic.py @@ -156,7 +156,7 @@ def publish(self, message, client=None, **attrs): ``client`` stored on the current topic. :type attrs: dict (string -> string) - :message attrs: key-value pairs to send as message attributes + :param attrs: key-value pairs to send as message attributes :rtype: str :returns: message ID assigned by the server to the published message @@ -232,7 +232,7 @@ def publish(self, message, **attrs): :param message: the message payload :type attrs: dict (string -> string) - :message attrs: key-value pairs to send as message attributes + :param attrs: key-value pairs to send as message attributes """ self.topic._timestamp_message(attrs) self.messages.append( diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py index a5acb45c645e..44433335368a 100644 --- a/gcloud/storage/_helpers.py +++ b/gcloud/storage/_helpers.py @@ -30,6 +30,9 @@ class _PropertyMixin(object): Non-abstract subclasses should implement: - connection - path + + :type name: string + :param name: The name of the object. """ @property @@ -38,11 +41,6 @@ def path(self): raise NotImplementedError def __init__(self, name=None): - """_PropertyMixin constructor. - - :type name: string - :param name: The name of the object. - """ self.name = name self._properties = {} self._changes = set() @@ -156,6 +154,13 @@ def _write_buffer_to_hash(buffer_object, hash_obj, digest_block_size=8192): :type buffer_object: bytes buffer :param buffer_object: Buffer containing bytes used to update a hash object. + + :type hash_obj: object that implements update + :param hash_obj: A hash object (MD5 or CRC32-C). + + :type digest_block_size: integer + :param digest_block_size: The block size to write to the hash. + Defaults to 8192. """ block = buffer_object.read(digest_block_size) diff --git a/gcloud/storage/_implicit_environ.py b/gcloud/storage/_implicit_environ.py index b90f99ed67b8..3b75232a28d8 100644 --- a/gcloud/storage/_implicit_environ.py +++ b/gcloud/storage/_implicit_environ.py @@ -31,6 +31,10 @@ class _DefaultsContainer(object): :type connection: :class:`gcloud.storage.connection.Connection` :param connection: Persistent implied connection from environment. + + :type implicit: boolean + :param implicit: Boolean indicating if the container should allow + implicit properties. """ @_lazy_property_deco diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py index 84e3c7911876..45725d12afdc 100644 --- a/gcloud/storage/acl.py +++ b/gcloud/storage/acl.py @@ -86,6 +86,13 @@ class _ACLEntity(object): This is a helper class that you likely won't ever construct outside of using the factor methods on the :class:`ACL` object. + + :type entity_type: string + :param entity_type: The type of entity (ie, 'group' or 'user'). + + :type identifier: string + :param identifier: The ID or e-mail of the entity. For the special + entity types (like 'allUsers') this is optional. """ READER_ROLE = 'READER' @@ -93,15 +100,6 @@ class _ACLEntity(object): OWNER_ROLE = 'OWNER' def __init__(self, entity_type, identifier=None): - """Entity constructor. - - :type entity_type: string - :param entity_type: The type of entity (ie, 'group' or 'user'). - - :type identifier: string - :param identifier: The ID or e-mail of the entity. For the special - entity types (like 'allUsers') this is optional. - """ self.identifier = identifier self.roles = set([]) self.type = entity_type @@ -416,13 +414,13 @@ def clear(self, connection=None): class BucketACL(ACL): - """An ACL specifically for a bucket.""" + """An ACL specifically for a bucket. + + :type bucket: :class:`gcloud.storage.bucket.Bucket` + :param bucket: The bucket to which this ACL relates. + """ def __init__(self, bucket): - """ - :type bucket: :class:`gcloud.storage.bucket.Bucket` - :param bucket: The bucket to which this ACL relates. - """ super(BucketACL, self).__init__() self.bucket = bucket diff --git a/gcloud/storage/api.py b/gcloud/storage/api.py index 6155145499e3..a0eb967a30fe 100644 --- a/gcloud/storage/api.py +++ b/gcloud/storage/api.py @@ -207,6 +207,9 @@ class _BucketIterator(Iterator): :type connection: :class:`gcloud.storage.connection.Connection` :param connection: The connection to use for querying the list of buckets. + + :type extra_params: dict or ``NoneType`` + :param extra_params: Extra query string parameters for the API call. """ def __init__(self, connection, extra_params=None): diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py index 151e99a18888..3d6806fb603e 100644 --- a/gcloud/storage/batch.py +++ b/gcloud/storage/batch.py @@ -40,6 +40,12 @@ class MIMEApplicationHTTP(MIMEApplication): Constructs payload from headers and body + :type method: string + :param method: HTTP method + + :type uri: string + :param uri: URI for HTTP request + :type headers: dict :param headers: HTTP headers @@ -155,6 +161,12 @@ def _do_request(self, method, url, headers, data, target_object): :type data: string :param data: The data to send as the body of the request. + :type target_object: object or :class:`NoneType` + :param target_object: This allows us to enable custom behavior in our + batch connection. Here we defer an HTTP request + and complete initialization of the object at a + later time. + :rtype: tuple of ``response`` (a dictionary of sorts) and ``content`` (a string). :returns: The HTTP response object and the content of the response. diff --git a/gcloud/storage/blob.py b/gcloud/storage/blob.py index a5e1201ee353..48dedf409310 100644 --- a/gcloud/storage/blob.py +++ b/gcloud/storage/blob.py @@ -57,9 +57,6 @@ class Blob(_PropertyMixin): :param chunk_size: The size of a chunk of data whenever iterating (1 MB). This must be a multiple of 256 KB per the API specification. - - :type properties: dict - :param properties: All the other data provided by Cloud Storage. """ _chunk_size = None # Default value for each instance. @@ -507,6 +504,10 @@ def upload_from_string(self, data, content_type='text/plain', :param data: The data to store in this blob. If the value is text, it will be encoded as UTF-8. + :type content_type: string + :param content_type: Optional type of content being uploaded. Defaults + to ``'text/plain'``. + :type connection: :class:`gcloud.storage.connection.Connection` or ``NoneType`` :param connection: Optional. The connection to use when sending diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py index f6bd8ff564d4..02d66d9561a8 100644 --- a/gcloud/storage/bucket.py +++ b/gcloud/storage/bucket.py @@ -78,9 +78,6 @@ class Bucket(_PropertyMixin): :type name: string :param name: The name of the bucket. - - :type properties: dictionary or ``NoneType`` - :param properties: The properties associated with the bucket. """ _iterator_class = _BlobIterator diff --git a/pylintrc_default b/pylintrc_default index 9acb94e94049..11c7b3a77768 100644 --- a/pylintrc_default +++ b/pylintrc_default @@ -38,6 +38,9 @@ ignore = # List of plugins (as comma separated values of python modules names) to load, # usually to register additional checkers. # DEFAULT: load-plugins= +# RATIONALE: We want to make sure our docstrings match the objects +# they document. +load-plugins=pylint.extensions.check_docs # DEPRECATED # DEFAULT: include-ids=no diff --git a/run_pylint.py b/run_pylint.py index efefcb1bb0fe..4cdb0435a735 100644 --- a/run_pylint.py +++ b/run_pylint.py @@ -81,6 +81,7 @@ def make_test_rc(base_rc_filename, additions_dict, target_filename): curr_val = curr_section.get(opt) if curr_val is None: raise KeyError('Expected to be adding to existing option.') + curr_val = curr_val.rstrip(',') curr_section[opt] = '%s, %s' % (curr_val, opt_val) with open(target_filename, 'w') as file_obj: diff --git a/tox.ini b/tox.ini index 7ba64253c22b..38f020bf8ee9 100644 --- a/tox.ini +++ b/tox.ini @@ -53,7 +53,7 @@ commands = python run_pylint.py deps = pep8 - pylint + -ehg+https://bitbucket.org/logilab/pylint#egg=pylint unittest2 protobuf==3.0.0-alpha-1 passenv = {[testenv:regression]passenv}