Merge pull request #191 from AndreyNikiforov/feature/remove-mt

remove multi-threaded downloading

Author: AndreyNikiforov

.dockerignore

@@ -13,3 +13,5 @@ __pycache__
 .coverage
 .coverage.*
 .git
+tests/fixures
+tests/__pycache__

CHANGELOG.md

@@ -5,6 +5,7 @@
 - feature: Folder structure can be set to 'none' instead of a date pattern,
 so all photos will be placed directly into the download directory.
 - fix: Empty directory structure being created #185
+- feature: removed multi-threaded downloading and added deprecation notice to --threads-num parameter #180, #188
 
 ## 1.6.2 (2020-10-23)
 

README.md

@@ -120,7 +120,7 @@ pip install icloudpd
                                         prints log messages on separate lines
                                         (Progress bar is disabled by default if
                                         there is no tty attached)
-        --threads-num INTEGER RANGE     Number of cpu threads (default: 1)
+        --threads-num INTEGER RANGE     Number of cpu threads -- deprecated. To be removed in future version
         --version                       Show the version and exit.
         -h, --help                      Show this message and exit.
 

icloudpd/base.py

@@ -9,7 +9,6 @@
 import itertools
 import subprocess
 import json
-import threading
 import click
 
 from tqdm import tqdm
@@ -29,11 +28,6 @@
 from icloudpd import constants
 from icloudpd.counter import Counter
 
-try:
-    import Queue as queue
-except ImportError:
-    import queue
-
 CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"])
 
 
@@ -195,7 +189,7 @@
               )
 @click.option(
     "--threads-num",
-    help="Number of cpu threads (default: 1)",
+    help="Number of cpu threads -- deprecated. To be removed in future version",
     type=click.IntRange(1),
     default=1,
 )
@@ -229,7 +223,7 @@ def main(
         log_level,
         no_progress_bar,
         notification_script,
-        threads_num,
+        threads_num,    # pylint: disable=W0613
 ):
     """Download all iCloud photos to a local directory"""
 
@@ -550,38 +544,12 @@ def download_photo(counter, photo):
                             icloud, photo, lp_download_path, lp_size
                         )
 
-    def get_threads_count():
-        """Disable threads if we have until_found or recent arguments"""
-        if until_found is None and recent is None:
-            return threads_num  # pragma: no cover
-        return 1
-
-    download_queue = queue.Queue(get_threads_count())
     consecutive_files_found = Counter(0)
 
     def should_break(counter):
         """Exit if until_found condition is reached"""
         return until_found is not None and counter.value() >= until_found
 
-    def worker(counter):
-        """Threaded worker"""
-        while True:
-            item = download_queue.get()
-            if item is None:
-                break
-
-            download_photo(counter, item)
-            download_queue.task_done()
-
-    threads = []
-    for _ in range(get_threads_count()):
-        thread = threading.Thread(
-            target=worker, args=(
-                consecutive_files_found, ))
-        thread.daemon = True
-        thread.start()
-        threads.append(thread)
-
     photos_iterator = iter(photos_enumerator)
     while True:
         try:
@@ -590,19 +558,11 @@ def worker(counter):
                     "Found %d consecutive previously downloaded photos. Exiting" %
                     until_found)
                 break
-            download_queue.put(next(photos_iterator))
+            item = next(photos_iterator)
+            download_photo(consecutive_files_found, item)
         except StopIteration:
             break
 
-    if not should_break(consecutive_files_found):
-        download_queue.join()
-
-    for _ in threads:
-        download_queue.put(None)
-
-    for thread in threads:
-        thread.join()
-
     if only_print_filenames:
         sys.exit(0)
 

tests/test_download_photos.py

@@ -295,7 +295,7 @@ def test_skip_existing_downloads(self):
             assert result.exit_code == 0
 
     @pytest.mark.skipif(sys.platform == 'win32',
-                    reason="does not run on windows")
+                    reason="requires large timeout on windows to create big files")
     def test_until_found(self):
         base_dir = os.path.normpath("tests/fixtures/Photos")
         if os.path.exists(base_dir):
@@ -322,7 +322,7 @@ def test_until_found(self):
         files_to_skip.append(("2018/07/30/IMG_7400.JPG", "photo", 2308885))
         files_to_skip.append(("2018/07/30/IMG_7400-medium.MOV", "photo", 1238639))
         files_to_skip.append(("2018/07/30/IMG_7399.JPG", "photo", 2251047))
-        files_to_download.append(("2018/07/30/IMG_7399-medium.MOV", "photo"))   # TODO this should not be downloaded, correct?
+        files_to_download.append(("2018/07/30/IMG_7399-medium.MOV", "photo"))
 
         for f in files_to_skip:
             with open(os.path.join(base_dir, f[0]), "a") as fi:
@@ -386,6 +386,11 @@ def test_until_found(self):
                         "INFO     Found 3 consecutive previously downloaded photos. Exiting",
                         self._caplog.text,
                     )
+                    self.assertNotIn(
+                        f"INFO     {os.path.join(base_dir, os.path.normpath('2018/07/30/IMG_7399-medium.MOV'))} already exists.", 
+                        self._caplog.text
+                    )
+
                     assert result.exit_code == 0
 
     def test_handle_io_error(self):