Skip to content

refactor: convert global modulestore to request-cached #37859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ormsbee wants to merge 1 commit into from
Closed

refactor: convert global modulestore to request-cached #37859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def setUp(self):
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True)

self.transformers = BlockStructureTransformers([self.TRANSFORMER_CLASS_TO_TEST(False)])
self.clear_caches()

def setup_gated_section(self, gated_block, gating_block):
"""
Expand Down Expand Up @@ -184,7 +185,7 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c
self.user,
)

with self.assertNumQueries(4):
with self.assertNumQueries(5):
self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL)

def test_staff_access(self):
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/grades/tests/test_course_grade_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def test_grading_exception(self, mock_course_grade):
else mock_course_grade.return_value
for student in self.students
]
with self.assertNumQueries(20):
with self.assertNumQueries(21):
all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students)
assert {student: str(all_errors[student]) for student in all_errors} == {
student3: 'Error for student3.',
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/user_api/accounts/tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from openedx.core.djangolib.testing.utils import skip_unless_lms
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order

from ..utils import format_social_link, validate_social_link
Expand Down Expand Up @@ -63,7 +63,7 @@ def test_social_link_input(self, platform_name, link_input, formatted_link_expec


@ddt.ddt
class CompletionUtilsTestCase(SharedModuleStoreTestCase, CompletionWaffleTestMixin, TestCase):
class CompletionUtilsTestCase(ModuleStoreTestCase, CompletionWaffleTestMixin, TestCase):
"""
Test completion utility functions
"""
Expand Down
48 changes: 22 additions & 26 deletions xmodule/modulestore/django.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
import django.dispatch # lint-amnesty, pylint: disable=wrong-import-position
import django.utils # lint-amnesty, pylint: disable=wrong-import-position
from django.utils.translation import get_language, to_locale # lint-amnesty, pylint: disable=wrong-import-position
from edx_django_utils.cache import DEFAULT_REQUEST_CACHE # lint-amnesty, pylint: disable=wrong-import-position
from edx_django_utils.cache import RequestCache # lint-amnesty, pylint: disable=wrong-import-position

from openedx.core.lib.cache_utils import request_cached # pylint: disable=wrong-import-position
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-position
from xmodule.modulestore.draft_and_published import BranchSettingMixin # lint-amnesty, pylint: disable=wrong-import-position
from xmodule.modulestore.mixed import MixedModuleStore # lint-amnesty, pylint: disable=wrong-import-position
Expand Down Expand Up @@ -55,6 +56,8 @@
log = logging.getLogger(__name__)
ASSET_IGNORE_REGEX = getattr(settings, "ASSET_IGNORE_REGEX", r"(^\._.*$)|(^\.DS_Store$)|(^.*~$)")

MODULESTORE_REQUEST_CACHE_NAMESPACE = "modulestore"


class SwitchedSignal(django.dispatch.Signal):
"""
Expand Down Expand Up @@ -275,7 +278,7 @@ def create_modulestore_instance(
if key in _options and isinstance(_options[key], str):
_options[key] = load_function(_options[key])

request_cache = DEFAULT_REQUEST_CACHE
request_cache = RequestCache(MODULESTORE_REQUEST_CACHE_NAMESPACE)

try:
metadata_inheritance_cache = caches['mongo_metadata_inheritance']
Expand Down Expand Up @@ -324,33 +327,27 @@ def fetch_disabled_xblock_types():
)


# A singleton instance of the Mixed Modulestore
_MIXED_MODULESTORE = None


@request_cached(MODULESTORE_REQUEST_CACHE_NAMESPACE)
def modulestore():
"""
Returns the Mixed modulestore
"""
global _MIXED_MODULESTORE # pylint: disable=global-statement
if _MIXED_MODULESTORE is None:
_MIXED_MODULESTORE = create_modulestore_instance(
settings.MODULESTORE['default']['ENGINE'],
contentstore(),
settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}),
settings.MODULESTORE['default'].get('OPTIONS', {})
)

if settings.FEATURES.get('CUSTOM_COURSES_EDX'):
# TODO: This import prevents a circular import issue, but is
# symptomatic of a lib having a dependency on code in lms. This
# should be updated to have a setting that enumerates modulestore
# wrappers and then uses that setting to wrap the modulestore in
# appropriate wrappers depending on enabled features.
from lms.djangoapps.ccx.modulestore import CCXModulestoreWrapper
_MIXED_MODULESTORE = CCXModulestoreWrapper(_MIXED_MODULESTORE)
mixed_modulestore = create_modulestore_instance(
settings.MODULESTORE['default']['ENGINE'],
contentstore(),
settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}),
settings.MODULESTORE['default'].get('OPTIONS', {})
)
if settings.FEATURES.get('CUSTOM_COURSES_EDX'):
# TODO: This import prevents a circular import issue, but is
# symptomatic of a lib having a dependency on code in lms. This
# should be updated to have a setting that enumerates modulestore
# wrappers and then uses that setting to wrap the modulestore in
# appropriate wrappers depending on enabled features.
from lms.djangoapps.ccx.modulestore import CCXModulestoreWrapper
mixed_modulestore = CCXModulestoreWrapper(mixed_modulestore)

return _MIXED_MODULESTORE
return mixed_modulestore


def clear_existing_modulestores():
Expand All @@ -360,8 +357,7 @@ def clear_existing_modulestores():

This is useful for flushing state between unit tests.
"""
global _MIXED_MODULESTORE # pylint: disable=global-statement
_MIXED_MODULESTORE = None
RequestCache(MODULESTORE_REQUEST_CACHE_NAMESPACE).clear()


class XBlockI18nService:
Expand Down
Loading