Skip to content

Commit

Permalink
Fixed long lines and some more pep8 errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
Kevin Leyow authored and Kevin Leyow committed Sep 28, 2014
1 parent f4902a8 commit b8a989f
Show file tree
Hide file tree
Showing 13 changed files with 54 additions and 39 deletions.
3 changes: 2 additions & 1 deletion gcloud/datastore/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ def get_connection(client_email, private_key_path):


def get_dataset(dataset_id, client_email, private_key_path):
"""Shortcut method to establish a connection to a particular dataset in the Cloud Datastore.
"""Shortcut method to establish a connection to a particular
dataset in the Cloud Datastore.
You'll generally use this as the first call to working with the API:
Expand Down
13 changes: 7 additions & 6 deletions gcloud/datastore/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ def _request(self, dataset_id, method, data):
"""
headers = {
'Content-Type': 'application/x-protobuf',
'Content-Length': str(len(data)),
}
'Content-Length': str(len(data)), }
headers, content = self.http.request(
uri=self.build_api_url(dataset_id=dataset_id, method=method),
method='POST', headers=headers, body=data)
Expand All @@ -99,7 +98,7 @@ def build_api_url(cls, dataset_id, method, base_url=None, api_version=None):
:type dataset_id: string
:param dataset_id: The ID of the dataset to connect to.
This is usually your project name in the cloud console.
This is usually your project name in the cloud console.
:type method: string
:param method: The API method to call (ie, runQuery, lookup, ...).
Expand Down Expand Up @@ -134,7 +133,7 @@ def dataset(self, *args, **kwargs):
"""Factory method for Dataset objects.
:param args: All args and kwargs will be passed along to the
:class:`gcloud.datastore.dataset.Dataset` initializer.
:class:`gcloud.datastore.dataset.Dataset` initializer.
:rtype: :class:`gcloud.datastore.dataset.Dataset`
:returns: A dataset object that will use this connection as its transport.
Expand All @@ -156,7 +155,8 @@ def begin_transaction(self, dataset_id, serializable=False):
request = datastore_pb.BeginTransactionRequest()

if serializable:
request.isolation_level = datastore_pb.BeginTransactionRequest.SERIALIZABLE
request.isolation_level = (
datastore_pb.BeginTransactionRequest.SERIALIZABLE)
else:
request.isolation_level = datastore_pb.BeginTransactionRequest.SNAPSHOT

Expand Down Expand Up @@ -226,7 +226,8 @@ def run_query(self, dataset_id, query_pb, namespace=None):
request.partition_id.namespace = namespace

request.query.CopyFrom(query_pb)
response = self._rpc(dataset_id, 'runQuery', request, datastore_pb.RunQueryResponse)
response = self._rpc(dataset_id, 'runQuery', request,
datastore_pb.RunQueryResponse)
return [e.entity for e in response.batch.entity_result]

def lookup(self, dataset_id, key_pbs):
Expand Down
3 changes: 2 additions & 1 deletion gcloud/datastore/demo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
__all__ = ['get_dataset', 'CLIENT_EMAIL', 'DATASET_ID', 'PRIVATE_KEY_PATH']


CLIENT_EMAIL = '754762820716-gimou6egs2hq1rli7el2t621a1b04t9i@developer.gserviceaccount.com'
CLIENT_EMAIL = ('754762820716-gimou6egs2hq1rli7el2t621a1b04t9i'
'@developer.gserviceaccount.com')
DATASET_ID = 'gcloud-datastore-demo'
PRIVATE_KEY_PATH = os.path.join(os.path.dirname(__file__), 'demo.key')

Expand Down
21 changes: 14 additions & 7 deletions gcloud/datastore/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,22 @@ class Entity(dict):
>>> dataset.entity('MyEntityKind')
<Entity[{'kind': 'MyEntityKind'}] {}>
- :func:`gcloud.datastore.dataset.Dataset.get_entity` to retrive an existing entity.
- :func:`gcloud.datastore.dataset.Dataset.get_entity`
to retrive an existing entity.
>>> dataset.get_entity(key)
<Entity[{'kind': 'EntityKind', id: 1234}] {'property': 'value'}>
You can the set values on the entity just like you would on any other dictionary.
You can the set values on the entity
just like you would on any other dictionary.
>>> entity['age'] = 20
>>> entity['name'] = 'JJ'
>>> entity
<Entity[{'kind': 'EntityKind', id: 1234}] {'age': 20, 'name': 'JJ'}>
And you can cast an entity to a regular Python dictionary with the `dict` builtin:
And you can cast an entity to a regular Python dictionary
with the `dict` builtin:
>>> dict(entity)
{'age': 20, 'name': 'JJ'}
Expand All @@ -68,7 +71,8 @@ def __init__(self, dataset=None, kind=None):
self._key = None

