PR#3425: Fix download-task all files in build/buildArch method tasks - koji

koji

#3425 Fix download-task all files in build/buildArch method tasks

Merged 2 years ago by tkopecek. Opened 2 years ago by jcupova.

jcupova/koji fix-download-task-usage into master

Fix download-task all files in build/buildArch method tasks

Jana Cupova • 2 years ago

3e25426

cli/koji_cli/commands.py

file modified

+46 -42

		`@@ -6897,7 +6897,9 @@`

		`def anon_handle_download_task(options, session, args):`
		`"[download] Download the output of a build task"`
		`- usage = "usage: %prog download-task <task_id>"`
		`+ usage = "usage: %prog download-task <task_id>\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), "`
		`@@ -6948,67 +6950,69 @@`
		`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.")`

tests/test_cli/test_download_task.py

file modified

+44 -9

		`@@ -57,6 +57,8 @@`
		`self.parent_task_info = {'id': self.parent_task_id, 'method': 'buildArch',`
		`'arch': 'taskarch', 'state': 2, 'parent': None}`
		`self.error_format = """Usage: %s download-task <task_id>`
		`+ 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 @@`
		`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 @@`
		`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 @@`
		`self.assertExitCode(ex, 0)`
		`actual = self.stdout.getvalue()`
		`expected = """Usage: %s download-task <task_id>`
		`+ 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 @@`
		`]`
		`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 @@`
		`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 @@`
		`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)`