Feature persistent caching #42
Conversation
growthbook/growthbook.py
Outdated
| @@ -248,19 +339,28 @@ async def _stop_session(self): | |||
|
|
|||
| class FeatureRepository(object): | |||
| def __init__(self) -> None: | |||
| self.cache: AbstractFeatureCache = InMemoryFeatureCache() | |||
| self.in_memory_cache: AbstractFeatureCache = InMemoryFeatureCache() | |||
There was a problem hiding this comment.
this will violate public interface of the class.
instead of squeezing more caches into single repo and forcing user to use (and setup) both, you can create another AbstractFeatureCache class (and name it something like LayeredFeatureCache) which should be able to combine as many caches user wants.
that aside I don't think that feature flag service needs multi-level cache, because flag should be as consistent as possible between processes and allowing multiple caches to serve data will make cache more variable.
There was a problem hiding this comment.
I agree that squeezing multiple caches directly into FeatureRepository isn't ideal. You're right, the LayeredFeatureCache approach is much cleaner and more flexible.
We've actually implemented LayeredFeatureCache as part of this. It allows us to combine different caching strategies (like in-memory and an external cache) under a single, consistent interface.
growthbook/growthbook.py
Outdated
| self.cache: Dict[str, CacheEntry] = {} | ||
|
|
||
| def _get_base_path(self) -> Path: | ||
| base_path = Path("./GrowthBook-Cache") |
There was a problem hiding this comment.
not configurable? looks like general directory but only this class knows about it for some reason
growthbook/growthbook.py
Outdated
| class FileFeatureCache(AbstractPersistentFeatureCache): | ||
| def __init__(self, cache_file: str): | ||
| self.cache_file = cache_file | ||
| self.cache: Dict[str, CacheEntry] = {} |
There was a problem hiding this comment.
maybe use protected variables? otherwise someone might use them as public API and we will stuck with them forever.
growthbook/growthbook.py
Outdated
| @@ -74,16 +91,14 @@ def set(self, key: str, value: Dict, ttl: int) -> None: | |||
| def clear(self) -> None: | |||
| pass | |||
|
|
|||
| class AbstractPersistentFeatureCache(AbstractFeatureCache): | |||
| @abstractmethod | |||
| def load(self) -> None: | |||
There was a problem hiding this comment.
looks like implementation detail which could be hidden and called from constructor or other methods
…est_multiple_instances_get_updated_on_cache_expiry tests
# Conflicts: # growthbook/growthbook.py
This PR implements two-level caching for GrowthBook feature data in the Python SDK:
On initialization, GrowthBook loads features from persistent file-based cache (if available and not expired).
The loaded data is stored in the in-memory cache and used for feature evaluations.
No network request is made unless the cached data is expired or missing.