Merge branch 'devel'

Author: ehrenfeu

CHANGELOG.md

@@ -4,6 +4,25 @@
 
 NOTE: potentially breaking changes are flagged with a 🧨 symbol.
 
+## 3.1.0
+
+### Added
+
+- `pyppms.common.fmt_time()` to string-format a datetime object that might also
+  be None (in which case a fixed string is returned).
+- `pyppms.booking.PpmsBooking.desc` has been added as a property to retrieve a
+  shorter description of the object than calling `str()` on it.
+- `pyppms.exceptions.NoDataError` has been added to indicate a PUMAPI response
+  did *not* contain any useful data.
+- `pyppms.common.parse_multiline_response()` will now raise the newly added
+  `NoDataError` in case the requested *runningsheet* for a day doesn't contain
+  any bookings to allow for properly dealing with "empty" days.
+
+### Changed
+
+- Several log messages have been demoted from `debug` to `trace` level and might
+  have been shortened / combined to reduce logging clutter.
+
 ## 3.0.0
 
 ### Changed

src/pyppms/booking.py

@@ -4,7 +4,7 @@
 
 from loguru import logger as log
 
-from .common import time_rel_to_abs
+from .common import time_rel_to_abs, fmt_time
 
 
 class PpmsBooking:
@@ -122,7 +122,7 @@ def starttime_fromstr(self, time_str, date=None):
             microsecond=0,
         )
         self.starttime = start
-        log.debug("New starttime: {}", self)
+        log.trace("New starttime: {}", self)
 
     def endtime_fromstr(self, time_str, date=None):
         """Change the ending time and / or day of a booking.
@@ -144,16 +144,9 @@ def endtime_fromstr(self, time_str, date=None):
             microsecond=0,
         )
         self.endtime = end
-        log.debug("New endtime: {}", self)
+        log.trace("New endtime: {}", self)
 
     def __str__(self):
-        def fmt_time(time):
-            # in case a booking was created from a "nextbooking" response it will not
-            # have the `endtime` attribute set, so treat this separately:
-            if time is None:
-                return "===UNDEFINED==="
-            return datetime.strftime(time, "%Y-%m-%d %H:%M")
-
         msg = (
             f"PpmsBooking(username=[{self.username}], "
             f"system_id=[{self.system_id}], "
@@ -165,3 +158,17 @@ def fmt_time(time):
         msg += ")"
 
         return msg
+
+    @property
+    def desc(self):
+        """Format a "short" description of the object.
+
+        Returns
+        -------
+        str
+            A string containing `username`, `system_id` and the booking times.
+        """
+        return (
+            f"{self.username}@{self.system_id} "
+            f"[{fmt_time(self.starttime)} -- {fmt_time(self.endtime)}]"
+        )

src/pyppms/common.py

@@ -8,6 +8,8 @@
 
 from loguru import logger as log
 
+from .exceptions import NoDataError
+
 
 def process_response_values(values):
     """Process (in-place) a list of strings, remove quotes, detect boolean etc.
@@ -120,11 +122,15 @@ def parse_multiline_response(text, graceful=True):
     -------
     list(dict)
         A list with dicts where the latter ones have the same form as produced
-        by the dict_from_single_response() function. Note that when graceful
+        by the dict_from_single_response() function. May be empty in case the
+        PUMAPI response didn't contain any useful data. Note that when graceful
         mode is requested, consistency among the dicts is not guaranteed.
 
     Raises
     ------
+    NoDataError
+        Raised when the response text was too short (less than two lines) and
+        the `graceful` parameter has been set to false.
     ValueError
         Raised when the response text is inconsistent and the `graceful`
         parameter has been set to false, or if parsing fails for any other
@@ -134,10 +140,10 @@ def parse_multiline_response(text, graceful=True):
     try:
         lines = text.splitlines()
         if len(lines) < 2:
-            log.warning("Response expected to have two or more lines: {}", text)
+            log.info("Response has less than TWO lines: >>>{}<<<", text)
             if not graceful:
-                raise ValueError("Invalid response format!")
-            return parsed
+                raise NoDataError("Invalid response format!")
+            return []
 
         header = lines[0].split(",")
         for i, entry in enumerate(header):
@@ -174,6 +180,9 @@ def parse_multiline_response(text, graceful=True):
             )
             log.warning(msg)
 
+    except NoDataError as err:
+        raise err
+
     except Exception as err:
         msg = f"Unable to parse data returned by PUMAPI: {text} - ERROR: {err}"
         log.error(msg)
@@ -198,3 +207,23 @@ def time_rel_to_abs(minutes_from_now):
     now = datetime.now().replace(second=0, microsecond=0)
     abstime = now + timedelta(minutes=int(minutes_from_now))
     return abstime