def dataset(self):
"""Get the :class:`gcloud.datastore.dataset.Dataset` in which this entity belonds.
"""Get the :class:`gcloud.datastore.dataset.Dataset`

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

Docstring first line should be a single line.

Change to

  def dataset(self):
    """Get the Dataset in which this entity belongs.

    ...
    """

This comment has been minimized.

Copy link
@kleyow

kleyow Sep 29, 2014

Owner

I really like the sphinx link to the model though, I'll try shorten it another way.

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

I am not familiar enough with Sphinx and am happy to be educated.

Link to best practices?

This comment has been minimized.

Copy link
@kleyow

kleyow Sep 29, 2014

Owner

I'm not familiar with it either but I believe its the tool that creates the documentation at http://googlecloudplatform.github.io/gcloud-python/

So this piece of documentation translates to whats here http://googlecloudplatform.github.io/gcloud-python/datastore-api.html#gcloud.datastore.entity.Entity.dataset

Someone correct me if I'm wrong.

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

Yes that's Sphinx. I thought you were intimating that there is actually a sphinx convention for the single-line short-description in a multi-line docstring.

The linking is nice! I suppose we can leave as-is for now and think of a strategy outside of this PR. SGTY @tseaver ?

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 30, 2014

@tseaver Can you address this question too?

in which this entity belonds.
.. note::
This is based on the :class:`gcloud.datastore.key.Key` set on the entity.
Expand Down Expand Up @@ -116,12 +120,14 @@ def kind(self):

@classmethod
def from_key(cls, key):
"""Factory method for creating an entity based on the :class:`gcloud.datastore.key.Key`.
"""Factory method for creating an entity based on the
:class:`gcloud.datastore.key.Key`.

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

Ditto on having a concise first line.

See http://legacy.python.org/dev/peps/pep-0257/#multi-line-docstrings

This comment has been minimized.

Copy link
@kleyow

kleyow Sep 29, 2014

Owner

Can we instead just revert it to its one line without shortening it, but just ignore doc string's line length of 80 pep8 and just stay in the practice of not making them overly long. Some just can't be shorted and I'd rather not remove the class anchor.

This comment has been minimized.

Copy link
@kleyow

kleyow Sep 30, 2014

Owner

Wait scratch that. Would it be better to just say

"""Factory method for creating an entity based on datastore key.

..note:: 
  Datastore key refers to :class:`gcloud.datastore.key.Key`.

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 30, 2014

I am fine reverting "it to its one line without shortening it" for this PR and determining how we want to handle these in another place and then implementing it in another PR.

:type key: :class:`gcloud.datastore.key.Key`
:param key: The key for the entity.
:returns: The :class:`Entity` derived from the :class:`gcloud.datastore.key.Key`.
:returns: The :class:`Entity` derived from the
:class:`gcloud.datastore.key.Key`.

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

@tseaver, is the convention to indent to the word "The", i.e.

    :returns: The :class:`Entity` derived from the
              :class:`gcloud.datastore.key.Key`.

or to do as above?

This comment has been minimized.

Copy link
@tseaver

tseaver Sep 30, 2014

I've seen both, and don't have a strong preference either way: we should double check that stuff renders right in the autodoc-generated HTML, whichever way we settle the question. Note that you can do that locally via:

$ tox -e docs
$ {browser-of-choice} docs/_build/html/{page-of-interest}.html

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 30, 2014

Good call @tseaver. @kleyow please change to

  @classmethod
  def from_key(cls, key):
    """Factory method for creating an entity based on the :class:`gcloud.datastore.key.Key`.

    :type key: :class:`gcloud.datastore.key.Key`
    :param key: The key for the entity.

    :returns: The :class:`Entity` derived from the
              :class:`gcloud.datastore.key.Key`.
    """

and do the same in other places where a line break indent has occurred in a returns statement.

Without the indent, the docs look like:

lineup_returns

whereas with the indent it is:

lineup_with_text

"""

