Add read lock to get_flag_keys method (#35)

Author: schmit

eppo_client/__init__.py

@@ -5,7 +5,6 @@
     ExperimentConfigurationRequestor,
 )
 from eppo_client.configuration_store import ConfigurationStore
-from eppo_client.constants import MAX_CACHE_ENTRIES
 from eppo_client.http_client import HttpClient, SdkParams
 from eppo_client.models import Flag
 from eppo_client.read_write_lock import ReadWriteLock
@@ -31,18 +30,15 @@ def init(config: Config) -> EppoClient:
         apiKey=config.api_key, sdkName="python", sdkVersion=__version__
     )
     http_client = HttpClient(base_url=config.base_url, sdk_params=sdk_params)
-    config_store: ConfigurationStore[Flag] = ConfigurationStore(
-        max_size=MAX_CACHE_ENTRIES
-    )
+    config_store: ConfigurationStore[Flag] = ConfigurationStore()
     config_requestor = ExperimentConfigurationRequestor(
         http_client=http_client, config_store=config_store
     )
     assignment_logger = config.assignment_logger
     is_graceful_mode = config.is_graceful_mode
     global __client
     global __lock
-    try:
-        __lock.acquire_write()
+    with __lock.writer():
         if __client:
             # if a client was already initialized, stop the background processes of the old client
             __client._shutdown()
@@ -52,8 +48,6 @@ def init(config: Config) -> EppoClient:
             is_graceful_mode=is_graceful_mode,
         )
         return __client
-    finally:
-        __lock.release_write()
 
 
 def get_instance() -> EppoClient:
@@ -67,11 +61,8 @@ def get_instance() -> EppoClient:
     """
     global __client
     global __lock
-    try:
-        __lock.acquire_read()
+    with __lock.reader():
         if __client:
             return __client
         else:
             raise Exception("init() must be called before get_instance()")
-    finally:
-        __lock.release_read()

eppo_client/configuration_store.py

@@ -1,33 +1,23 @@
 from typing import Dict, Optional, TypeVar, Generic
-from cachetools import LRUCache
 
 from eppo_client.read_write_lock import ReadWriteLock
 
 T = TypeVar("T")
 
 
 class ConfigurationStore(Generic[T]):
-    def __init__(self, max_size: int):
-        self.__cache: LRUCache = LRUCache(maxsize=max_size)
+    def __init__(self):
+        self.__cache: Dict[str, T] = {}
         self.__lock = ReadWriteLock()
 
     def get_configuration(self, key: str) -> Optional[T]:
-        try:
-            self.__lock.acquire_read()
-            return self.__cache[key]
-        except KeyError:
-            return None  # key does not exist
-        finally:
-            self.__lock.release_read()
+        with self.__lock.reader():
+            return self.__cache.get(key, None)
 
     def set_configurations(self, configs: Dict[str, T]):
-        try:
-            self.__lock.acquire_write()
-            self.__cache.clear()
-            for key, config in configs.items():
-                self.__cache[key] = config
-        finally:
-            self.__lock.release_write()
+        with self.__lock.writer():
+            self.__cache = configs
 
     def get_keys(self):
-        return list(self.__cache.keys())
+        with self.__lock.reader():
+            return list(self.__cache.keys())

eppo_client/constants.py

@@ -1,6 +1,3 @@
-# configuration cache
-MAX_CACHE_ENTRIES = 1000  # arbitrary; the caching library requires a max limit
-
 # poller
 SECOND_MILLIS = 1000
 MINUTE_MILLIS = 60 * SECOND_MILLIS

eppo_client/read_write_lock.py

@@ -1,6 +1,7 @@
 import threading
+from contextlib import contextmanager
 
-# Copied from: https://www.oreilly.com/library/view/python-cookbook/0596001673/ch06s04.html
+# Adapted from: https://www.oreilly.com/library/view/python-cookbook/0596001673/ch06s04.html
 
 
 class ReadWriteLock:
@@ -14,21 +15,15 @@ def __init__(self):
     def acquire_read(self):
         """Acquire a read lock. Blocks only if a thread has
         acquired the write lock."""
-        self._read_ready.acquire()
-        try:
+        with self._read_ready:
             self._readers += 1
-        finally:
-            self._read_ready.release()
 
     def release_read(self):
         """Release a read lock."""
-        self._read_ready.acquire()
-        try:
+        with self._read_ready:
             self._readers -= 1
             if not self._readers:
                 self._read_ready.notify_all()
-        finally:
-            self._read_ready.release()
 
     def acquire_write(self):
         """Acquire a write lock. Blocks until there are no
@@ -40,3 +35,19 @@ def acquire_write(self):
     def release_write(self):
         """Release a write lock."""
         self._read_ready.release()
+
+    @contextmanager
+    def reader(self):
+        try:
+            self.acquire_read()
+            yield
+        finally:
+            self.release_read()
+
+    @contextmanager
+    def writer(self):
+        try:
+            self.acquire_write()
+            yield
+        finally:
+            self.release_write()

eppo_client/version.py

@@ -1 +1 @@
-__version__ = "3.0.1"
+__version__ = "3.0.2"

test/configuration_store_test.py

@@ -5,7 +5,7 @@
 
 TEST_MAX_SIZE = 10
 
-store: ConfigurationStore[str] = ConfigurationStore(max_size=TEST_MAX_SIZE)
+store: ConfigurationStore[str] = ConfigurationStore()
 mock_flag = Flag(
     key="mock_flag",
     variation_type=VariationType.STRING,
@@ -40,7 +40,7 @@ def test_evicts_old_entries_when_max_size_exceeded():
 
 
 def test_evicts_old_entries_when_setting_new_flags():
-    store: ConfigurationStore[str] = ConfigurationStore(max_size=TEST_MAX_SIZE)
+    store: ConfigurationStore[str] = ConfigurationStore()
 
     store.set_configurations({"flag": mock_flag, "second_flag": mock_flag})
     assert store.get_configuration("flag") == mock_flag