PR#4123: download-logs: prefer getBuildLogs for builds - koji

koji

#4123 download-logs: prefer getBuildLogs for builds

Opened 8 months ago by mikem. Modified 6 hours ago

mikem/koji download-logs-nvr into master

fall back to task files for non-complete builds

Mike McLean • 7 months ago

4ea3dcf

download-logs: prefer getBuildLogs for builds

Mike McLean • 8 months ago

7dfd788

cli/koji_cli/commands.py

file modified

+13 -7

		`@@ -7035,25 +7035,31 @@`
		`save_logs(child_task['id'], match, task_log_dir, recurse)`

		`ensure_connection(session, options)`
		`- task_id = None`
		`- build_id = None`
		`for arg in args:`
		`+ task_id = None`
		`+ build_id = None`
		`if suboptions.nvr:`
		`suboptions.recurse = True`
		`binfo = session.getBuild(arg)`
		`if binfo is None:`
		`error("There is no build with n-v-r: %s" % arg)`
		`- if binfo.get('task_id'):`
		`- task_id = binfo['task_id']`
		`- sys.stdout.write("Using task ID: %s\n" % task_id)`
		`- elif binfo.get('build_id'):`
		`+ b_state = koji.BUILD_STATES[binfo['state']]`
		`+ # getBuildLogs will only work for complete builds`
		`+ if b_state == 'COMPLETE':`
		`build_id = binfo['build_id']`
		`sys.stdout.write("Using build ID: %s\n" % build_id)`
		`+ elif binfo.get('task_id'):`
		`+ # try falling back to the task logs`
		`+ task_id = binfo['task_id']`
		`+ sys.stdout.write("Build %s is %s\n" % (arg, b_state))`
		`+ sys.stdout.write("Using task ID: %s\n" % task_id)`
		`+ else:`
		`+ sys.stdout.write("Unable to download build %s (state=%s)\n" % (arg, b_state))`
		`else:`
		`try:`
		`task_id = int(arg)`
		`except ValueError:`
		`- error("Task id must be number: %r" % arg)`
		`+ error("Task id must be a number: %r" % arg)`
		`if task_id:`
		`save_logs(task_id, suboptions.match, suboptions.dir, suboptions.recurse)`
		`elif build_id:`

tests/test_cli/test_download_logs.py

file modified

+35 -5

		`@@ -3,6 +3,8 @@`
		`import mock`
		`import six`

		`+ import koji`
		`+`
		`from koji_cli.commands import anon_handle_download_logs`
		`from . import utils`

		`@@ -38,6 +40,7 @@`
		`""" % (self.progname, self.progname, self.progname)`
		`self.nvr = 'bash-1.2.3-f26'`
		`self.task_id = 123456`
		`+ self.build_id = 232323`

		`self.builtin_open = None`
		`if six.PY2:`
		`@@ -56,7 +59,7 @@`
		`anon_handle_download_logs,`
		`self.options, self.session, [task_id],`
		`stdout='',`
		`- stderr='Task id must be number: %r\n' % task_id,`
		`+ stderr='Task id must be a number: %r\n' % task_id,`
		`activate_session=None,`
		`exit_code=1`
		`)`
		`@@ -99,7 +102,31 @@`
		`self.session.getTaskChildren.assert_not_called()`

		`def test_anon_handle_download_logs_nvr(self):`
		`- self.session.getBuild.return_value = {'task_id': self.task_id}`
		`+ self.session.getBuild.return_value = {'task_id': self.task_id,`
		`+ 'build_id': self.build_id,`
		`+ 'state': koji.BUILD_STATES['COMPLETE']}`
		`+ self.session.getBuildLogs.return_value = [`
		`+ {'dir': 'noarch', 'name': 'test.log', 'path': 'path/to/test.log'},`
		`+ ]`
		`+ rv = anon_handle_download_logs(self.options, self.session, ['--nvr', self.nvr])`
		`+ actual = self.stdout.getvalue()`
		`+ expected = 'Using build ID: %s\n' % self.build_id`
		`+ self.assertMultiLineEqual(actual, expected)`
		`+ self.assertIsNone(rv)`
		`+`
		`+ self.session.getBuild.assert_called_once_with(self.nvr)`
		`+ self.session.getBuildLogs.assert_called_once_with(self.build_id)`
		`+ self.session.getTaskInfo.assert_not_called()`
		`+ self.session.downloadTaskOutput.assert_not_called()`
		`+ self.session.getTaskChildren.assert_not_called()`
		`+ self.download_file.assert_called_once()`
		`+`
		`+ def test_anon_handle_download_logs_nvr_with_task_id(self):`
		`+ # for a failed build with a task id we should fall back to fetching task logs`
		`+ self.session.getBuild.return_value = {'build_id': self.build_id,`
		`+ 'task_id': self.task_id,`
		`+ 'nvr': self.nvr,`
		`+ 'state': koji.BUILD_STATES['FAILED']}`
		`self.session.getTaskInfo.return_value = {`
		`'arch': 'x86_64',`
		`'state': 'CLOSED',`
		`@@ -109,7 +136,7 @@`

		`rv = anon_handle_download_logs(self.options, self.session, ['--nvr', self.nvr])`
		`actual = self.stdout.getvalue()`
		`- expected = 'Using task ID: %s\n' % self.task_id`
		`+ expected = 'Build %s is FAILED\nUsing task ID: %s\n' % (self.nvr, self.task_id)`
		`self.assertMultiLineEqual(actual, expected)`
		`self.assertIsNone(rv)`

		`@@ -118,11 +145,14 @@`
		`self.session.downloadTaskOutput.assert_not_called()`
		`self.session.getTaskChildren.assert_has_calls([mock.call(self.task_id), mock.call(23), ])`

		`+`
		`def test_anon_handle_download_logs_nvr_without_task_id(self):`
		`- self.session.getBuild.return_value = {'build_id': 1, 'nvr': self.nvr}`
		`+ self.session.getBuild.return_value = {'build_id': self.build_id,`
		`+ 'nvr': self.nvr,`
		`+ 'state': koji.BUILD_STATES['FAILED']}`
		`rv = anon_handle_download_logs(self.options, self.session, ['--nvr', self.nvr])`
		`actual = self.stdout.getvalue()`
		`- expected = 'Using build ID: 1\n'`
		`+ expected = 'Unable to download build %s (state=FAILED)\n' % self.nvr`
		`self.assertMultiLineEqual(actual, expected)`
		`self.assertIsNone(rv)`