Simplifies the Cleaner and adds docstrings

We were passing some unnecessary parameters to the Cleaner (locator) that could be derived from other data (uuid and appointments). Also standarises the order of the  parameters to match the rest of the methods

pisa/cleaner.py

@@ -2,13 +2,27 @@ from pisa.logger import Logger
 
 logger = Logger("Cleaner")
 
-# Dictionaries in Python are "passed-by-reference", so no return is needed for the Cleaner"
-# https://docs.python.org/3/faq/programming.html#how-do-i-write-a-function-with-output-parameters-call-by-reference
-
 
 class Cleaner:
+    """
+    The ``Cleaner`` is the class in charge of removing expired / completed data from the tower.
+
+    Mutable objects (like dicts) are passed-by-reference in Python, so no return is needed for the Cleaner.
+    """
+
     @staticmethod
     def delete_expired_appointment(expired_appointments, appointments, locator_uuid_map, db_manager):
+        """
+        Deletes appointments which ``end_time`` has been reached (with no trigger) both from memory
+        (:mod:`Watcher <pisa.watcher>`) and disk.
+
+        Args:
+            expired_appointments (list): a list of appointments to be deleted.
+            appointments (dict): a dictionary containing all the :mod:`Watcher <pisa.watcher>` appointments.
+            locator_uuid_map (dict): a ``locator:uuid`` map for the :mod:`Watcher <pisa.watcher>` appointments.
+            db_manager (DBManager): a :mod:`DBManager <pisa.db_manager>` instance to interact with the database.
+        """
+
         for uuid in expired_appointments:
             locator = appointments[uuid].locator
 
@@ -26,16 +40,26 @@ class Cleaner:
             db_manager.delete_watcher_appointment(uuid)
 
     @staticmethod
-    def delete_completed_appointment(locator, uuid, appointments, locator_uuid_map, db_manager):
+    def delete_completed_appointment(uuid, appointments, locator_uuid_map, db_manager):
+        """
+        Deletes a triggered appointment from memory (:mod:`Watcher <pisa.watcher>`) and flags it as triggered in disk.
+
+        Args:
+            uuid (str): a unique 16-byte hex-encoded str that identifies the appointment.
+            appointments (dict): a dictionary containing all the :mod:`Watcher <pisa.watcher>` appointments.
+            locator_uuid_map (dict): a ``locator:uuid`` map for the :mod:`Watcher <pisa.watcher>` appointments.
+            db_manager (DBManager): a :mod:`DBManager <pisa.db_manager>` instance to interact with the database.
+        """
+
         # Delete the appointment
         appointment = appointments.pop(uuid)
 
         # If there was only one appointment that matches the locator we can delete the whole list
-        if len(locator_uuid_map[locator]) == 1:
+        if len(locator_uuid_map[appointment.locator]) == 1:
-            locator_uuid_map.pop(locator)
+            locator_uuid_map.pop(appointment.locator)
         else:
             # Otherwise we just delete the appointment that matches locator:appointment_pos
-            locator_uuid_map[locator].remove(uuid)
+            locator_uuid_map[appointment.locator].remove(uuid)
 
         # DISCUSS: instead of deleting the appointment, we will mark it as triggered and delete it from both
         #          the watcher's and responder's db after fulfilled
@@ -43,7 +67,19 @@ class Cleaner:
         db_manager.store_watcher_appointment(uuid, appointment.to_json(triggered=True))
 
     @staticmethod
-    def delete_completed_jobs(jobs, tx_job_map, completed_jobs, height, db_manager):
+    def delete_completed_jobs(completed_jobs, height, jobs, tx_job_map, db_manager):
+        """
+        Deletes a completed job both from memory (:mod:`Responder <pisa.responder>`) and disk (from the
+        :mod:`Responder <pisa.responder>` and :mod:`Watcher <pisa.watcher>` databases).
+
+        Args:
+            jobs (dict): a dictionary containing all the :mod:`Responder <pisa.responder>` jobs.
+            tx_job_map (dict): a ``penalty_txid:uuid`` map for the :mod:`Responder <pisa.responder>` jobs.
+            completed_jobs (list): a list of completed jobs to be deleted.
+            height (int): the block height at which the jobs were completed.
+            db_manager (DBManager): a :mod:`DBManager <pisa.db_manager>` instance to interact with the database.
+        """
+
         for uuid, confirmations in completed_jobs:
             logger.info(
                 "Job completed. Appointment ended after reaching enough confirmations.",

pisa/responder.py

@@ -156,7 +156,7 @@ class Responder:
                     txs_to_rebroadcast = self.get_txs_to_rebroadcast(txs)
                     completed_jobs = self.get_completed_jobs(height)
 
-                    Cleaner.delete_completed_jobs(self.jobs, self.tx_job_map, completed_jobs, height, self.db_manager)
+                    Cleaner.delete_completed_jobs(completed_jobs, height, self.jobs, self.tx_job_map, self.db_manager)
                     self.rebroadcast(txs_to_rebroadcast, block_hash)
 
                 # NOTCOVERED

pisa/watcher.py

@@ -138,7 +138,7 @@ class Watcher:
 
                     # Delete the appointment and update db
                     Cleaner.delete_completed_appointment(
-                        filtered_match["locator"], uuid, self.appointments, self.locator_uuid_map, self.db_manager
+                        uuid, self.appointments, self.locator_uuid_map, self.db_manager
                     )
 
                 # Register the last processed block for the watcher

test/unit/test_cleaner.py

@@ -96,9 +96,7 @@ def test_delete_completed_appointments(db_manager):
     uuids = list(appointments.keys())
 
     for uuid in uuids:
-        Cleaner.delete_completed_appointment(
+        Cleaner.delete_completed_appointment(uuid, appointments, locator_uuid_map, db_manager)
-            appointments[uuid].locator, uuid, appointments, locator_uuid_map, db_manager
-        )
 
     # All appointments should have been deleted
     assert len(appointments) == 0
@@ -118,7 +116,7 @@ def test_delete_completed_jobs_db_match(db_manager):
 
         completed_jobs = [(job, 6) for job in selected_jobs]
 
-        Cleaner.delete_completed_jobs(jobs, tx_job_map, completed_jobs, height, db_manager)
+        Cleaner.delete_completed_jobs(completed_jobs, height, jobs, tx_job_map, db_manager)
 
         assert not set(completed_jobs).issubset(jobs.keys())
 
@@ -156,5 +154,5 @@ def test_delete_completed_jobs_no_db_match(db_manager):
         completed_jobs = [(job, 6) for job in selected_jobs]
 
         # We should be able to delete the correct ones and not fail in the others
-        Cleaner.delete_completed_jobs(jobs, tx_job_map, completed_jobs, height, db_manager)
+        Cleaner.delete_completed_jobs(completed_jobs, height, jobs, tx_job_map, db_manager)
         assert not set(completed_jobs).issubset(jobs.keys())