From 293f7508b4d0508e787c63f2e3566b2b8590291c Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Thu, 15 Sep 2022 13:09:05 +0200 Subject: [PATCH] Download all files, skip downloaded files Fixes: https://pagure.io/koji/issue/3499 --- cli/koji_cli/commands.py | 37 ++++++++-------------------- tests/test_cli/test_download_task.py | 30 +++++++++++++--------- 2 files changed, 28 insertions(+), 39 deletions(-) diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index b3476833..ffa8e2d4 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -7001,27 +7001,11 @@ def anon_handle_download_task(options, session, args): print("No files for download found.") return - downloads_new_names = [(new_filename, vol) for (_, _, vol, new_filename, _) in downloads] - if not suboptions.dirpertask: - not_uniques = list({x for x in downloads_new_names if downloads_new_names.count(x) > 1}) - if not_uniques: - files_dict = {} - for nu in not_uniques: - if not nu[0].endswith('src.rpm'): - for (_, _, vol, new_filename, task_id) in downloads: - if new_filename == nu[0] and vol == nu[1]: - files_dict.setdefault(new_filename, {'vol': vol, 'tasks': []}) - files_dict[new_filename]['tasks'].append(task_id) - for key, value in files_dict.items(): - warn('Duplicate file %s for volume %s (tasks [%s])' % (key, value['vol'], - ", ".join(value['tasks']))) - if files_dict != {}: - error("Download files names conflict, use --dirpertask") - # perform the download number = 0 pathinfo = koji.PathInfo(topdir=suboptions.topurl) - srpm_downloaded = [] + files_downloaded = [] + dirpertask_msg = False for (task, filename, volume, new_filename, task_id) in downloads: if suboptions.dirpertask: koji.ensuredir(task_id) @@ -7036,18 +7020,17 @@ def anon_handle_download_task(options, session, args): if '..' in filename: error('Invalid file name: %s' % filename) url = '%s/%s/%s' % (pathinfo.work(volume), pathinfo.taskrelpath(task["id"]), filename) - if not new_filename.endswith('src.rpm'): + if (new_filename, volume) not in files_downloaded: download_file(url, new_filename, quiet=suboptions.quiet, noprogress=suboptions.noprogress, size=len(downloads), num=number) + files_downloaded.append((new_filename, volume)) else: - if (new_filename, volume) not in srpm_downloaded: - download_file(url, new_filename, quiet=suboptions.quiet, - noprogress=suboptions.noprogress, size=len(downloads), num=number) - srpm_downloaded.append((new_filename, volume)) - else: - if not suboptions.quiet: - print("Downloading [%d/%d] %s" % (number, len(downloads), new_filename)) - print("File %s already downloaded, skipping" % new_filename) + if not suboptions.quiet: + print("Downloading [%d/%d]: %s" % (number, len(downloads), new_filename)) + print("File %s already downloaded, skipping" % new_filename) + dirpertask_msg = True + if dirpertask_msg: + warn("Duplicate files, for download all duplicate files use --dirpertask.") def anon_handle_wait_repo(options, session, args): diff --git a/tests/test_cli/test_download_task.py b/tests/test_cli/test_download_task.py index fc1b5448..7e2be278 100644 --- a/tests/test_cli/test_download_task.py +++ b/tests/test_cli/test_download_task.py @@ -722,16 +722,9 @@ Options: # Run it and check immediate output # args: task_id --dirpertask --log # expected: failure - self.assert_system_exit( - anon_handle_download_task, - self.options, self.session, args, - stderr="Duplicate file somerpm.noarch.rpm for volume DEFAULT (tasks [22222, 55555])\n" - "Download files names conflict, use --dirpertask\n", - stdout='', - activate_session=None, - exit_code=1) - actual = self.stdout.getvalue() - expected = '' + anon_handle_download_task(self.options, self.session, args) + actual = self.stderr.getvalue() + expected = 'Duplicate files, for download all duplicate files use --dirpertask.\n' self.assertMultiLineEqual(actual, expected) # Finally, assert that things were called as we expected. self.ensure_connection.assert_called_once_with(self.session, self.options) @@ -743,7 +736,20 @@ Options: call(self.session, 33333), call(self.session, 44444), call(self.session, 55555)]) - self.assertListEqual(self.download_file.mock_calls, []) + self.assertListEqual(self.download_file.mock_calls, [ + call('https://topurl/work/tasks/2222/22222/somerpm.src.rpm', + 'somerpm.src.rpm', quiet=None, noprogress=None, size=7, num=1), + call('https://topurl/vol/vol1/work/tasks/2222/22222/somerpm.src.rpm', + 'vol1/somerpm.src.rpm', quiet=None, noprogress=None, size=7, num=2), + call('https://topurl/work/tasks/2222/22222/somerpm.noarch.rpm', + 'somerpm.noarch.rpm', quiet=None, noprogress=None, size=7, num=3), + call('https://topurl/work/tasks/3333/33333/somerpm.x86_64.rpm', + 'somerpm.x86_64.rpm', quiet=None, noprogress=None, size=7, num=4), + call('https://topurl/vol/vol2/work/tasks/3333/33333/somerpm.x86_64.rpm', + 'vol2/somerpm.x86_64.rpm', quiet=None, noprogress=None, size=7, num=5), + call('https://topurl/vol/vol2/work/tasks/4444/44444/somerpm.s390.rpm', + 'vol2/somerpm.s390.rpm', quiet=None, noprogress=None, size=7, num=6), + ]) def test_handle_download_task_without_all_json_not_downloaded(self): args = [str(self.parent_task_id)] @@ -811,7 +817,7 @@ Options: rv = anon_handle_download_task(self.options, self.session, args) actual = self.stdout.getvalue() - expected = 'Downloading [3/9] somerpm.src.rpm\n' \ + expected = 'Downloading [3/9]: somerpm.src.rpm\n' \ 'File somerpm.src.rpm already downloaded, skipping\n' self.assertMultiLineEqual(actual, expected) # Finally, assert that things were called as we expected.