From 7dfd788faad34855b65927f41f05002d57bebc11 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 01 2024 20:18:14 +0000 Subject: [PATCH 1/2] download-logs: prefer getBuildLogs for builds --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index a44dcaa..ebd9e7b 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -7035,25 +7035,25 @@ def anon_handle_download_logs(options, session, args): 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'): + if binfo.get('build_id'): build_id = binfo['build_id'] sys.stdout.write("Using build ID: %s\n" % build_id) + elif binfo.get('task_id'): + task_id = binfo['task_id'] + sys.stdout.write("Using task ID: %s\n" % task_id) 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: diff --git a/tests/test_cli/test_download_logs.py b/tests/test_cli/test_download_logs.py index ca7bb64..9b4b1ab 100644 --- a/tests/test_cli/test_download_logs.py +++ b/tests/test_cli/test_download_logs.py @@ -56,7 +56,7 @@ Note this command only downloads task logs, not build logs. 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 ) From 4ea3dcf96b3c13001828eb1da582c3c3c4fde99b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 13 2024 18:44:22 +0000 Subject: [PATCH 2/2] fall back to task files for non-complete builds --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index ebd9e7b..b91549e 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -7043,12 +7043,18 @@ def anon_handle_download_logs(options, session, args): binfo = session.getBuild(arg) if binfo is None: error("There is no build with n-v-r: %s" % arg) - if 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) diff --git a/tests/test_cli/test_download_logs.py b/tests/test_cli/test_download_logs.py index 9b4b1ab..a5d9033 100644 --- a/tests/test_cli/test_download_logs.py +++ b/tests/test_cli/test_download_logs.py @@ -3,6 +3,8 @@ from __future__ import absolute_import import mock import six +import koji + from koji_cli.commands import anon_handle_download_logs from . import utils @@ -38,6 +40,7 @@ Note this command only downloads task logs, not build logs. """ % (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: @@ -99,7 +102,31 @@ Note this command only downloads task logs, not build logs. 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 @@ Note this command only downloads task logs, not build logs. 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 @@ Note this command only downloads task logs, not build logs. 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)