+
+
+def fmt_time(time):
+    """Format a `datetime` or `None` object to string.
+
+    This is useful to apply it to booking times as they might be `None` e.g. in
+    case they have been created from a "nextbooking" response.
+
+    Parameters
+    ----------
+    time : datetime.datetime or None
+
+    Returns
+    -------
+    str
+        The formatted time, or a specific string in case the input was `None`.
+    """
+    if time is None:
+        return "===UNDEFINED==="
+    return datetime.strftime(time, "%Y-%m-%d %H:%M")

src/pyppms/exceptions.py

@@ -0,0 +1,5 @@
+"""PyPPMS exception classes."""
+
+
+class NoDataError(ValueError):
+    """Exception indicating no data was received from PUMAPI."""

src/pyppms/ppms.py

@@ -20,6 +20,7 @@
 from .user import PpmsUser
 from .system import PpmsSystem
 from .booking import PpmsBooking
+from .exceptions import NoDataError
 
 
 class PpmsConnection:
@@ -133,7 +134,7 @@ def __authenticate(self):
         )
         self.status["auth_state"] = "attempting"
         response = self.request("auth")
-        log.debug(f"Authenticate response: {response.text}")
+        log.trace(f"Authenticate response: {response.text}")
         self.status["auth_response"] = response.text
         self.status["auth_httpstatus"] = response.status_code
 
@@ -166,8 +167,11 @@ def __authenticate(self):
             log.error(msg)
             raise requests.exceptions.ConnectionError(msg)
 
-        log.info(f"Authentication succeeded, response=[{response.text}]")
-        log.debug(f"HTTP Status: {response.status_code}")
+        log.info(
+            "Authentication succeeded, response=[{}], http_status=[{}]",
+            response.text,
+            response.status_code,
+        )
         self.status["auth_state"] = "good"
 
     def request(self, action, parameters={}, skip_cache=False):
@@ -249,14 +253,14 @@ def __interception_path(self, req_data, create_dir=False):
         action = req_data["action"]
 
         if self.cache_users_only and action != "getuser":
-            log.debug(f"NOT caching '{action}' (cache_users_only is set)")
+            log.trace(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:
                 os.makedirs(intercept_dir)
-                log.debug(f"Created dir to store response: {intercept_dir}")
+                log.trace(f"Created dir to store response: {intercept_dir}")
             except Exception as err:  # pylint: disable-msg=broad-except
                 log.warning(f"Failed creating [{intercept_dir}]: {err}")
                 return None
@@ -316,7 +320,10 @@ def __init__(self, text, status_code):
 
         with open(intercept_file, "r", encoding="utf-8") as infile:
             text = infile.read()
-        log.debug(f"Read intercepted response text from [{intercept_file}]")
+        log.debug(
+            "Read intercepted response text from [{}]",
+            intercept_file[len(str(self.cache_path)) :],
+        )
 
         status_code = 200
         status_file = os.path.splitext(intercept_file)[0] + "_status-code.txt"
@@ -590,12 +597,14 @@ def get_running_sheet(self, core_facility_ref, date, ignore_uncached_users=False
         response = self.request("getrunningsheet", parameters)
         try:
             entries = parse_multiline_response(response.text, graceful=False)
+        except NoDataError:
+            # in case no bookings exist the response will be empty!
+            log.debug("Runningsheet for the given day was empty!")
+            return []
         except Exception as err:  # pylint: disable-msg=broad-except
             log.error("Parsing runningsheet details failed: {}", err)
-            # NOTE: in case no future bookings exist the response will be empty!
-            log.error("Possibly the runningsheet is empty as no bookings exist?")
-            log.debug("Runningsheet PUMPAI response was: {}", response.text)
-            return bookings
+            log.debug("Runningsheet PUMPAI response was: >>>{}<<<", response.text)
+            return []
 
         for entry in entries:
             full = entry["User"]
@@ -702,7 +711,7 @@ def get_systems_matching(self, localisation, name_contains):
         systems = self.get_systems()
         for sys_id, system in systems.items():
             if loc.lower() not in str(system.localisation).lower():
-                log.debug(
+                log.trace(
                     "System [{}] location ({}) is NOT matching ({}), ignoring",
                     system.name,
                     system.localisation,
@@ -715,7 +724,7 @@ def get_systems_matching(self, localisation, name_contains):
             #           system.name, loc, name_contains)
             for valid_name in name_contains:
                 if valid_name in system.name:
-                    log.debug("System [{}] matches all criteria", system.name)
+                    log.trace("System [{}] matches all criteria", system.name)
                     system_ids.append(sys_id)
                     break