diff --git a/.dockerignore b/.dockerignore index 78a23d440..b5ce50626 100644 --- a/.dockerignore +++ b/.dockerignore @@ -13,3 +13,5 @@ __pycache__ .coverage .coverage.* .git +tests/fixures +tests/__pycache__ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index c5e1c0d4d..360aa9274 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/README.md b/README.md index 8ed341f43..f374f35ec 100644 --- a/README.md +++ b/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. diff --git a/icloudpd/base.py b/icloudpd/base.py index 117fb45e7..2a16a292a 100755 --- a/icloudpd/base.py +++ b/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) diff --git a/tests/test_download_photos.py b/tests/test_download_photos.py index a920e8ecb..e3986a524 100644 --- a/tests/test_download_photos.py +++ b/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):