diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index ae747f97fa..85bc0a9d41 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/common/lib/xmodule/xmodule/contentstore/mongo.py @@ -56,7 +56,7 @@ class MongoContentStore(ContentStore): """ Closes any open connections to the underlying databases """ - self.fs_files.database.connection.close() + self.fs_files.database.client.close() def _drop_database(self, database=True, collections=True, connections=True): """ @@ -70,10 +70,10 @@ class MongoContentStore(ContentStore): If connections is True, then close the connection to the database as well. """ - connection = self.fs_files.database.connection + connection = self.fs_files.database.client if database: - connection.drop_database(self.fs_files.database) + connection.drop_database(self.fs_files.database.name) elif collections: self.fs_files.drop() self.chunks.drop() @@ -320,15 +320,18 @@ class MongoContentStore(ContentStore): } }) - items = self.fs_files.aggregate(pipeline_stages) - if items['result']: - result = items['result'][0] - count = result['count'] - assets = list(result['results']) - else: - # no results - count = 0 - assets = [] + cursor = self.fs_files.aggregate(pipeline_stages) + # Set values if result of query is empty + count = 0 + assets = [] + try: + result = cursor.next() + if result: + count = result['count'] + assets = list(result['results']) + except StopIteration: + # Skip if no assets were returned + pass # We're constructing the asset key immediately after retrieval from the database so that # callers are insulated from knowing how our identifiers are stored. diff --git a/common/lib/xmodule/xmodule/course_metadata_utils.py b/common/lib/xmodule/xmodule/course_metadata_utils.py index c862356513..3ae03b3418 100644 --- a/common/lib/xmodule/xmodule/course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/course_metadata_utils.py @@ -201,7 +201,7 @@ def sorting_dates(start, advertised_start, announcement): start = dateutil.parser.parse(advertised_start) if start.tzinfo is None: start = start.replace(tzinfo=utc) - except (ValueError, AttributeError): + except (TypeError, ValueError, AttributeError): start = start now = datetime.now(utc) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index bd3c5b5264..ede5b88e33 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -603,14 +603,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ Closes any open connections to the underlying database """ - self.collection.database.connection.close() - - def mongo_wire_version(self): - """ - Returns the wire version for mongo. Only used to unit tests which instrument the connection. - """ - self.database.connection._ensure_connected() - return self.database.connection.max_wire_version + self.collection.database.client.close() def _drop_database(self, database=True, collections=True, connections=True): """ @@ -627,14 +620,14 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # drop the assets super(MongoModuleStore, self)._drop_database(database, collections, connections) - connection = self.collection.database.connection + connection = self.collection.database.client if database: connection.drop_database(self.collection.database.proxied_object) elif collections: self.collection.drop() else: - self.collection.remove({}) + self.collection.delete_many({}) if connections: connection.close() @@ -1913,7 +1906,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ source_assets = self._find_course_assets(source_course_key) dest_assets = {'assets': source_assets.asset_md.copy(), 'course_id': six.text_type(dest_course_key)} - self.asset_collection.remove({'course_id': six.text_type(dest_course_key)}) + self.asset_collection.delete_many({'course_id': six.text_type(dest_course_key)}) # Update the document. self.asset_collection.insert(dest_assets) @@ -1985,7 +1978,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # A single document exists per course to store the course asset metadata. try: course_assets = self._find_course_assets(course_key) - self.asset_collection.remove(course_assets.doc_id) + self.asset_collection.delete_many({'_id': course_assets.doc_id}) except ItemNotFoundError: # When deleting asset metadata, if a course's asset metadata is not present, no big deal. pass @@ -1994,9 +1987,11 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ Check that the db is reachable. """ - if self.database.connection.alive(): + try: + # The ismaster command is cheap and does not require auth. + self.database.client.admin.command('ismaster') return {ModuleStoreEnum.Type.mongo: True} - else: + except pymongo.errors.ConnectionFailure: raise HeartbeatFailure("Can't connect to {}".format(self.database.name), 'mongo') def ensure_indexes(self): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 9a264fae1d..2f746f9932 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -178,7 +178,7 @@ class DraftModuleStore(MongoModuleStore): # delete all of the db records for the course course_query = self._course_key_to_son(course_key) - self.collection.remove(course_query, multi=True) + self.collection.delete_many(course_query) self.delete_all_asset_metadata(course_key, user_id) self._emit_course_deleted_signal(course_key) @@ -651,7 +651,7 @@ class DraftModuleStore(MongoModuleStore): if len(to_be_deleted) > 0: bulk_record = self._get_bulk_ops_record(root_usages[0].course_key) bulk_record.dirty = True - self.collection.remove({'_id': {'$in': to_be_deleted}}, safe=self.collection.safe) + self.collection.delete_many({'_id': {'$in': to_be_deleted}}) def has_changes(self, xblock): """ @@ -766,7 +766,7 @@ class DraftModuleStore(MongoModuleStore): bulk_record = self._get_bulk_ops_record(course_key) if len(to_be_deleted) > 0: bulk_record.dirty = True - self.collection.remove({'_id': {'$in': to_be_deleted}}) + self.collection.delete_many({'_id': {'$in': to_be_deleted}}) self._flag_publish_event(course_key) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index cdfd854d09..ec456b85c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -296,9 +296,11 @@ class MongoConnection(object): """ Check that the db is reachable. """ - if self.database.connection.alive(): + try: + # The ismaster command is cheap and does not require auth. + self.database.client.admin.command('ismaster') return True - else: + except pymongo.errors.ConnectionFailure: raise HeartbeatFailure("Can't connect to {}".format(self.database.name), 'mongo') def get_structure(self, key, course_context=None): @@ -589,13 +591,7 @@ class MongoConnection(object): """ Closes any open connections to the underlying databases """ - self.database.connection.close() - - def mongo_wire_version(self): - """ - Returns the wire version for mongo. Only used to unit tests which instrument the connection. - """ - return self.database.connection.max_wire_version + self.database.client.close() def _drop_database(self, database=True, collections=True, connections=True): """ @@ -609,7 +605,7 @@ class MongoConnection(object): If connections is True, then close the connection to the database as well. """ - connection = self.database.connection + connection = self.database.client if database: connection.drop_database(self.database.name) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index bd9e387865..aa7e15c699 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -751,12 +751,6 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ self.db_connection.close_connections() - def mongo_wire_version(self): - """ - Returns the wire version for mongo. Only used to unit tests which instrument the connection. - """ - return self.db_connection.mongo_wire_version() - def _drop_database(self, database=True, collections=True, connections=True): """ A destructive operation to drop the underlying database and close all connections. diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 531d4082a5..1c208014d1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -610,8 +610,6 @@ def mongo_uses_error_check(store): """ Does mongo use the error check as a separate message? """ - if hasattr(store, 'mongo_wire_version'): - return store.mongo_wire_version() <= 1 if hasattr(store, 'modulestores'): return any([mongo_uses_error_check(substore) for substore in store.modulestores]) return False @@ -630,16 +628,16 @@ def check_mongo_calls_range(max_finds=float("inf"), min_finds=0, max_sends=None, :param min_sends: If non-none, make sure number of send calls are >=min_sends """ with check_sum_of_calls( - pymongo.message, - ['query', 'get_more'], + pymongo.collection.Collection, + ['find'], max_finds, min_finds, ): if max_sends is not None or min_sends is not None: with check_sum_of_calls( - pymongo.message, + pymongo.collection.Collection, # mongo < 2.6 uses insert, update, delete and _do_batched_insert. >= 2.6 _do_batched_write - ['insert', 'update', 'delete', '_do_batched_write_command', '_do_batched_insert', ], + ['insert', 'update', 'bulk_write', '_delete'], max_sends if max_sends is not None else float("inf"), min_sends if min_sends is not None else 0, ): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py index ebc7b4b852..a86f1df952 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py @@ -155,11 +155,11 @@ class CountMongoCallsCourseTraversal(TestCase): (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, True, False, 359), # The line below shows the way this traversal *should* be done # (if you'll eventually access all the fields and load all the definitions anyway). - (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 4), + (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 3), (MIXED_SPLIT_MODULESTORE_BUILDER, None, True, True, 38), (MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 38), (MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, True, 38), - (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 4), + (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 3), (MIXED_SPLIT_MODULESTORE_BUILDER, None, True, False, 3), (MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 3), (MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, False, 3) @@ -179,7 +179,7 @@ class CountMongoCallsCourseTraversal(TestCase): @ddt.data( (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 176), - (MIXED_SPLIT_MODULESTORE_BUILDER, 5), + (MIXED_SPLIT_MODULESTORE_BUILDER, 4), ) @ddt.unpack def test_lazy_when_course_previously_cached(self, store_builder, num_mongo_calls): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py index c66d4a4997..e89cf15ad4 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py @@ -4,6 +4,7 @@ from __future__ import absolute_import import unittest from mock import patch +from pymongo.errors import ConnectionFailure from xmodule.exceptions import HeartbeatFailure from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection @@ -17,7 +18,7 @@ class TestHeartbeatFailureException(unittest.TestCase): def test_heartbeat_raises_exception_when_connection_alive_is_false(self, *calls): # pylint: disable=W0613 with patch('mongodb_proxy.MongoProxy') as mock_proxy: - mock_proxy.return_value.alive.return_value = False + mock_proxy.return_value.admin.command.side_effect = ConnectionFailure('Test') useless_conn = MongoConnection('useless', 'useless', 'useless') with self.assertRaises(HeartbeatFailure): diff --git a/common/lib/xmodule/xmodule/mongo_utils.py b/common/lib/xmodule/xmodule/mongo_utils.py index bb70697f5e..b6d5d414f9 100644 --- a/common/lib/xmodule/xmodule/mongo_utils.py +++ b/common/lib/xmodule/xmodule/mongo_utils.py @@ -7,10 +7,19 @@ import logging import pymongo from mongodb_proxy import MongoProxy -from pymongo import ReadPreference +from pymongo.read_preferences import ( + ReadPreference, + read_pref_mode_from_name, + _MONGOS_MODES, + _MODES +) + logger = logging.getLogger(__name__) # pylint: disable=invalid-name +# This will yeld a map of all available Mongo modes and their name +MONGO_READ_PREFERENCE_MAP = dict(zip(_MONGOS_MODES, _MODES)) + # pylint: disable=bad-continuation def connect_to_mongodb( @@ -39,10 +48,16 @@ def connect_to_mongodb( # If the MongoDB server uses a separate authentication database that should be specified here auth_source = kwargs.pop('auth_source', '') or None - # If read_preference is given as a name of a valid ReadPreference. constant - # such as "SECONDARY_PREFERRED", convert it. Otherwise pass it through unchanged. + # If read_preference is given as a name of a valid ReadPreference. + # constant such as "SECONDARY_PREFERRED" or a mongo mode such as + # "secondaryPreferred", convert it. Otherwise pass it through unchanged. if 'read_preference' in kwargs: - read_preference = getattr(ReadPreference, kwargs['read_preference'], None) + read_preference = MONGO_READ_PREFERENCE_MAP.get( + kwargs['read_preference'], + kwargs['read_preference'] + ) + + read_preference = getattr(ReadPreference, read_preference, None) if read_preference is not None: kwargs['read_preference'] = read_preference diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index a888f7374c..1d4cf7ba25 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -350,4 +350,4 @@ def add_repo(repo, rdir_in, branch=None): cil.save() log.debug(u'saved CourseImportLog for %s', cil.course_id) - mdb.disconnect() + mdb.close() diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index f09b6b311e..45b1d5a8ca 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -553,7 +553,7 @@ class GitLogs(TemplateView): page = min(max(1, given_page), paginator.num_pages) logs = paginator.page(page) - mdb.disconnect() + mdb.close() context = { 'logs': logs, 'course_id': text_type(course_id) if course_id else None, diff --git a/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py b/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py index e1c39038c4..a43e91859f 100644 --- a/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py +++ b/lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py @@ -19,7 +19,7 @@ from testfixtures import LogCapture from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from common.test.utils import MockS3Mixin +from common.test.utils import MockS3Mixin, py2_only from lms.djangoapps.verify_student.models import SoftwareSecurePhotoVerification from lms.djangoapps.verify_student.tests.test_models import FAKE_SETTINGS, mock_software_secure_post @@ -206,11 +206,14 @@ class TestSendVerificationExpiryEmail(MockS3Mixin, ModuleStoreTestCase): self.assertEqual(len(mail.outbox), 1) self.assertIsNone(attempt.expiry_email_date) + @py2_only def test_user_enrolled_in_verified_course(self): """ Test that if the user is enrolled in verified track, then after sending the default no of emails, `expiry_email_date` is updated to now() so that it's filtered in the future to send - email again + email again. + + Does not work on python 3 with the latest mongo driver version due to class inheritance issues. """ user = UserFactory.create() course = CourseFactory() diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index ea8527d848..be055ff81e 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/lms/djangoapps/verify_student/tests/test_services.py @@ -23,7 +23,7 @@ FAKE_SETTINGS = { @patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) @ddt.ddt -class TestIDVerificationService(MockS3Mixin, ModuleStoreTestCase): +class TestIDVerificationService(ModuleStoreTestCase, MockS3Mixin): """ Tests for IDVerificationService. """ diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 23bd88aae0..da4633ff4a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -192,7 +192,7 @@ pygments==2.5.2 pygraphviz==1.5 pyjwkest==1.3.2 pyjwt==1.5.2 -pymongo==2.9.1 +pymongo==3.9.0 pynliner==0.8.0 pyparsing==2.2.0 # via pycontracts pysrt==1.1.1 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 333c0da673..0007a3ba9d 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -255,7 +255,7 @@ pylint-celery==0.3 pylint-django==0.7.2 pylint-plugin-utils==0.3 pylint==1.7.6 -pymongo==2.9.1 +pymongo==3.9.0 pynliner==0.8.0 pyparsing==2.2.0 pyquery==1.4.1 diff --git a/requirements/edx/paver.in b/requirements/edx/paver.in index a8fb579c86..e0e7d05b03 100644 --- a/requirements/edx/paver.in +++ b/requirements/edx/paver.in @@ -18,7 +18,7 @@ mock # Stub out code with mock objects and make a path.py==8.2.1 # Easier manipulation of filesystem paths paver # Build, distribution and deployment scripting tool psutil==1.2.1 # Library for retrieving information on running processes and system utilization -pymongo==2.9.1 # via edx-opaque-keys +pymongo==3.9.0 # via edx-opaque-keys python-memcached # Python interface to the memcached memory cache daemon requests # Simple interface for making HTTP requests stevedore # Support for runtime plugins, used for XBlocks and edx-platform Django app plugins diff --git a/requirements/edx/paver.txt b/requirements/edx/paver.txt index 47f048fc7f..c02248a221 100644 --- a/requirements/edx/paver.txt +++ b/requirements/edx/paver.txt @@ -19,7 +19,7 @@ pathtools==0.1.2 # via watchdog paver==1.3.4 pbr==5.4.4 # via stevedore psutil==1.2.1 -pymongo==2.9.1 +pymongo==3.9.0 python-memcached==1.59 pyyaml==5.2 # via watchdog requests==2.22.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index de1e2d4738..3a8290e09e 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -244,7 +244,7 @@ pylint-celery==0.3 # via edx-lint pylint-django==0.7.2 # via edx-lint pylint-plugin-utils==0.3 # via pylint-celery, pylint-django pylint==1.7.6 # via edx-lint, pylint-celery, pylint-django -pymongo==2.9.1 +pymongo==3.9.0 pynliner==0.8.0 pyparsing==2.2.0 pyquery==1.4.1