Merge branch 'devel'

Author: ehrenfeu

CHANGELOG.md

@@ -4,6 +4,33 @@
 
 NOTE: potentially breaking changes are flagged with a 🧨 symbol.
 
+## 2.3.0
+
+### Added
+
+- `pyppms.ppms.PpmsConnection()` now takes an optional parameter
+  `cache_users_only` that will prevent any request but `getuser` from being
+  stored in the local cache. This is useful in scenarios where frequent requests
+  to PPMS are being done to fetch booking states and such that would be slowed
+  down enormously if no user caching was present. Obviously the cached users
+  need to be refreshed explicitly on a regular basis then. Defaults to `False`
+  which will result in the same behavior as before.
+  Please note that several things are implicitly being cached (in memory) during
+  the lifetime of the `PpmsConnection` object (e.g. the PPMS systems) unless
+  their corresponding method is being called with `force_refresh=True`.
+- `pyppms.ppms.PpmsConnection.update_users()` and
+  `pyppms.ppms.PpmsConnection.get_users()` now both have an optional parameter
+  `active_only` (defaulting to `True`) that can be used to also request users
+  that are marked as *inactive* in PPMS.
+
+### Changed
+
+- `pyppms.ppms.PpmsConnection.get_user()` is only logging a `DEBUG` level
+  message (before: `ERROR`) in case the requested user can't be found since it
+  also raises a `KeyError`. This is done to prevent cluttering up the logs of
+  calling code that might use this method to figure out if an account exists in
+  PPMS and properly deals with the exception raised.
+
 ## 2.2.0
 
 ### Added

TODO.md

@@ -1,5 +1,7 @@
 # PyPPMS Development ToDos
 
+- explain cache setup for a production scenario (`cache_users_only` and regular
+  explicit cache refreshes for example)
 - all methods returning a list of user objects (get_group_users, get_admins, ...) should
   be refactored to return a dict with those objects instead, having the username
   ('login') as the key.

pyproject.toml

@@ -36,7 +36,6 @@ requires = ["poetry-core>=1.0.0", "poetry-dynamic-versioning"]
 
 [tool.poetry-dynamic-versioning]
 enable = true
-pattern = "^pyppms-((?P<epoch>\\d+)!)?(?P<base>\\d+(\\.\\d+)*)"
 
 [tool.pytest.ini_options]
 addopts = "-rs -vv --cov=pyppms --cov-report html --maxfail=1"

src/pyppms/ppms.py

@@ -57,7 +57,7 @@ class PpmsConnection:
         ``auth_httpstatus``
     """
 
-    def __init__(self, url, api_key, timeout=10, cache=""):
+    def __init__(self, url, api_key, timeout=10, cache="", cache_users_only=False):
         """Constructor for the PPMS connection object.
 
         Open a connection to the PUMAPI defined in `url` and try to authenticate
@@ -83,6 +83,11 @@ def __init__(self, url, api_key, timeout=10, cache=""):
             individual text files. Useful for testing and for speeding up
             slow requests like 'getusers'. By default empty, which will result
             in no caching being done.
+        cache_users_only : bool, optional
+            If set to `True`, only `getuser` requests will be cached on disk.
+            This can be used in to speed up the slow requests (through the
+            cache), while everything else will be handled through online
+            requests. By default `False`.
 
         Raises
         ------
@@ -101,6 +106,7 @@ def __init__(self, url, api_key, timeout=10, cache=""):
             "auth_httpstatus": -1,
         }
         self.cache_path = cache
+        self.cache_users_only = cache_users_only
 
         # run in cache-only mode (e.g. for testing or off-line usage) if no API
         # key has been specified, skip authentication then:
@@ -241,6 +247,11 @@ def __interception_path(self, req_data, create_dir=False):
             request (except credentials like 'apikey').
         """
         action = req_data["action"]
+
+        if self.cache_users_only and action != "getuser":
+            LOG.debug(f"NOT caching '{action}' (cache_users_only is set)")
+            return None
+
         intercept_dir = os.path.join(self.cache_path, action)
         if create_dir and not os.path.exists(intercept_dir):  # pragma: no cover
             try:
@@ -300,7 +311,7 @@ def __init__(self, text, status_code):
             raise LookupError("No cache path configured")
 
         intercept_file = self.__interception_path(req_data, create_dir=False)
-        if not os.path.exists(intercept_file):  # pragma: no cover
+        if not intercept_file or not os.path.exists(intercept_file):  # pragma: no cover
             raise LookupError(f"No cache hit for [{intercept_file}]")
 
         with open(intercept_file, "r", encoding="utf-8") as infile:
@@ -563,7 +574,7 @@ def get_running_sheet(self, core_facility_ref, date, ignore_uncached_users=False
             similar. Note that only the date part is relevant, time will be ignored.
         ignore_uncached_users : bool, optional
             If set to `True` any booking for a user that is not present in the instance
-            attribuge `fullname_mapping` will be ignored in the resulting list.
+            attribute `fullname_mapping` will be ignored in the resulting list.
 
         Returns
         -------
@@ -743,7 +754,7 @@ def get_user(self, login_name, skip_cache=False):
 
         if not response.text:
             msg = f"User [{login_name}] is unknown to PPMS"
-            LOG.error(msg)
+            LOG.debug(msg)
             raise KeyError(msg)
 
         user = PpmsUser(response.text)
@@ -871,14 +882,17 @@ def get_user_ids(self, active=False):
         LOG.debug(", ".join(users))
         return users
 
-    def get_users(self, force_refresh=False):
+    def get_users(self, force_refresh=False, active_only=True):
         """Get user objects for all (or cached) PPMS users.
 
         Parameters
         ----------
         force_refresh : bool, optional
             Re-request information from PPMS even if user details have been
             cached locally before, by default False.
+        active_only : bool, optional
+            If set to `False` also "inactive" users will be fetched from PPMS,
+            by default `True`.
 
         Returns
         -------
@@ -888,7 +902,7 @@ def get_users(self, force_refresh=False):
         if self.users and not force_refresh:
             LOG.debug("Using cached details for %s users", len(self.users))
         else:
-            self.update_users()
+            self.update_users(active_only=active_only)
 
         return self.users
 
@@ -1185,7 +1199,7 @@ def update_systems(self):
 
         self.systems = systems
 
-    def update_users(self, user_ids=[]):
+    def update_users(self, user_ids=[], active_only=True):
         """Update cached details for a list of users from PPMS.
 
         Get the user details on a list of users (or all active ones) from PPMS and store
@@ -1199,9 +1213,12 @@ def update_users(self, user_ids=[]):
         user_ids : list(str), optional
             A list of user IDs (login names) to request the cache for, by
             default [] which will result in all *active* users to be requested.
+        active_only : bool, optional
+            If set to `False` also "inactive" users will be fetched from PPMS,
+            by default `True`.
         """
         if not user_ids:
-            user_ids = self.get_user_ids(active=True)
+            user_ids = self.get_user_ids(active=active_only)
 
         LOG.debug("Updating details on %s users", len(user_ids))
         for user_id in user_ids: