Deprecate WaffleSwitch.override* methods

This allows us to get rid of the custom WaffleSwitch and
WaffleSwitchNamespace classes from waffle_utils in favour of
edx_toggles.toggles classes.
This commit is contained in:
Régis Behmo
2020-10-23 13:06:12 +02:00
parent 2307dff4c9
commit 3b127f8c92
26 changed files with 159 additions and 203 deletions

View File

@@ -13,6 +13,8 @@ from course_modes.models import CourseMode
from openedx.core.djangoapps.certificates import api
from openedx.core.djangoapps.certificates.config import waffle as certs_waffle
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangoapps.waffle_utils import WaffleSwitch
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from student.tests.factories import CourseEnrollmentFactory, UserFactory
@@ -66,8 +68,8 @@ class MockGeneratedCertificate(object):
@contextmanager
def configure_waffle_namespace(feature_enabled):
namespace = certs_waffle.waffle()
with namespace.override(certs_waffle.AUTO_CERTIFICATE_GENERATION, active=feature_enabled):
auto_certificate_generation_switch = WaffleSwitch(namespace, certs_waffle.AUTO_CERTIFICATE_GENERATION)
with override_waffle_switch(auto_certificate_generation_switch, active=feature_enabled):
yield

View File

@@ -4,7 +4,7 @@ waffle switches for the Block Structure framework.
"""
from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace
from openedx.core.djangoapps.waffle_utils import WaffleSwitch, WaffleSwitchNamespace
from openedx.core.lib.cache_utils import request_cached
from .models import BlockStructureConfiguration
@@ -25,6 +25,16 @@ def waffle():
return WaffleSwitchNamespace(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
WaffleSwitch objects as top-level constants.
"""
return WaffleSwitch(waffle(), name, module_name=__name__)
@request_cached()
def num_versions_to_keep():
"""

View File

@@ -133,7 +133,7 @@ class Command(BaseCommand):
Generates course blocks for the given course_keys per the given options.
"""
if options.get('with_storage'):
waffle().override_for_request(STORAGE_BACKING_FOR_CACHE)
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
for course_key in course_keys:
try:

View File

