Easier and more self documenting patterns for loading/saving Persistent

[python_utils.git] / arper.py

diff --git a/arper.py b/arper.py
index b4c079edcc750307356b60fdba39ca5b43f5c38a..ffe4b7431746d6178badd3ec099ce9796663a1a8 100644 (file)
--- a/arper.py
+++ b/arper.py
@@ -73,14 +73,23 @@ class Arper(persistent.Persistent):
         cached_local_state: Optional[BiDict] = None,
         cached_supplimental_state: Optional[BiDict] = None,
     ) -> None:
+        """For most purposes, ignore the arguments.  Because this is a
+        Persistent subclass the decorator will handle invoking our load
+        and save methods to read/write persistent state transparently.
+
+        Args:
+            cached_local_state: local state to initialize mapping
+            cached_supplimental_state: remote state to initialize mapping
+        """
+
         self.state = BiDict()
         if cached_local_state is not None:
             logger.debug('Loading Arper map from cached local state.')
             self.state = cached_local_state
         else:
             logger.debug('No usable cached state; calling /usr/sbin/arp')
-            self.update_from_arp_scan()
-            self.update_from_arp()
+            self._update_from_arp_scan()
+            self._update_from_arp()
         if len(self.state) < config.config['arper_min_entries_to_be_valid']:
             raise Exception(f'Arper didn\'t find enough entries; only got {len(self.state)}.')
         if cached_supplimental_state is not None:
@@ -90,7 +99,9 @@ class Arper(persistent.Persistent):
         for mac, ip in self.state.items():
             logger.debug('%s <-> %s', mac, ip)
 
-    def update_from_arp_scan(self):
+    def _update_from_arp_scan(self):
+        """Internal method to initialize our state via a call to arp-scan."""
+
         network_spec = site_config.get_config().network
         try:
             output = exec_utils.cmd(
@@ -108,7 +119,9 @@ class Arper(persistent.Persistent):
                 logger.debug('ARPER: %s => %s', mac, ip)
                 self.state[mac] = ip
 
-    def update_from_arp(self):
+    def _update_from_arp(self):
+        """Internal method to initialize our state via a call to arp."""
+
         try:
             output = exec_utils.cmd('/usr/sbin/arp -a', timeout_seconds=10.0)
         except Exception as e:
@@ -123,19 +136,47 @@ class Arper(persistent.Persistent):
                 self.state[mac] = ip
 
     def get_ip_by_mac(self, mac: str) -> Optional[str]:
-        mac = mac.lower()
-        return self.state.get(mac, None)
+        """Given a MAC address, see if we know it's IP address and, if so,
+        return it.  If not, return None.
+
+        Args:
+            mac: the MAC address to lookup.  Should be formatted like
+                 ab:cd:ef:g1:23:45.
+
+        Returns:
+            The IPv4 address associated with that MAC address (as a string)
+            or None if it's not known.
+        """
+        m = string_utils.extract_mac_address(mac)
+        if not m:
+            return None
+        m = m.lower()
+        if not string_utils.is_mac_address(m):
+            return None
+        return self.state.get(m, None)
 
     def get_mac_by_ip(self, ip: str) -> Optional[str]:
+        """Given an IPv4 address (as a string), check to see if we know what
+        MAC address is associated with it and, if so, return it.  If not,
+        return None.
+
+        Args:
+            ip: the IPv4 address to look up.
+
+        Returns:
+            The associated MAC address, if known.  Or None if not.
+        """
         return self.state.inverse.get(ip, None)
 
     @classmethod
-    def load_state(
+    def _load_state(
         cls,
         cache_file: str,
         freshness_threshold_sec: int,
         state: BiDict,
     ):
+        """Internal helper method behind load."""
+
         if not file_utils.file_is_readable(cache_file):
             logger.debug('Can\'t read %s', cache_file)
             return
@@ -162,11 +203,13 @@ class Arper(persistent.Persistent):
     @classmethod
     @overrides
     def load(cls) -> Any:
+        """Internal helper method to fulfull Persistent requirements."""
+
         local_state: BiDict = BiDict()
         cache_file = config.config['arper_cache_location']
         max_staleness = config.config['arper_cache_max_staleness'].total_seconds()
         logger.debug('Trying to load main arper cache from %s...', cache_file)
-        cls.load_state(cache_file, max_staleness, local_state)
+        cls._load_state(cache_file, max_staleness, local_state)
         if len(local_state) <= config.config['arper_min_entries_to_be_valid']:
             msg = f'{cache_file} is invalid: only {len(local_state)} entries.  Deleting it.'
             logger.warning(msg)
@@ -180,13 +223,15 @@ class Arper(persistent.Persistent):
         cache_file = config.config['arper_supplimental_cache_location']
         max_staleness = config.config['arper_cache_max_staleness'].total_seconds()
         logger.debug('Trying to suppliment arper state from %s', cache_file)
-        cls.load_state(cache_file, max_staleness, supplimental_state)
+        cls._load_state(cache_file, max_staleness, supplimental_state)
         if len(local_state) > 0:
             return cls(local_state, supplimental_state)
         return None
 
     @overrides
     def save(self) -> bool:
+        """Internal helper method to fulfull Persistent requirements."""
+
         if len(self.state) > config.config['arper_min_entries_to_be_valid']:
             logger.debug('Persisting state to %s', config.config["arper_cache_location"])
             with file_utils.FileWriter(config.config['arper_cache_location']) as wf: