diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 6506d1f6..b59c37d6 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -6878,7 +6878,9 @@ def anon_handle_download_logs(options, session, args): def anon_handle_download_task(options, session, args): "[download] Download the output of a build task" - usage = "usage: %prog download-task " + usage = "usage: %prog download-task \n" \ + "Default behavior without --all option downloads .rpm files only for build " \ + "and buildArch tasks.\n" parser = OptionParser(usage=get_usage_str(usage)) parser.add_option("--arch", dest="arches", metavar="ARCH", action="append", default=[], help="Only download packages for this arch (may be used multiple times), " @@ -6929,67 +6931,69 @@ def anon_handle_download_task(options, session, args): if not suboptions.parentonly: list_tasks.extend(session.getTaskChildren(base_task_id)) - # support file types - expected_types = ['rpm', 'log'] - for type in session.getArchiveTypes(): - expected_types.extend(type['extensions'].split(' ')) + required_tasks = {} + for task in list_tasks: + if task["id"] not in required_tasks: + required_tasks[task["id"]] = task + + for task_id in required_tasks: + if required_tasks[task_id]["state"] != koji.TASK_STATES.get("CLOSED"): + if task_id == base_task_id: + error("Task %d has not finished yet." % task_id) + else: + error("Child task %d has not finished yet." % task_id) # get files for download downloads = [] build_methods_list = ['buildArch', 'build'] + rpm_file_types = ['rpm', 'src.rpm'] for task in list_tasks: taskarch = task['arch'] task_id = str(task['id']) if len(suboptions.arches) == 0 or taskarch in suboptions.arches: files = list_task_output_all_volumes(session, task["id"]) - filetype = None for filename in files: if filename.endswith('src.rpm'): filetype = 'src.rpm' else: - for ft in expected_types: - if filename.endswith('.%s' % ft): - filetype = ft - break - if not filetype: - warn('Unsupported file type for download-task: %s' % filename) - else: - if suboptions.all and task['method'] not in build_methods_list: - if filetype != 'log': - for volume in files[filename]: - if suboptions.dirpertask: - new_filename = '%s/%s' % (task_id, filename) - else: - if taskarch not in filename and filetype != 'src.rpm': - part_filename = filename[:-len('.%s' % filetype)] - new_filename = "%s.%s.%s" % (part_filename, - taskarch, filetype) - else: - new_filename = filename - downloads.append((task, filename, volume, new_filename, task_id)) - elif task['method'] in build_methods_list: - if filetype in ['rpm', 'src.rpm']: - filearch = filename.split(".")[-2] - for volume in files[filename]: - if len(suboptions.arches) == 0 or filearch in suboptions.arches: - if suboptions.dirpertask: - new_filename = '%s/%s' % (task_id, filename) - else: - new_filename = filename - downloads.append((task, filename, volume, new_filename, - task_id)) - - if filetype == 'log' and suboptions.logs: + filetype = filename.rsplit('.', 1)[1] + if suboptions.all and not (task['method'] in build_methods_list and + filetype in rpm_file_types): + if filetype != 'log': for volume in files[filename]: if suboptions.dirpertask: new_filename = '%s/%s' % (task_id, filename) else: - if taskarch not in filename: - part_filename = filename[:-len('.log')] - new_filename = "%s.%s.log" % (part_filename, taskarch) + if taskarch not in filename and filetype != 'src.rpm': + part_filename = filename[:-len('.%s' % filetype)] + new_filename = "%s.%s.%s" % (part_filename, + taskarch, filetype) else: new_filename = filename downloads.append((task, filename, volume, new_filename, task_id)) + elif task['method'] in build_methods_list: + if filetype in rpm_file_types: + filearch = filename.split(".")[-2] + for volume in files[filename]: + if len(suboptions.arches) == 0 or filearch in suboptions.arches: + if suboptions.dirpertask: + new_filename = '%s/%s' % (task_id, filename) + else: + new_filename = filename + downloads.append((task, filename, volume, new_filename, + task_id)) + + if filetype == 'log' and suboptions.logs: + for volume in files[filename]: + if suboptions.dirpertask: + new_filename = '%s/%s' % (task_id, filename) + else: + if taskarch not in filename: + part_filename = filename[:-len('.log')] + new_filename = "%s.%s.log" % (part_filename, taskarch) + else: + new_filename = filename + downloads.append((task, filename, volume, new_filename, task_id)) if len(downloads) == 0: print("No files for download found.") diff --git a/tests/test_cli/test_download_task.py b/tests/test_cli/test_download_task.py index 6d0ab3bf..c35d4a6a 100644 --- a/tests/test_cli/test_download_task.py +++ b/tests/test_cli/test_download_task.py @@ -57,6 +57,8 @@ class TestDownloadTask(utils.CliTestCase): self.parent_task_info = {'id': self.parent_task_id, 'method': 'buildArch', 'arch': 'taskarch', 'state': 2, 'parent': None} self.error_format = """Usage: %s download-task +Default behavior without --all option downloads .rpm files only for build and buildArch tasks. + (Specify the --help global option for a list of other help options) %s: error: {message} @@ -257,8 +259,7 @@ class TestDownloadTask(utils.CliTestCase): self.ensure_connection.assert_called_once_with(self.session, self.options) self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) - self.list_task_output_all_volumes.assert_called_once_with( - self.session, self.parent_task_id) + self.list_task_output_all_volumes.assert_not_called() self.download_file.assert_not_called() def test_handle_download_child_not_finished(self): @@ -285,8 +286,7 @@ class TestDownloadTask(utils.CliTestCase): self.ensure_connection.assert_called_once_with(self.session, self.options) self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) - self.list_task_output_all_volumes.assert_has_calls( - [mock.call(self.session, self.parent_task_id), mock.call(self.session, 22222)]) + self.list_task_output_all_volumes.assert_not_called() self.download_file.assert_not_called() def test_handle_download_invalid_file_name(self): @@ -321,6 +321,8 @@ class TestDownloadTask(utils.CliTestCase): self.assertExitCode(ex, 0) actual = self.stdout.getvalue() expected = """Usage: %s download-task +Default behavior without --all option downloads .rpm files only for build and buildArch tasks. + (Specify the --help global option for a list of other help options) Options: @@ -447,7 +449,7 @@ Options: ] self.list_task_output_all_volumes.side_effect = [ {'somerpm.src.rpm': ['DEFAULT', 'vol1']}, - {'somerpm.x86_64.rpm': ['DEFAULT', 'vol2']}, + {'somerpm.json': ['DEFAULT', 'vol2']}, {'somerpm.noarch.rpm': ['vol3'], 'somelog.log': ['DEFAULT', 'vol1']}, ] @@ -468,10 +470,10 @@ Options: call(self.session, 33333), call(self.session, 55555)]) self.assertListEqual(self.download_file.mock_calls, [ - call('https://topurl/work/tasks/3333/33333/somerpm.x86_64.rpm', - 'somerpm.x86_64.rpm', quiet=None, noprogress=None, size=3, num=1), - call('https://topurl/vol/vol2/work/tasks/3333/33333/somerpm.x86_64.rpm', - 'vol2/somerpm.x86_64.rpm', quiet=None, noprogress=None, size=3, num=2), + call('https://topurl/work/tasks/3333/33333/somerpm.json', + 'somerpm.x86_64.json', quiet=None, noprogress=None, size=3, num=1), + call('https://topurl/vol/vol2/work/tasks/3333/33333/somerpm.json', + 'vol2/somerpm.x86_64.json', quiet=None, noprogress=None, size=3, num=2), call('https://topurl/vol/vol3/work/tasks/5555/55555/somerpm.noarch.rpm', 'vol3/somerpm.noarch.rpm', quiet=None, noprogress=None, size=3, num=3), ]) @@ -718,3 +720,36 @@ Options: call(self.session, 44444), call(self.session, 55555)]) self.assertListEqual(self.download_file.mock_calls, []) + + def test_handle_download_task_without_all_json_not_downloaded(self): + args = [str(self.parent_task_id)] + self.session.getTaskInfo.return_value = self.parent_task_info + self.session.getTaskChildren.return_value = [] + self.list_task_output_all_volumes.return_value = { + 'somerpm.src.rpm': ['DEFAULT', 'vol1'], + 'somerpm.x86_64.rpm': ['DEFAULT', 'vol2'], + 'somerpm.noarch.rpm': ['vol3'], + 'somelog.log': ['DEFAULT', 'vol1'], + 'somefile.json': ['DEFAULT', 'vol1'], + } + + calls = self.gen_calls(self.list_task_output_all_volumes.return_value, + 'https://topurl/%swork/tasks/3333/123333/%s', + ['somelog.log', 'somefile.json']) + + # Run it and check immediate output + # args: task_id + # expected: success + rv = anon_handle_download_task(self.options, self.session, args) + + actual = self.stdout.getvalue() + expected = '' + self.assertMultiLineEqual(actual, expected) + # Finally, assert that things were called as we expected. + self.ensure_connection.assert_called_once_with(self.session, self.options) + self.session.getTaskInfo.assert_called_once_with(self.parent_task_id) + self.session.getTaskChildren.assert_called_once_with(self.parent_task_id) + self.list_task_output_all_volumes.assert_called_once_with(self.session, + self.parent_task_id) + self.assertListEqual(self.download_file.mock_calls, calls) + self.assertIsNone(rv)