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

Merge pull request #26017 from regisb/regisb/refactor_set_request_cache

Browse Source 
[BD-21] Get rid of calls to `set_request_cache_with_short_name`
This commit is contained in:
Robert Raposa
2021-01-13 09:57:13 -05:00
committed by GitHub
parent 50b705405e 3a29cff016
commit e0aa459732
11 changed files with 50 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
lms/djangoapps/course_api/blocks/tests/test_api.py
View File

@@ -17,7 +17,7 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls
from xmodule.modulestore.tests.sample_courses import BlockInfo
from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache
from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle_switch
from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE
from ..api import get_blocks
@@ -230,7 +230,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
)
@ddt.unpack
def test_query_counts_cached(self, store_type, with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
course = self._create_course(store_type)
self._get_blocks(
course,
@@ -247,7 +247,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
@ddt.unpack
def test_query_counts_uncached(self, store_type_tuple, with_storage_backing):
store_type, expected_mongo_queries = store_type_tuple
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
course = self._create_course(store_type)
clear_course_from_cache(course.id)

37
openedx/core/djangoapps/content/block_structure/config/__init__.py
View File

@@ -2,37 +2,32 @@
This module contains various configuration settings via
waffle switches for the Block Structure framework.
"""
from edx_django_utils.cache import RequestCache
from edx_toggles.toggles.__future__ import WaffleSwitch
from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleSwitchNamespace
from openedx.core.lib.cache_utils import request_cached
from .models import BlockStructureConfiguration
# Namespace
WAFFLE_NAMESPACE = u'block_structure'
# Switches
INVALIDATE_CACHE_ON_PUBLISH = u'invalidate_cache_on_publish'
STORAGE_BACKING_FOR_CACHE = u'storage_backing_for_cache'
RAISE_ERROR_WHEN_NOT_FOUND = u'raise_error_when_not_found'
INVALIDATE_CACHE_ON_PUBLISH = WaffleSwitch(
"block_structure.invalidate_cache_on_publish", __name__
)
STORAGE_BACKING_FOR_CACHE = WaffleSwitch(
"block_structure.storage_backing_for_cache", __name__
)
RAISE_ERROR_WHEN_NOT_FOUND = WaffleSwitch(
"block_structure.raise_error_when_not_found", __name__
)
def waffle():
def enable_storage_backing_for_cache_in_request():
"""
Returns the namespaced and cached Waffle class for BlockStructures.
Manually override the value of the STORAGE_BACKING_FOR_CACHE switch in the context of the request.
This function should not be replicated, as it accesses a protected member, and it shouldn't.
"""
return LegacyWaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix=u'BlockStructure: ')
def waffle_switch(name):
"""
Return the waffle switch associated to this namespace.
WARNING: do not replicate this pattern. Instead of declaring waffle switch names as strings, you should create
LegacyWaffleSwitch objects as top-level constants.
"""
return LegacyWaffleSwitch(waffle(), name, module_name=__name__)
# pylint: disable=protected-access
STORAGE_BACKING_FOR_CACHE._cached_switches[STORAGE_BACKING_FOR_CACHE.name] = True
@request_cached()

5
openedx/core/djangoapps/content/block_structure/management/commands/generate_course_blocks.py
View File

@@ -7,13 +7,12 @@ import logging
import six
from django.core.management.base import BaseCommand
import six
from six import text_type
import openedx.core.djangoapps.content.block_structure.api as api
import openedx.core.djangoapps.content.block_structure.store as store
import openedx.core.djangoapps.content.block_structure.tasks as tasks
from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle
from openedx.core.djangoapps.content.block_structure.config import enable_storage_backing_for_cache_in_request
from openedx.core.lib.command_utils import (
get_mutually_exclusive_required_option,
parse_course_keys,
@@ -133,7 +132,7 @@ class Command(BaseCommand):
Generates course blocks for the given course_keys per the given options.
"""
if options.get('with_storage'):
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
enable_storage_backing_for_cache_in_request()
for course_key in course_keys:
try:

5
openedx/core/djangoapps/content/block_structure/manager.py
View File

@@ -102,10 +102,9 @@ class BlockStructureManager(object):
BlockStructureTransformers.verify_versions(block_structure)
except (BlockStructureNotFound, TransformerDataIncompatible):
if config.waffle().is_enabled(config.RAISE_ERROR_WHEN_NOT_FOUND):
if config.RAISE_ERROR_WHEN_NOT_FOUND.is_enabled():
raise
else:
block_structure = self._update_collected()
block_structure = self._update_collected()
return block_structure

2
openedx/core/djangoapps/content/block_structure/signals.py
View File

@@ -29,7 +29,7 @@ def update_block_structure_on_course_publish(sender, course_key, **kwargs): # p
if isinstance(course_key, LibraryLocator):
return
if config.waffle().is_enabled(config.INVALIDATE_CACHE_ON_PUBLISH):
if config.INVALIDATE_CACHE_ON_PUBLISH.is_enabled():
try:
clear_course_from_cache(course_key)
except BlockStructureNotFound:

31
openedx/core/djangoapps/content/block_structure/store.py
View File

@@ -7,8 +7,8 @@ Module for the Storage of BlockStructure objects.
from logging import getLogger
import six
from django.utils.encoding import python_2_unicode_compatible
from openedx.core.lib.cache_utils import zpickle, zunpickle
from . import config
@@ -28,6 +28,7 @@ class StubModel(object):
By using this stub, we eliminate the need for extra
conditional statements in the code.
"""
def __init__(self, root_block_usage_key):
self.data_usage_key = root_block_usage_key
@@ -45,6 +46,7 @@ class BlockStructureStore(object):
"""
Storage for BlockStructure objects.
"""
def __init__(self, cache):
"""
Arguments:
@@ -121,7 +123,7 @@ class BlockStructureStore(object):
Returns whether the data in storage for the given key is
already up-to-date with the version in the given modulestore.
"""
if _is_storage_backing_enabled():
if config.STORAGE_BACKING_FOR_CACHE.is_enabled():
try:
bs_model = self._get_model(root_block_usage_key)
root_block = modulestore.get_item(root_block_usage_key)
@@ -135,7 +137,7 @@ class BlockStructureStore(object):
"""
Returns the model associated with the given key.
"""
if _is_storage_backing_enabled():
if config.STORAGE_BACKING_FOR_CACHE.is_enabled():
return BlockStructureModel.get(root_block_usage_key)
else:
return StubModel(root_block_usage_key)
@@ -145,7 +147,7 @@ class BlockStructureStore(object):
Updates or creates the model for the given block_structure
and serialized_data.
"""
if _is_storage_backing_enabled():
if config.STORAGE_BACKING_FOR_CACHE.is_enabled():
root_block = block_structure[block_structure.root_block_usage_key]
bs_model, _ = BlockStructureModel.update_or_create(
serialized_data,
@@ -187,7 +189,7 @@ class BlockStructureStore(object):
Raises:
BlockStructureNotFound if not found.
"""
if not _is_storage_backing_enabled():
if not config.STORAGE_BACKING_FOR_CACHE.is_enabled():
raise BlockStructureNotFound(bs_model.data_usage_key)
return bs_model.get_serialized_data()
@@ -229,14 +231,12 @@ class BlockStructureStore(object):
Returns the cache key to use for the given
BlockStructureModel or StubModel.
"""
if _is_storage_backing_enabled():
if config.STORAGE_BACKING_FOR_CACHE.is_enabled():
return six.text_type(bs_model)
else:
return "v{version}.root.key.{root_usage_key}".format(
version=six.text_type(BlockStructureBlockData.VERSION),
root_usage_key=six.text_type(bs_model.data_usage_key),
)
return "v{version}.root.key.{root_usage_key}".format(
version=six.text_type(BlockStructureBlockData.VERSION),
root_usage_key=six.text_type(bs_model.data_usage_key),
)
@staticmethod
def _version_data_of_block(root_block):
@@ -260,10 +260,3 @@ class BlockStructureStore(object):
field_name: getattr(bs_model, field_name, None)
for field_name in BlockStructureModel.VERSION_FIELDS
}
def _is_storage_backing_enabled():
"""
Returns whether storage backing for Block Structures is enabled.
"""
return config.waffle().is_enabled(config.STORAGE_BACKING_FOR_CACHE)

6
openedx/core/djangoapps/content/block_structure/tasks.py
View File

@@ -14,7 +14,7 @@ from opaque_keys.edx.keys import CourseKey
from capa.responsetypes import LoncapaProblemError
from openedx.core.djangoapps.content.block_structure import api
from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle
from openedx.core.djangoapps.content.block_structure.config import enable_storage_backing_for_cache_in_request
from xmodule.modulestore.exceptions import ItemNotFoundError
log = logging.getLogger('edx.celery.task')
@@ -63,7 +63,7 @@ def _update_course_in_cache(self, **kwargs):
Updates the course blocks (mongo -> BlockStructure) for the specified course.
"""
if kwargs.get('with_storage'):
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
enable_storage_backing_for_cache_in_request()
_call_and_retry_if_needed(self, api.update_course_in_cache, **kwargs)
@@ -94,7 +94,7 @@ def _get_course_in_cache(self, **kwargs):
Gets the course blocks for the specified course, updating the cache if needed.
"""
if kwargs.get('with_storage'):
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
enable_storage_backing_for_cache_in_request()
_call_and_retry_if_needed(self, api.get_course_in_cache, **kwargs)

6
openedx/core/djangoapps/content/block_structure/tests/test_manager.py
View File

@@ -9,7 +9,7 @@ from django.test import TestCase
from edx_toggles.toggles.testutils import override_waffle_switch
from ..block_structure import BlockStructureBlockData
from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE, waffle_switch
from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE
from ..exceptions import BlockStructureNotFound, UsageKeyNotInBlockStructure
from ..manager import BlockStructureManager
from ..transformers import BlockStructureTransformers
@@ -179,14 +179,14 @@ class TestBlockStructureManager(UsageKeyFactoryMixin, ChildrenMapTestMixin, Test
assert TestTransformer1.collect_call_count == 1
def test_get_collected_error_raised(self):
with override_waffle_switch(waffle_switch(RAISE_ERROR_WHEN_NOT_FOUND), active=True):
with override_waffle_switch(RAISE_ERROR_WHEN_NOT_FOUND, active=True):
with mock_registered_transformers(self.registered_transformers):
with self.assertRaises(BlockStructureNotFound):
self.bs_manager.get_collected()
@ddt.data(True, False)
def test_update_collected_if_needed(self, with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with mock_registered_transformers(self.registered_transformers):
assert TestTransformer1.collect_call_count == 0

4
openedx/core/djangoapps/content/block_structure/tests/test_signals.py
View File

@@ -12,7 +12,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from ..api import get_block_structure_manager
from ..config import INVALIDATE_CACHE_ON_PUBLISH, waffle_switch
from ..config import INVALIDATE_CACHE_ON_PUBLISH
from ..signals import update_block_structure_on_course_publish
from .helpers import is_course_in_block_structure_cache
@@ -56,7 +56,7 @@ class CourseBlocksSignalTest(ModuleStoreTestCase):
def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear):
test_display_name = "Jedi 101"
with override_waffle_switch(waffle_switch(INVALIDATE_CACHE_ON_PUBLISH), active=invalidate_cache_enabled):
with override_waffle_switch(INVALIDATE_CACHE_ON_PUBLISH, active=invalidate_cache_enabled):
self.course.display_name = test_display_name
self.store.update_item(self.course, self.user.id)

10
openedx/core/djangoapps/content/block_structure/tests/test_store.py
View File

@@ -8,7 +8,7 @@ from edx_toggles.toggles.testutils import override_waffle_switch
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from ..config import STORAGE_BACKING_FOR_CACHE, waffle_switch
from ..config import STORAGE_BACKING_FOR_CACHE
from ..config.models import BlockStructureConfiguration
from ..exceptions import BlockStructureNotFound
from ..store import BlockStructureStore
@@ -48,13 +48,13 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI
@ddt.data(True, False)
def test_get_none(self, with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with self.assertRaises(BlockStructureNotFound):
self.store.get(self.block_structure.root_block_usage_key)
@ddt.data(True, False)
def test_add_and_get(self, with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
self.store.add(self.block_structure)
stored_value = self.store.get(self.block_structure.root_block_usage_key)
self.assertIsNotNone(stored_value)
@@ -62,7 +62,7 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI
@ddt.data(True, False)
def test_delete(self, with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
self.store.add(self.block_structure)
self.store.delete(self.block_structure.root_block_usage_key)
with self.assertRaises(BlockStructureNotFound):
@@ -75,7 +75,7 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI
self.store.get(self.block_structure.root_block_usage_key)
def test_uncached_with_storage(self):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=True):
with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=True):
self.store.add(self.block_structure)
self.mock_cache.map.clear()
stored_value = self.store.get(self.block_structure.root_block_usage_key)

4
openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py
View File

@@ -500,9 +500,7 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
self.assertEqual(len(submitted), len(self.course_strings))
# simulate one of the courses being published
with override_waffle_switch(
block_structure_config.waffle_switch(block_structure_config.STORAGE_BACKING_FOR_CACHE), True
):
with override_waffle_switch(block_structure_config.STORAGE_BACKING_FOR_CACHE, True):
update_block_structure_on_course_publish(None, self.course.id)
# make sure only the published course was dumped