From 36e865e46f467bd21beec207b6e7f7b50045617a Mon Sep 17 00:00:00 2001 From: Andrey Nikiforov Date: Mon, 26 Oct 2020 21:59:10 -0700 Subject: [PATCH 1/5] remove multi threaded downloads --- .dockerignore | 2 ++ icloudpd/base.py | 46 +++-------------------------------- tests/test_download_photos.py | 3 +-- 3 files changed, 6 insertions(+), 45 deletions(-) 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/icloudpd/base.py b/icloudpd/base.py index d205813fb..f08b3cfcd 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"]) @@ -228,7 +222,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""" @@ -553,38 +547,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: @@ -593,19 +561,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..dc2ec2e22 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,6 @@ 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? for f in files_to_skip: with open(os.path.join(base_dir, f[0]), "a") as fi: From c72ac21898c14791569c85ee434873dc72d953a2 Mon Sep 17 00:00:00 2001 From: Andrey Nikiforov Date: Mon, 26 Oct 2020 22:01:08 -0700 Subject: [PATCH 2/5] add deprecation notice for --threads-num option --- README.md | 2 +- icloudpd/base.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8929c89ad..d6fe58983 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,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 -- depricated. 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 f08b3cfcd..3d1c68c6f 100755 --- a/icloudpd/base.py +++ b/icloudpd/base.py @@ -188,7 +188,7 @@ ) @click.option( "--threads-num", - help="Number of cpu threads (default: 1)", + help="Number of cpu threads -- depricated. To be removed in future version", type=click.IntRange(1), default=1, ) From d0e3682a6a1336e9c1a49abb99c5df7b7d6adbdc Mon Sep 17 00:00:00 2001 From: Andrey Nikiforov Date: Mon, 26 Oct 2020 22:06:53 -0700 Subject: [PATCH 3/5] update change log --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fec38feb0..33a593fdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Unreleased +- feat: removed multi-threaded downloading and added deprecation notice to --threads-num parameter #180, #188 + ## 1.6.2 (2020-10-23) - Began recording updates in `CHANGELOG.md` From bd4896ccd5399a43a72e4709457cd0143ca8d40d Mon Sep 17 00:00:00 2001 From: Andrey Nikiforov Date: Tue, 27 Oct 2020 07:26:18 -0700 Subject: [PATCH 4/5] fixed spelling mistake --- README.md | 2 +- icloudpd/base.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d6fe58983..5091eb5ad 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,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 -- depricated. To be removed in future version + --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 3d1c68c6f..56247f519 100755 --- a/icloudpd/base.py +++ b/icloudpd/base.py @@ -188,7 +188,7 @@ ) @click.option( "--threads-num", - help="Number of cpu threads -- depricated. To be removed in future version", + help="Number of cpu threads -- deprecated. To be removed in future version", type=click.IntRange(1), default=1, ) From bf3bbd73c6176cf1bcf4be3e88739ae5ef361168 Mon Sep 17 00:00:00 2001 From: Andrey Nikiforov Date: Tue, 27 Oct 2020 07:37:43 -0700 Subject: [PATCH 5/5] add test to validate that no files are downloaded after until_found --- tests/test_download_photos.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_download_photos.py b/tests/test_download_photos.py index dc2ec2e22..e3986a524 100644 --- a/tests/test_download_photos.py +++ b/tests/test_download_photos.py @@ -322,6 +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")) for f in files_to_skip: with open(os.path.join(base_dir, f[0]), "a") as fi: @@ -385,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):