#209: Add comments and move code to a more logical location

datagateway_api/common/icat/helpers.py

@@ -27,7 +27,7 @@
     PythonICATLimitFilter,
     PythonICATWhereFilter,
 )
-from datagateway_api.common.icat.icat_client_pool import ExtendedLRUCache
+from datagateway_api.common.icat.lru_cache import ExtendedLRUCache
 from datagateway_api.common.icat.query import ICATQuery
 
 
@@ -79,7 +79,20 @@ def wrapper_requires_session(*args, **kwargs):
 @cached(cache=ExtendedLRUCache())
 def get_cached_client(session_id, client_pool):
     """
-    TODO - Add docstring
+    Get a client from cache using session ID as the cache parameter (client_pool will
+    always be given the same object, so won't impact on argument hashing)
+
+    An available client is fetched from the object pool, given a session ID, and kept
+    around in this cache until it becomes 'least recently used'. At this point, the
+    session ID is flushed and the client is returned to the pool. More details about
+    client handling can be found in the README
+
+    TODO - Add details of client handling in the README
+
+    :param session_id: The user's session ID
+    :type session_id: :class:`str`
+    :param client_pool: Client object pool used to fetch an unused client
+    :type client_pool: :class:`ObjectPool`
     """
 
     # Get a client from the pool

datagateway_api/common/icat/icat_client_pool.py

@@ -1,8 +1,5 @@
 import logging
 
-from cachetools.func import _cache
-from cachetools.lru import LRUCache
-
 from icat.client import Client
 from object_pool import ObjectPool
 
@@ -16,6 +13,7 @@ class ICATClient(Client):
 
     def __init__(self):
         super().__init__(config.get_icat_url(), checkCert=config.get_icat_check_cert())
+        # When clients are cleaned up, sessions won't be logged out
         self.autoLogout = False
 
     def clean_up(self):
@@ -27,6 +25,12 @@ def clean_up(self):
 
 
 def create_client_pool():
+    """
+    Function to create an object pool for ICAT client objects
+
+    The ObjectPool class uses the singleton design pattern
+    """
+
     return ObjectPool(
         ICATClient, min_init=5, max_capacity=20, max_reusable=0, expires=0,
     )
@@ -50,20 +54,3 @@ def release_client(self):
 
 def get_executor(client_pool):
     return ClientPoolExecutor(client_pool)
-
-
-class ExtendedLRUCache(LRUCache):
-    def __init__(self):
-        super().__init__(maxsize=8)
-
-    def popitem(self):
-        key, client = super().popitem()
-        session_id, client_pool = key
-        log.debug(f"Item popped from LRU cache: {key}, {client}")
-        # Put client back into pool
-        # Passes in default stats for now, though these aren't used in the API
-        client_pool._queue_resource(client, client_pool._get_default_stats())
-
-
-def my_lru_cache():
-    return _cache(ExtendedLRUCache())

datagateway_api/common/icat/lru_cache.py

@@ -0,0 +1,29 @@
+import logging
+
+from cachetools.lru import LRUCache
+
+log = logging.getLogger()
+
+
+class ExtendedLRUCache(LRUCache):
+    """
+    An extension to cachetools' LRUCache class to allow client objects to be pushed back
+    into the pool
+
+    This version of LRU cache was chosen instead of the builtin LRU cache as it allows
+    for addtional actions to be added when an item leaves the cache (controlled by
+    `popitem()`). Since the builtin version was just a function (using a couple of
+    wrapper functions), adding additional functionality wasn't possible.
+    """
+
+    def __init__(self):
+        super().__init__(maxsize=8)
+
+    def popitem(self):
+        key, client = super().popitem()
+        session_id, client_pool = key
+        log.debug(f"Item popped from LRU cache: {key}, {client}")
+        # TODO - Session ID should probably get flushed here?
+        # Put client back into pool
+        # Passes in default stats for now, though these aren't used in the API
+        client_pool._queue_resource(client, client_pool._get_default_stats())