@@ -60,7 +60,7 @@ def _update_course_in_cache(self, **kwargs):
Updates the course blocks (mongo -> BlockStructure) for the specified course.
"""
if kwargs.get('with_storage'):
waffle().override_for_request(STORAGE_BACKING_FOR_CACHE)
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
_call_and_retry_if_needed(self, api.update_course_in_cache, **kwargs)
@@ -89,7 +89,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().override_for_request(STORAGE_BACKING_FOR_CACHE)
waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True)
_call_and_retry_if_needed(self, api.get_course_in_cache, **kwargs)

View File

@@ -7,8 +7,10 @@ import ddt
import six
from django.test import TestCase
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from ..block_structure import BlockStructureBlockData
from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE, waffle
from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE, waffle_switch
from ..exceptions import BlockStructureNotFound, UsageKeyNotInBlockStructure
from ..manager import BlockStructureManager
from ..transformers import BlockStructureTransformers
@@ -178,14 +180,14 @@ class TestBlockStructureManager(UsageKeyFactoryMixin, ChildrenMapTestMixin, Test
assert TestTransformer1.collect_call_count == 1
def test_get_collected_error_raised(self):
with waffle().override(RAISE_ERROR_WHEN_NOT_FOUND, active=True):
with override_waffle_switch(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 waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing):
with mock_registered_transformers(self.registered_transformers):
assert TestTransformer1.collect_call_count == 0

View File

@@ -7,12 +7,13 @@ import ddt
from mock import patch
from opaque_keys.edx.locator import CourseLocator, LibraryLocator
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from xmodule.modulestore.exceptions import ItemNotFoundError
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
from ..config import INVALIDATE_CACHE_ON_PUBLISH, waffle_switch
from ..signals import update_block_structure_on_course_publish
from .helpers import is_course_in_block_structure_cache
@@ -56,7 +57,7 @@ class CourseBlocksSignalTest(ModuleStoreTestCase):
def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear):
test_display_name = "Jedi 101"
with waffle().override(INVALIDATE_CACHE_ON_PUBLISH, active=invalidate_cache_enabled):
with override_waffle_switch(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)

View File

@@ -5,9 +5,10 @@ Tests for block_structure/cache.py
import ddt
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from ..config import STORAGE_BACKING_FOR_CACHE, waffle
from ..config import STORAGE_BACKING_FOR_CACHE, waffle_switch
from ..config.models import BlockStructureConfiguration
from ..exceptions import BlockStructureNotFound
from ..store import BlockStructureStore
@@ -47,13 +48,13 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI
@ddt.data(True, False)
def test_get_none(self, with_storage_backing):
with waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with override_waffle_switch(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 waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with override_waffle_switch(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)
@@ -61,7 +62,7 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI
@ddt.data(True, False)
def test_delete(self, with_storage_backing):
with waffle().override(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
with override_waffle_switch(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):
@@ -74,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 waffle().override(STORAGE_BACKING_FOR_CACHE, active=True):
with override_waffle_switch(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)

View File

@@ -10,30 +10,23 @@ import ddt
import mock
import six
from django.core.management import call_command
import openedx.core.djangoapps.content.block_structure.config as block_structure_config
from openedx.core.djangoapps.content.block_structure.signals import update_block_structure_on_course_publish
from openedx.core.djangoapps.coursegraph.management.commands.dump_to_neo4j import ModuleStoreSerializer
from openedx.core.djangoapps.coursegraph.management.commands.tests.utils import MockGraph, MockNodeSelector
from openedx.core.djangoapps.coursegraph.tasks import (
coerce_types,
serialize_course,
serialize_item,
should_dump_course,
strip_branch_and_version
)
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from openedx.core.djangolib.testing.utils import skip_unless_lms
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.core.djangoapps.coursegraph.management.commands.dump_to_neo4j import (
ModuleStoreSerializer
)
from openedx.core.djangoapps.coursegraph.management.commands.tests.utils import (
MockGraph,
MockNodeSelector,
)
from openedx.core.djangoapps.coursegraph.tasks import (
serialize_item,
serialize_course,
coerce_types,
should_dump_course,
strip_branch_and_version,
)
from openedx.core.djangoapps.content.block_structure.signals import (
update_block_structure_on_course_publish
)
import openedx.core.djangoapps.content.block_structure.config as block_structure_config
class TestDumpToNeo4jCommandBase(SharedModuleStoreTestCase):
"""
@@ -507,7 +500,9 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase):
self.assertEqual(len(submitted), len(self.course_strings))
# simulate one of the courses being published
with block_structure_config.waffle().override(block_structure_config.STORAGE_BACKING_FOR_CACHE):
with override_waffle_switch(
block_structure_config.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

View File

@@ -32,6 +32,7 @@ from openedx.core.djangoapps.user_authn.views.login import (
AllowedAuthUser,
_check_user_auth_flow
)
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_switch
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.core.lib.api.test_utils import ApiTestCase
@@ -725,7 +726,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
'THIRD_PARTY_AUTH_ONLY_HINT': provider_tpa_hint,
}
with ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.override(switch_enabled):
with override_waffle_switch(ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY, switch_enabled):
if not is_third_party_authenticated:
site = self.set_up_site(allowed_domain, default_site_configuration_values)
@@ -778,7 +779,7 @@ class LoginTest(SiteMixin, CacheIsolationTestCase):
'THIRD_PARTY_AUTH_ONLY_HINT': provider_tpa_hint,
}
with ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.override(True):
with override_waffle_switch(ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY, True):
site = self.set_up_site(allowed_domain, default_site_configuration_values)
with self.assertLogs(level='WARN') as log:

