From 029bbe508042efb68dfd3ca0570b11bb2aab961a Mon Sep 17 00:00:00 2001 From: Bernie Hackett Date: Mon, 23 Feb 2015 14:43:28 -0800 Subject: [PATCH] PEP8 / Pylint cleanups. --- pymongo/collection.py | 45 +++++++++++++++++++++---------------- pymongo/common.py | 1 + pymongo/cursor.py | 19 ++++++++-------- pymongo/database.py | 17 ++++++++------ pymongo/mongo_client.py | 49 +++++++++++++++++++++-------------------- test/test_client.py | 4 ++-- 6 files changed, 74 insertions(+), 61 deletions(-) diff --git a/pymongo/collection.py b/pymongo/collection.py index bfcaeec1c..324e7ac97 100644 --- a/pymongo/collection.py +++ b/pymongo/collection.py @@ -37,15 +37,19 @@ from pymongo.helpers import _check_write_command_response, _command from pymongo.message import _INSERT, _UPDATE, _DELETE from pymongo.options import ReturnDocument, _WriteOp from pymongo.read_preferences import ReadPreference -from pymongo.results import * +from pymongo.results import (BulkWriteResult, + DeleteResult, + InsertOneResult, + InsertManyResult, + UpdateResult) from pymongo.write_concern import WriteConcern try: from collections import OrderedDict - ordered_types = (SON, OrderedDict) + _ORDERED_TYPES = (SON, OrderedDict) except ImportError: - ordered_types = SON + _ORDERED_TYPES = (SON,) _NO_OBJ_ERROR = "No matching object found" @@ -146,7 +150,7 @@ class Collection(common.BaseObject): def _command( self, command, read_preference=None, codec_options=None, **kwargs): """Internal command helper. - + :Parameters: - `command` - The command itself, as a SON instance. - `read_preference` (optional) - An subclass of @@ -200,9 +204,8 @@ class Collection(common.BaseObject): def __eq__(self, other): if isinstance(other, Collection): - us = (self.__database, self.__name) - them = (other.__database, other.__name) - return us == them + return (self.__database == other.database and + self.__name == other.name) return NotImplemented def __ne__(self, other): @@ -334,20 +337,24 @@ class Collection(common.BaseObject): if manipulate: def gen(): - db = self.__database + """Generator that applies SON manipulators to each document + and adds _id if necessary. + """ + _db = self.__database for doc in docs: # Apply user-configured SON manipulators. This order of # operations is required for backwards compatibility, # see PYTHON-709. - doc = db._apply_incoming_manipulators(doc, self) + doc = _db._apply_incoming_manipulators(doc, self) if '_id' not in doc: doc['_id'] = ObjectId() - doc = db._apply_incoming_copying_manipulators(doc, self) + doc = _db._apply_incoming_copying_manipulators(doc, self) ids.append(doc['_id']) yield doc else: def gen(): + """Generator that only tracks existing _ids.""" for doc in docs: ids.append(doc.get('_id')) yield doc @@ -1312,8 +1319,8 @@ class Collection(common.BaseObject): } return CommandCursor(self, cursor_info, address) - # TODO key and condition ought to be optional, but deprecation - # could be painful as argument order would have to change. + # key and condition ought to be optional, but deprecation + # would be painful as argument order would have to change. def group(self, key, condition, initial, reduce, finalize=None, **kwargs): """Perform a query similar to an SQL *group by* operation. @@ -1747,10 +1754,10 @@ class Collection(common.BaseObject): ", find_one_and_replace, or find_one_and_update instead", DeprecationWarning, stacklevel=2) - if (not update and not kwargs.get('remove', None)): + if not update and not kwargs.get('remove', None): raise ValueError("Must either update or remove") - if (update and kwargs.get('remove', None)): + if update and kwargs.get('remove', None): raise ValueError("Can't do both update and remove") # No need to include empty args @@ -1766,7 +1773,7 @@ class Collection(common.BaseObject): kwargs['sort'] = helpers._index_document(sort) # Accept OrderedDict, SON, and dict with len == 1 so we # don't break existing code already using find_and_modify. - elif (isinstance(sort, ordered_types) or + elif (isinstance(sort, _ORDERED_TYPES) or isinstance(sort, dict) and len(sort) == 1): warnings.warn("Passing mapping types for `sort` is deprecated," " use a list of (key, direction) pairs instead", @@ -1774,8 +1781,8 @@ class Collection(common.BaseObject): kwargs['sort'] = sort else: raise TypeError("sort must be a list of (key, direction) " - "pairs, a dict of len 1, or an instance of " - "SON or OrderedDict") + "pairs, a dict of len 1, or an instance of " + "SON or OrderedDict") fields = kwargs.pop("fields", None) @@ -1806,10 +1813,10 @@ class Collection(common.BaseObject): def __iter__(self): return self - def next(self): + def __next__(self): raise TypeError("'Collection' object is not iterable") - __next__ = next + next = __next__ def __call__(self, *args, **kwargs): """This is only here so that some API misusages are easier to debug. diff --git a/pymongo/common.py b/pymongo/common.py index 02c5efa53..eb03de208 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -298,6 +298,7 @@ def validate_auth_mechanism_properties(option, value): def validate_document_class(option, value): + """Validate the document_class option.""" if not issubclass(value, collections.MutableMapping): raise ConfigurationError("%s must be a sublass of " "collections.MutableMapping" % (option,)) diff --git a/pymongo/cursor.py b/pymongo/cursor.py index c82ec07a2..817d34b62 100644 --- a/pymongo/cursor.py +++ b/pymongo/cursor.py @@ -236,6 +236,7 @@ class Cursor(object): return self._clone(True) def _clone(self, deepcopy=True): + """Internal clone helper.""" clone = self._clone_base() values_to_clone = ("spec", "projection", "skip", "limit", "max_time_ms", "comment", "max", "min", @@ -800,7 +801,7 @@ class Cursor(object): self.__spec["$where"] = code return self - def __send_message(self, message): + def __send_message(self, msg): """Send a query or getmore message and handles the response. If message is ``None`` this is an exhaust cursor, which reads @@ -811,7 +812,7 @@ class Cursor(object): """ client = self.__collection.database.connection - if message: + if msg: kwargs = { "read_preference": self.__read_preference, "exhaust": self.__exhaust, @@ -820,7 +821,7 @@ class Cursor(object): kwargs["address"] = self.__address try: - response = client._send_message_with_response(message, **kwargs) + response = client._send_message_with_response(msg, **kwargs) self.__address = response.address if self.__exhaust: # 'response' is an ExhaustResponse. @@ -873,7 +874,7 @@ class Cursor(object): self.__id = doc["cursor_id"] # starting from doesn't get set on getmore's for tailable cursors - if not (self.__query_flags & _QUERY_OPTIONS["tailable_cursor"]): + if not self.__query_flags & _QUERY_OPTIONS["tailable_cursor"]: assert doc["starting_from"] == self.__retrieved, ( "Result batch started from %s, expected %s" % ( doc['starting_from'], self.__retrieved)) @@ -970,20 +971,20 @@ class Cursor(object): def __iter__(self): return self - def next(self): + def __next__(self): if self.__empty: raise StopIteration - db = self.__collection.database + _db = self.__collection.database if len(self.__data) or self._refresh(): if self.__manipulate: - return db._fix_outgoing(self.__data.popleft(), - self.__collection) + return _db._fix_outgoing(self.__data.popleft(), + self.__collection) else: return self.__data.popleft() else: raise StopIteration - __next__ = next + next = __next__ def __enter__(self): return self diff --git a/pymongo/database.py b/pymongo/database.py index 532e574d3..b8668a7f8 100644 --- a/pymongo/database.py +++ b/pymongo/database.py @@ -116,6 +116,7 @@ class Database(common.BaseObject): """ base = SONManipulator() def method_overwritten(instance, method): + """Test if this method has been overridden.""" return (getattr( instance, method).__func__ != getattr(base, method).__func__) @@ -188,9 +189,8 @@ class Database(common.BaseObject): def __eq__(self, other): if isinstance(other, Database): - us = (self.__connection, self.__name) - them = (other.__connection, other.__name) - return us == them + return (self.__connection == other.connection and + self.__name == other.name) return NotImplemented def __ne__(self, other): @@ -296,11 +296,13 @@ class Database(common.BaseObject): read_preference, write_concern, **kwargs) def _apply_incoming_manipulators(self, son, collection): + """Apply incoming manipulators to `son`.""" for manipulator in self.__incoming_manipulators: son = manipulator.transform_incoming(son, collection) return son def _apply_incoming_copying_manipulators(self, son, collection): + """Apply incoming copying manipulators to `son`.""" for manipulator in self.__incoming_copying_manipulators: son = manipulator.transform_incoming(son, collection) return son @@ -523,7 +525,7 @@ class Database(common.BaseObject): if "result" in res: info = res["result"] if (info.find("exception") != -1 or - info.find("corrupt") != -1): + info.find("corrupt") != -1): raise CollectionInvalid("%s invalid: " "%s" % (name, info)) elif not res.get("valid", False): @@ -694,12 +696,13 @@ class Database(common.BaseObject): def __iter__(self): return self - def next(self): + def __next__(self): raise TypeError("'Database' object is not iterable") - __next__ = next + next = __next__ def _default_role(self, read_only): + """Return the default user role for this database.""" if self.name == "admin": if read_only: return "readAnyDatabase" @@ -775,7 +778,7 @@ class Database(common.BaseObject): # First admin user add fails gle from mongos 2.0.x # and 2.2.x. elif (exc.details and - 'getlasterror' in exc.details.get('note', '')): + 'getlasterror' in exc.details.get('note', '')): pass else: raise diff --git a/pymongo/mongo_client.py b/pymongo/mongo_client.py index 7152e9a9f..2f8205198 100644 --- a/pymongo/mongo_client.py +++ b/pymongo/mongo_client.py @@ -46,7 +46,6 @@ from pymongo import (common, helpers, message, periodic_executor, - pool, uri_parser) from pymongo.client_options import ClientOptions from pymongo.cursor_manager import CursorManager @@ -383,22 +382,22 @@ class MongoClient(common.BaseObject): index in cache[dbname][coll] and now < cache[dbname][coll][index]) - def _cache_index(self, database, collection, index, cache_for): + def _cache_index(self, dbname, collection, index, cache_for): """Add an index to the index cache for ensure_index operations.""" now = datetime.datetime.utcnow() expire = datetime.timedelta(seconds=cache_for) + now if database not in self.__index_cache: - self.__index_cache[database] = {} - self.__index_cache[database][collection] = {} - self.__index_cache[database][collection][index] = expire + self.__index_cache[dbname] = {} + self.__index_cache[dbname][collection] = {} + self.__index_cache[dbname][collection][index] = expire - elif collection not in self.__index_cache[database]: - self.__index_cache[database][collection] = {} - self.__index_cache[database][collection][index] = expire + elif collection not in self.__index_cache[dbname]: + self.__index_cache[dbname][collection] = {} + self.__index_cache[dbname][collection][index] = expire else: - self.__index_cache[database][collection][index] = expire + self.__index_cache[dbname][collection][index] = expire def _purge_index(self, database_name, collection_name=None, index_name=None): @@ -631,12 +630,12 @@ class MongoClient(common.BaseObject): """ topology = self._get_topology() # Starts monitors if necessary. try: - s = topology.select_server(writable_server_selector) + svr = topology.select_server(writable_server_selector) # When directly connected to a secondary, arbiter, etc., # select_server returns it, whatever the selector. Check # again if the server is writable. - return s.description.is_writable + return svr.description.is_writable except ConnectionFailure: return False @@ -764,7 +763,7 @@ class MongoClient(common.BaseObject): raise OperationFailure(details["err"], code, result) def _send_message( - self, message, with_last_error=False, command=False, + self, msg, with_last_error=False, command=False, check_primary=True, address=None): """Send a message to MongoDB, optionally returning response as a dict. @@ -775,7 +774,7 @@ class MongoClient(common.BaseObject): is ``False``. :Parameters: - - `message`: (request_id, data). + - `msg`: (request_id, data). - `with_last_error` (optional): check getLastError status after sending the message. - `check_primary` (optional): don't try to write to a non-primary; @@ -809,7 +808,7 @@ class MongoClient(common.BaseObject): response = self._reset_on_error( server, server.send_message_with_response, - message, + msg, self.__all_credentials) try: @@ -825,15 +824,15 @@ class MongoClient(common.BaseObject): self._reset_on_error( server, server.send_message, - message, + msg, self.__all_credentials) def _send_message_with_response( - self, message, read_preference=None, exhaust=False, address=None): + self, msg, read_preference=None, exhaust=False, address=None): """Send a message to MongoDB and return a Response. :Parameters: - - `message`: (request_id, data, max_doc_size) or (request_id, data). + - `msg`: (request_id, data, max_doc_size) or (request_id, data). - `read_preference` (optional): A ReadPreference. - `exhaust` (optional): If True, the socket used stays checked out. It is returned along with its Pool in the Response. @@ -857,11 +856,11 @@ class MongoClient(common.BaseObject): return self._reset_on_error( server, server.send_message_with_response, - message, + msg, self.__all_credentials, exhaust) - def _reset_on_error(self, server, fn, *args, **kwargs): + def _reset_on_error(self, server, func, *args, **kwargs): """Execute an operation. Reset the server on network error. Returns fn()'s return value on success. On error, clears the server's @@ -870,7 +869,7 @@ class MongoClient(common.BaseObject): Re-raises any exception thrown by fn(). """ try: - return fn(*args, **kwargs) + return func(*args, **kwargs) except NetworkTimeout: # The socket has been closed. Don't reset the server. raise @@ -976,6 +975,7 @@ class MongoClient(common.BaseObject): # This method is run periodically by a background thread. def _process_kill_cursors_queue(self): + """Process any pending kill cursors requests.""" address_to_cursor_ids = defaultdict(list) # Other threads or the GC may append to the queue concurrently. @@ -1005,8 +1005,9 @@ class MongoClient(common.BaseObject): def database_names(self): """Get a list of the names of all databases on the connected server.""" return [db["name"] for db in - self.admin.command("listDatabases", - read_preference=ReadPreference.PRIMARY)["databases"]] + self.admin.command( + "listDatabases", + read_preference=ReadPreference.PRIMARY)["databases"]] def drop_database(self, name_or_database): """Drop a database. @@ -1112,7 +1113,7 @@ class MongoClient(common.BaseObject): def __iter__(self): return self - def next(self): + def __next__(self): raise TypeError("'MongoClient' object is not iterable") - __next__ = next + next = __next__ diff --git a/test/test_client.py b/test/test_client.py index 76b2322a1..dc2f66260 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -819,7 +819,7 @@ class TestClient(IntegrationTest): with client_knobs(server_wait_time=0.01): with self.assertRaises(AutoReconnect): self.client._send_message_with_response( - message=message.get_more('collection', 101, 1234), + msg=message.get_more('collection', 101, 1234), address=('not-a-member', 27017)) @client_context.require_replica_set @@ -830,7 +830,7 @@ class TestClient(IntegrationTest): with client_knobs(server_wait_time=0.01): with self.assertRaises(AutoReconnect): self.client._send_message( - message=message.kill_cursors([1234]), + msg=message.kill_cursors([1234]), check_primary=False, address=('not-a-member', 27017))