Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #21197 from open-craft/giovanni/upgrade-pymongo

Browse Source 
BB-1744: Bump pymongo version and fix course export issue
This commit is contained in:
Feanil Patel
2019-12-26 11:16:15 -05:00
committed by GitHub
parent a4b64c3a79 5983d5dd42
commit f359c9cd8f
19 changed files with 75 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
common/lib/xmodule/xmodule/contentstore/mongo.py
View File

@@ -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.

2
common/lib/xmodule/xmodule/course_metadata_utils.py
View File

@@ -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)

23
common/lib/xmodule/xmodule/modulestore/mongo/base.py
View File

@@ -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):

6
common/lib/xmodule/xmodule/modulestore/mongo/draft.py
View File

@@ -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)

16
common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py
View File

@@ -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)

6
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -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.

10
common/lib/xmodule/xmodule/modulestore/tests/factories.py
View File

@@ -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,
):

6
common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py
View File

@@ -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):

3
common/lib/xmodule/xmodule/modulestore/tests/test_split_mongo_mongo_connection.py
View File

@@ -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):

23
common/lib/xmodule/xmodule/mongo_utils.py
View File

@@ -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.<NAME> constant
# such as "SECONDARY_PREFERRED", convert it. Otherwise pass it through unchanged.
# If read_preference is given as a name of a valid ReadPreference.<NAME>
# 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

2
lms/djangoapps/dashboard/git_import.py
View File

@@ -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()

2
lms/djangoapps/dashboard/sysadmin.py
View File

@@ -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,

7
lms/djangoapps/verify_student/management/commands/tests/test_send_verification_expiry_email.py
View File

@@ -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()

2
lms/djangoapps/verify_student/tests/test_services.py
View File

@@ -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.
"""

2
requirements/edx/base.txt
View File

@@ -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

2
requirements/edx/development.txt
View File

@@ -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

2
requirements/edx/paver.in
View File

@@ -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

2
requirements/edx/paver.txt
View File

@@ -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

2
requirements/edx/testing.txt
View File

@@ -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