return cls().key(key)
Expand All @@ -135,7 +141,8 @@ def from_protobuf(cls, pb, dataset=None):
:type key: :class:`gcloud.datastore.datastore_v1_pb2.Entity`
:param key: The Protobuf representing the entity.
:returns: The :class:`Entity` derived from the :class:`gcloud.datastore.datastore_v1_pb2.Entity`.
:returns: The :class:`Entity` derived from the
:class:`gcloud.datastore.datastore_v1_pb2.Entity`.
"""

# This is here to avoid circular imports.
Expand Down
6 changes: 3 additions & 3 deletions gcloud/datastore/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ class Query(object):
'<=': datastore_pb.PropertyFilter.LESS_THAN_OR_EQUAL,
'>': datastore_pb.PropertyFilter.GREATER_THAN,
'>=': datastore_pb.PropertyFilter.GREATER_THAN_OR_EQUAL,
'=': datastore_pb.PropertyFilter.EQUAL,
}
'=': datastore_pb.PropertyFilter.EQUAL, }
"""Mapping of operator strings and their protobuf equivalents."""

def __init__(self, kind=None, dataset=None):
Expand All @@ -63,7 +62,8 @@ def _clone(self):
return clone

def to_protobuf(self):
"""Convert the :class:`Query` instance to a :class:`gcloud.datastore.datastore_v1_pb2.Query`.
"""Convert the :class:`Query` instance to a

This comment has been minimized.

Copy link
@dhermes

dhermes Sep 29, 2014

Ditto on multi-line docstring first liner.

:class:`gcloud.datastore.datastore_v1_pb2.Query`.
:rtype: :class:`gclouddatstore.datastore_v1_pb2.Query`
:returns: A Query protobuf that can be sent to the protobuf API.
Expand Down
3 changes: 2 additions & 1 deletion gcloud/storage/acl.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ def save(self):


class DefaultObjectACL(BucketACL):
"""A subclass of BucketACL representing the default object ACL for a bucket."""
"""A subclass of BucketACL representing the
default object ACL for a bucket."""

def save(self):
"""Save this ACL as the default object ACL for the current bucket."""
Expand Down
3 changes: 2 additions & 1 deletion gcloud/storage/bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ def get_all_keys(self):
return list(self)

def new_key(self, key):
"""Given a path name (or a Key), return a :class:`gcloud.storage.key.Key` object.
"""Given a path name (or a Key), return a :class:`gcloud.storage.key.Key`
object.
This is really useful when you're not sure
if you have a Key object or a string path name.
Expand Down
7 changes: 4 additions & 3 deletions gcloud/storage/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ def generate_signed_url(self, resource, expiration,
expiration = int(time.mktime(expiration.timetuple()))

if not isinstance(expiration, (int, long)):
raise ValueError('Expected an integer timestamp, datetime, or timedelta. '
'Got %s' % type(expiration))
raise ValueError('Expected an integer timestamp, datetime, or '
'timedelta. Got %s' % type(expiration))

# Generate the string to sign.
signature_string = '\n'.join([
Expand All @@ -476,7 +476,8 @@ def generate_signed_url(self, resource, expiration,
resource])

# Take our PKCS12 (.p12) key and make it into a RSA key we can use...
pkcs12 = crypto.load_pkcs12(base64.b64decode(self.credentials.private_key), 'notasecret')
pkcs12 = crypto.load_pkcs12(base64.b64decode(self.credentials.private_key),
'notasecret')
pem = crypto.dump_privatekey(crypto.FILETYPE_PEM, pkcs12.get_privatekey())
pem_key = RSA.importKey(pem)

Expand Down
3 changes: 2 additions & 1 deletion gcloud/storage/demo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
__all__ = ['get_connection', 'CLIENT_EMAIL', 'PRIVATE_KEY_PATH', 'PROJECT']


CLIENT_EMAIL = '606734090113-6ink7iugcv89da9sru7lii8bs3i0obqg@developer.gserviceaccount.com'
CLIENT_EMAIL = ('606734090113-6ink7iugcv89da9sru7lii8bs3i0obqg@'
'developer.gserviceaccount.com')
PRIVATE_KEY_PATH = os.path.join(os.path.dirname(__file__), 'demo.key')
PROJECT = 'gcloud-storage-demo'

Expand Down
6 changes: 4 additions & 2 deletions gcloud/storage/iterator.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ def __init__(self, connection):
super(BucketIterator, self).__init__(connection=connection, path='/b')

def get_items_from_response(self, response):
"""Factory method which yields :class:`gcloud.storage.bucket.Bucket` items from a response.
"""Factory method which yields :class:`gcloud.storage.bucket.Bucket`
items from a response.
:type response: dict
:param response: The JSON API response for a page of buckets.
Expand Down Expand Up @@ -174,7 +175,8 @@ def __init__(self, bucket):
connection=bucket.connection, path=bucket.path + '/o')

def get_items_from_response(self, response):
"""Factory method which yields :class:`gcloud.storage.key.Key` items from a response.
"""Factory method which yields :class:`gcloud.storage.key.Key`
items from a response.
:type response: dict
:param response: The JSON API response for a page of keys.
Expand Down
9 changes: 4 additions & 5 deletions gcloud/storage/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ def public_url(self):
return '{storage_base_url}/{self.bucket.name}/{self.name}'.format(
storage_base_url='http://commondatastorage.googleapis.com', self=self)

def generate_signed_url(self, expiration, method='GET'): # pragma NO COVER UGH
def generate_signed_url(self, expiration,
method='GET'): # pragma NO COVER UGH
"""Generates a signed URL for this key.
If you have a key that you want to allow access to
Expand Down Expand Up @@ -207,8 +208,7 @@ def set_contents_from_file(self, fh, rewind=False, size=None,
# Set up a resumable upload session.
headers = {
'X-Upload-Content-Type': content_type or 'application/unknown',
'X-Upload-Content-Length': total_bytes
}
'X-Upload-Content-Length': total_bytes}

