diff --git a/README.md b/README.md index 6deaf1f..fc96f26 100644 --- a/README.md +++ b/README.md @@ -148,11 +148,11 @@ Feature flags: https://github.com/boxine/bx_django_utils/blob/master/bx_django_u #### bx_django_utils.feature_flags.admin_views -* [`ManageFeatureFlagsBaseView()`](https://github.com/boxine/bx_django_utils/blob/master/bx_django_utils/feature_flags/admin_views.py#L31-L101) - Base admin extra view to manage all existing feature flags in admin. +* [`ManageFeatureFlagsBaseView()`](https://github.com/boxine/bx_django_utils/blob/master/bx_django_utils/feature_flags/admin_views.py#L31-L105) - Base admin extra view to manage all existing feature flags in admin. #### bx_django_utils.feature_flags.data_classes -* [`FeatureFlag()`](https://github.com/boxine/bx_django_utils/blob/master/bx_django_utils/feature_flags/data_classes.py#L18-L176) - A feature flag that persistent the state into django cache/database. +* [`FeatureFlag()`](https://github.com/boxine/bx_django_utils/blob/master/bx_django_utils/feature_flags/data_classes.py#L21-L205) - A feature flag that persistent the state into django cache/database. #### bx_django_utils.feature_flags.test_utils diff --git a/bx_django_utils/feature_flags/README.md b/bx_django_utils/feature_flags/README.md index 8e53aec..34843e1 100644 --- a/bx_django_utils/feature_flags/README.md +++ b/bx_django_utils/feature_flags/README.md @@ -72,5 +72,7 @@ class ManageFeatureFlagsAdminExtraView(ManageFeatureFlagsBaseView): pass ``` +## Performance considerations - +By default, each time the flags state is evaluated (e.g. when calling `foo_feature_flag.is_enabled()`), the flag state is fetched from the underlying storage. This may cause poor performance in hot code paths. +You can limit this evaluation to once per n seconds by passing the `cache_duration=timedelta(seconds=n)` argument to the `FeatureFlag` constructor. diff --git a/bx_django_utils/feature_flags/admin_views.py b/bx_django_utils/feature_flags/admin_views.py index 53a4b66..9709b5c 100644 --- a/bx_django_utils/feature_flags/admin_views.py +++ b/bx_django_utils/feature_flags/admin_views.py @@ -84,6 +84,10 @@ def form_valid(self, form): feature_flag.set_state(new_state) change_message = f'Set "{feature_flag.human_name}" to {feature_flag.state.name}' + if hasattr(feature_flag, '_cache_duration'): + eta = int(feature_flag._cache_duration.total_seconds()) + change_message += f' (will take up to {eta} seconds to take full effect)' + # Create a LogEntry for this action: content_type_id = ContentType.objects.get_for_model(FeatureFlagModel).id log_entry = LogEntry.objects.log_action( diff --git a/bx_django_utils/feature_flags/data_classes.py b/bx_django_utils/feature_flags/data_classes.py index 1b24ec5..8c702fc 100644 --- a/bx_django_utils/feature_flags/data_classes.py +++ b/bx_django_utils/feature_flags/data_classes.py @@ -1,7 +1,9 @@ +import datetime import logging +import time from collections.abc import Iterable, Iterator from contextlib import contextmanager -from typing import Optional +from typing import Callable, Optional from django.core.cache import cache @@ -11,9 +13,10 @@ from bx_django_utils.feature_flags.utils import validate_cache_key from bx_django_utils.models.manipulate import create_or_update2 - logger = logging.getLogger(__name__) +DEFAULT_DURATION = datetime.timedelta(seconds=0) + class FeatureFlag: """ @@ -30,7 +33,11 @@ def __init__( initial_enabled: bool, description: Optional[str] = None, cache_key_prefix: str = 'feature-flags', + cache_duration: datetime.timedelta = DEFAULT_DURATION, ): + """ + :param cache_duration: how long the state of the flag should be cached in-process + """ self.human_name = human_name self.description = description @@ -50,6 +57,12 @@ def __init__( else: self.initial_state = State.DISABLED + if cache_duration: + self._cache_duration: datetime.timedelta = cache_duration + self._cache_time_func: Callable[[], float] = time.monotonic + self._cache_from: float = self._cache_time_func() + self._cache_value: State = self.initial_state + def enable(self) -> bool: return self.set_state(new_state=State.ENABLED) @@ -82,6 +95,22 @@ def reset(self) -> None: @property def is_enabled(self) -> bool: + if not hasattr(self, '_cache_duration'): # caching is disabled + return self._compute_is_enabled() + + elapsed = self._cache_time_func() - self._cache_from + + # cache is still valid + if elapsed <= self._cache_duration.total_seconds(): + return bool(self._cache_value.value) + + # cache is invalid -> recompute + state = self._compute_is_enabled() + self._cache_from = self._cache_time_func() + self._cache_value = State(state) + return state + + def _compute_is_enabled(self) -> bool: try: raw_value = cache.get(self.cache_key) except Exception as err: diff --git a/bx_django_utils/feature_flags/tests/test_feature_flags.py b/bx_django_utils/feature_flags/tests/test_feature_flags.py index ef4e269..c6916cf 100644 --- a/bx_django_utils/feature_flags/tests/test_feature_flags.py +++ b/bx_django_utils/feature_flags/tests/test_feature_flags.py @@ -1,3 +1,4 @@ +import datetime import logging from unittest.mock import patch @@ -249,6 +250,27 @@ def increment(): increment() self.assertEqual(some_var, 1) + def test_cache(self): + duration = datetime.timedelta(seconds=60) + ff = FeatureFlag( + cache_key='cache-test', + human_name='Cache Test', + initial_enabled=True, + cache_duration=duration, + ) + self.assertTrue(ff) + + ff.disable() + self.assertTrue(ff) # still enabled due to caching! + + orig_time_func = ff._cache_time_func + + def future(): + return orig_time_func() + duration.total_seconds() + 1 + + ff._cache_time_func = future + self.assertFalse(ff) # now it's disabled due to cache expiration + class IsolatedFeatureFlagsTestCase(FeatureFlagTestCaseMixin, TestCase): """""" # noqa - Don't add to README