Prevent localmove.py from excess logging (#471)

Co-authored-by: Daniel Garcia Briseno <daniel.garciabriseno@nasa.gov>

Author: dgarciabriseno

install/__test__/test_localmove_retry.py

@@ -0,0 +1,57 @@
+"""Tests for LocalFileMove retry limit functionality"""
+import unittest
+from unittest.mock import patch, MagicMock
+from queue import Queue
+
+from helioviewer.hvpull.downloader.localmove import LocalFileMove, MAX_RETRY_ATTEMPTS
+
+
+class TestLocalFileMoveRetry(unittest.TestCase):
+    """Test that file move retries are limited to MAX_RETRY_ATTEMPTS"""
+
+    def setUp(self):
+        self.queue = Queue()
+        self.mover = LocalFileMove("/tmp/incoming", self.queue)
+
+    @patch('shutil.move')
+    @patch('os.path.exists', return_value=True)
+    def test_successful_move_does_not_retry(self, mock_exists, mock_move):
+        """A successful move should not add anything to the queue"""
+        self.mover.process(["server1", 100, "/source/file.jp2"])
+
+        mock_move.assert_called_once()
+        self.assertTrue(self.queue.empty())
+
+    @patch('shutil.move', side_effect=IOError("File busy"))
+    @patch('os.path.exists', return_value=True)
+    def test_failed_move_retries_up_to_max(self, mock_exists, mock_move):
+        """A failing move should retry MAX_RETRY_ATTEMPTS times then give up"""
+        # First attempt (retry_count=0)
+        self.mover.process(["server1", 100, "/source/file.jp2"])
+
+        # Should be re-queued with retry_count=1
+        self.assertEqual(self.queue.qsize(), 1)
+        item = self.queue.get()
+        self.assertEqual(item[3], 1)  # retry_count = 1
+
+        # Second attempt (retry_count=1)
+        self.mover.process(item)
+        item = self.queue.get()
+        self.assertEqual(item[3], 2)  # retry_count = 2
+
+        # Third attempt (retry_count=2)
+        self.mover.process(item)
+        item = self.queue.get()
+        self.assertEqual(item[3], 3)  # retry_count = 3
+
+        # Fourth attempt (retry_count=3) - should give up, not re-queue
+        self.mover.process(item)
+        self.assertTrue(self.queue.empty(), "Should not retry after MAX_RETRY_ATTEMPTS")
+
+    def test_max_retry_attempts_is_three(self):
+        """Verify the constant is set to 3"""
+        self.assertEqual(MAX_RETRY_ATTEMPTS, 3)
+
+
+if __name__ == '__main__':
+    unittest.main()

install/helioviewer/hvpull/downloader/localmove.py

@@ -6,6 +6,9 @@
 import shutil
 from .downloader_interface import Downloader
 
+# Maximum number of retry attempts per file before giving up
+MAX_RETRY_ATTEMPTS = 3
+
 class LocalFileMove(Downloader):
     def __init__(self, incoming, queue):
         """Creates a new LocalFileMover"""
@@ -16,9 +19,17 @@ def __init__(self, incoming, queue):
         # self.queue
 
     def process(self, item):
-        """Downloads the file at the specified URI"""
-        #ValueError: too many values to unpack
-        server, percent, uri = item
+        """Downloads the file at the specified URI
+
+        Item format: [server, percent, uri] or [server, percent, uri, retry_count]
+        The retry_count is optional and defaults to 0 for backward compatibility.
+        """
+        # Support both old format (3 elements) and new format (4 elements with retry count)
+        if len(item) == 3:
+            server, percent, uri = item
+            retry_count = 0
+        else:
+            server, percent, uri, retry_count = item
 
         # @TODO: compute path to download file to...
 
@@ -38,8 +49,14 @@ def process(self, item):
             t2 = time.time()
             logging.info("(%s) locally moved %s to %s", server, uri, filepath)
         except IOError:
-            # If download fails, add back into queue and try again later
-            logging.warning("Failed to move %s. Adding to end of queue to retry later.", uri)
-            self.queue.put([server, percent, uri])
+            # If move fails, check retry count before re-queuing
+            if retry_count < MAX_RETRY_ATTEMPTS:
+                retry_count += 1
+                logging.warning("Failed to move %s. Adding to end of queue to retry later (attempt %d/%d).",
+                               uri, retry_count, MAX_RETRY_ATTEMPTS)
+                self.queue.put([server, percent, uri, retry_count])
+            else:
+                logging.error("Failed to move %s after %d attempts. Giving up on this file.",
+                             uri, MAX_RETRY_ATTEMPTS)
         except:
             logging.warning("Failed to move %s.", uri)