View File

@@ -3,97 +3,15 @@ Extra utilities for waffle: most classes are defined in edx_toggles.toggles (htt
we keep here some extra classes for usage within edx-platform. These classes cover course override use cases.
"""
import logging
from contextlib import contextmanager
from opaque_keys.edx.keys import CourseKey
from edx_toggles.toggles import WaffleFlag, WaffleFlagNamespace
from edx_toggles.toggles import WaffleSwitch as BaseWaffleSwitch
from edx_toggles.toggles import WaffleSwitchNamespace as BaseWaffleSwitchNamespace
# pylint: disable=unused-import
from edx_toggles.toggles import WaffleFlag, WaffleFlagNamespace, WaffleSwitch, WaffleSwitchNamespace
log = logging.getLogger(__name__)
class WaffleSwitchNamespace(BaseWaffleSwitchNamespace):
"""
Waffle switch namespace that implements custom overriding methods. We should eventually get rid of this class.
To test WaffleSwitchNamespace, use the provided context managers. For example:
with WAFFLE_SWITCHES.override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=True):
...
Note: this should eventually be deprecated in favour of a dedicated `override_waffle_switch` context manager.
"""
@contextmanager
def override(self, switch_name, active=True):
"""
Overrides the active value for the given switch for the duration of this
contextmanager.
Note: The value is overridden in the request cache AND in the model.
"""
previous_active = self.is_enabled(switch_name)
try:
self.override_for_request(switch_name, active)
with self.override_in_model(switch_name, active):
yield
finally:
self.override_for_request(switch_name, previous_active)
def override_for_request(self, switch_name, active=True):
"""
Overrides the active value for the given switch for the remainder of
this request (as this is not a context manager).
Note: The value is overridden in the request cache, not in the model.
"""
namespaced_switch_name = self._namespaced_name(switch_name)
self._cached_switches[namespaced_switch_name] = active
log.info(
"%sSwitch '%s' set to %s for request.",
self.log_prefix,
namespaced_switch_name,
active,
)
@contextmanager
def override_in_model(self, switch_name, active=True):
"""
Overrides the active value for the given switch for the duration of this
contextmanager.
Note: The value is overridden in the model, not the request cache.
Note: This should probably be moved to a test class.
"""
# Import is placed here to avoid model import at project startup.
# pylint: disable=import-outside-toplevel
from waffle.testutils import override_switch as waffle_override_switch
namespaced_switch_name = self._namespaced_name(switch_name)
with waffle_override_switch(namespaced_switch_name, active):
log.info(
"%sSwitch '%s' set to %s in model.",
self.log_prefix,
namespaced_switch_name,
active,
)
yield
class WaffleSwitch(BaseWaffleSwitch):
"""
This class should be removed in favour of edx_toggles.toggles.WaffleSwitch once we get rid of the
WaffleSwitchNamespace class.
"""
NAMESPACE_CLASS = WaffleSwitchNamespace
@contextmanager
def override(self, active=True):
with self.waffle_namespace.override(self.switch_name, active):
yield
class CourseWaffleFlag(WaffleFlag):
"""
Represents a single waffle flag that can be forced on/off for a course. This class should be used instead of

View File

@@ -7,7 +7,8 @@ import ddt
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
# Note that we really shouldn't import from edx_toggles' internal API
# TODO: we really shouldn't import from edx_toggles' internal API, but that's currently the only way to mock the
# monitoring functions.
import edx_toggles.toggles.internal.waffle
from edx_django_utils.cache import RequestCache
from mock import call, patch

View File

@@ -5,7 +5,7 @@ Test utilities for waffle utilities.
# Import from edx-toggles to preserve import paths
# TODO: Deprecate and remove
# pylint: disable=unused-import
from edx_toggles.toggles.testutils import override_waffle_flag
from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch
# Can be used with FilteredQueryCountMixin.assertNumQueries() to blacklist
# waffle tables. For example: