Since this thing is on the innerwebs I suppose it should have a

[python_utils.git] / lockfile.py

diff --git a/lockfile.py b/lockfile.py
index 2d429147937d01b37a057910c50004374917af0f..ae48e576ccbcfb123ddfcde6ab32c71dfc381ad0 100644 (file)
--- a/lockfile.py
+++ b/lockfile.py
@@ -1,5 +1,11 @@
 #!/usr/bin/env python3
 
+# © Copyright 2021-2022, Scott Gasch
+
+"""File-based locking helper."""
+
+from __future__ import annotations
+import contextlib
 import datetime
 import json
 import logging
@@ -8,7 +14,7 @@ import signal
 import sys
 import warnings
 from dataclasses import dataclass
-from typing import Optional
+from typing import Literal, Optional
 
 import config
 import datetime_utils
@@ -18,7 +24,7 @@ cfg = config.add_commandline_args(f'Lockfile ({__file__})', 'Args related to loc
 cfg.add_argument(
     '--lockfile_held_duration_warning_threshold_sec',
     type=float,
-    default=10.0,
+    default=60.0,
     metavar='SECONDS',
     help='If a lock is held for longer than this threshold we log a warning',
 )
@@ -26,17 +32,21 @@ logger = logging.getLogger(__name__)
 
 
 class LockFileException(Exception):
+    """An exception related to lock files."""
+
     pass
 
 
 @dataclass
 class LockFileContents:
+    """The contents we'll write to each lock file."""
+
     pid: int
     commandline: str
     expiration_timestamp: Optional[float]
 
 
-class LockFile(object):
+class LockFile(contextlib.AbstractContextManager):
     """A file locking mechanism that has context-manager support so you
     can use it in a with statement.  e.g.
 
@@ -56,9 +66,10 @@ class LockFile(object):
         expiration_timestamp: Optional[float] = None,
         override_command: Optional[str] = None,
     ) -> None:
-        self.is_locked = False
-        self.lockfile = lockfile_path
-        self.override_command = override_command
+        self.is_locked: bool = False
+        self.lockfile: str = lockfile_path
+        self.locktime: Optional[int] = None
+        self.override_command: Optional[str] = override_command
         if do_signal_cleanup:
             signal.signal(signal.SIGINT, self._signal)
             signal.signal(signal.SIGTERM, self._signal)
@@ -71,7 +82,7 @@ class LockFile(object):
         return not os.path.exists(self.lockfile)
 
     def try_acquire_lock_once(self) -> bool:
-        logger.debug(f"Trying to acquire {self.lockfile}.")
+        logger.debug("Trying to acquire %s.", self.lockfile)
         try:
             # Attempt to create the lockfile.  These flags cause
             # os.open to raise an OSError if the file already
@@ -81,13 +92,12 @@ class LockFile(object):
                 contents = self._get_lockfile_contents()
                 logger.debug(contents)
                 f.write(contents)
-            logger.debug(f'Success; I own {self.lockfile}.')
+            logger.debug('Success; I own %s.', self.lockfile)
             self.is_locked = True
             return True
         except OSError:
             pass
-        msg = f'Could not acquire {self.lockfile}.'
-        logger.warning(msg)
+        logger.warning('Couldn\'t acquire %s.', self.lockfile)
         return False
 
     def acquire_with_retries(
@@ -125,16 +135,18 @@ class LockFile(object):
         logger.warning(msg)
         raise LockFileException(msg)
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, _, value, traceback) -> Literal[False]:
         if self.locktime:
             ts = datetime.datetime.now().timestamp()
             duration = ts - self.locktime
             if duration >= config.config['lockfile_held_duration_warning_threshold_sec']:
-                str_duration = datetime_utils.describe_duration_briefly(duration)
+                # Note: describe duration briefly only does 1s granularity...
+                str_duration = datetime_utils.describe_duration_briefly(int(duration))
                 msg = f'Held {self.lockfile} for {str_duration}'
                 logger.warning(msg)
                 warnings.warn(msg, stacklevel=2)
         self.release()
+        return False
 
     def __del__(self):
         if self.is_locked:
@@ -164,22 +176,27 @@ class LockFile(object):
                     line = lines[0]
                     line_dict = json.loads(line)
                     contents = LockFileContents(**line_dict)
-                    logger.debug(f'Blocking lock contents="{contents}"')
+                    logger.debug('Blocking lock contents="%s"', contents)
 
                     # Does the PID exist still?
                     try:
                         os.kill(contents.pid, 0)
                     except OSError:
-                        msg = f'Lockfile {self.lockfile}\'s pid ({contents.pid}) is stale; force acquiring'
-                        logger.warning(msg)
+                        logger.warning(
+                            'Lockfile %s\'s pid (%d) is stale; force acquiring...',
+                            self.lockfile,
+                            contents.pid,
+                        )
                         self.release()
 
                     # Has the lock expiration expired?
                     if contents.expiration_timestamp is not None:
                         now = datetime.datetime.now().timestamp()
                         if now > contents.expiration_timestamp:
-                            msg = f'Lockfile {self.lockfile} expiration time has passed; force acquiring'
-                            logger.warning(msg)
+                            logger.warning(
+                                'Lockfile %s\'s expiration time has passed; force acquiring',
+                                self.lockfile,
+                            )
                             self.release()
         except Exception:
-            pass
+            pass  # If the lockfile doesn't exist or disappears, good.