upload_url = self.connection.build_api_url(
path=self.bucket.path + '/o',
Expand All @@ -231,8 +231,7 @@ def set_contents_from_file(self, fh, rewind=False, size=None,
end = bytes_uploaded + chunk_size - 1

headers = {
'Content-Range': 'bytes %d-%d/%d' % (start, end, total_bytes),
}
'Content-Range': 'bytes %d-%d/%d' % (start, end, total_bytes), }

response, content = self.connection.make_request(
content_type='text/plain',
Expand Down
2 changes: 1 addition & 1 deletion gcloud/storage/test_bucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def test_get_metadata_none_set_acl_hit(self):
self.assertEqual(kw[0]['path'], '/b/%s' % NAME)
self.assertEqual(kw[0]['query_params'], {'projection': 'full'})

def test_get_metadata_none_set_defaultObjectAcl_miss_explicit_default(self):
def test_get_metadata_none_set_defaultObjectAcl_miss_clear_default(self):
NAME = 'name'
after = {'bar': 'Bar'}
connection = _Connection(after)
Expand Down
14 changes: 7 additions & 7 deletions gcloud/storage/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def test_make_request_no_data_no_content_type_no_headers(self):
self.assertEqual(http._called_with['body'], None)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0, })
'Content-Length': 0, })

def test_make_request_w_data_no_extra_headers(self):
PROJECT = 'project'
Expand All @@ -170,7 +170,7 @@ def test_make_request_w_data_no_extra_headers(self):
self.assertEqual(http._called_with['body'], {})
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0,
'Content-Length': 0,
'Content-Type': 'application/json', })

def test_make_request_w_extra_headers(self):
Expand All @@ -186,7 +186,7 @@ def test_make_request_w_extra_headers(self):
self.assertEqual(http._called_with['body'], None)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0,
'Content-Length': 0,
'X-Foo': 'foo', })

def test_api_request_defaults(self):
Expand All @@ -206,7 +206,7 @@ def test_api_request_defaults(self):
self.assertEqual(http._called_with['body'], None)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0, })
'Content-Length': 0, })

def test_api_request_w_path(self):
PROJECT = 'project'
Expand All @@ -223,7 +223,7 @@ def test_api_request_w_path(self):
self.assertEqual(http._called_with['body'], None)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0, })
'Content-Length': 0, })

def test_api_request_w_non_json_response(self):
PROJECT = 'project'
Expand Down Expand Up @@ -273,7 +273,7 @@ def test_api_request_w_query_params(self):
self.assertEqual(http._called_with['body'], None)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': 0, })
'Content-Length': 0, })

def test_api_request_w_data(self):
import json
Expand All @@ -293,7 +293,7 @@ def test_api_request_w_data(self):
self.assertEqual(http._called_with['body'], DATAJ)
self.assertEqual(http._called_with['headers'],
{'Accept-Encoding': 'gzip',
'Content-Length': len(DATAJ),
'Content-Length': len(DATAJ),
'Content-Type': 'application/json', })

def test_api_request_w_404(self):
Expand Down

0 comments on commit b8a989f

Please sign in to comment.