PR#625: watch-logs --mine --follow - koji

koji

#625 watch-logs --mine --follow

Merged 6 years ago by mikem. Opened 6 years ago by tkopecek.

tkopecek/koji issue621 into master

unit test

Tomas Kopecek • 6 years ago

58fd917

move _list_task to client lib

Tomas Kopecek • 6 years ago

926d139

watch-logs --mine --follow

Tomas Kopecek • 6 years ago

d7ff474

cli/koji_cli/commands.py

file modified

+13 -60

		`@@ -37,7 +37,7 @@`
		`_unique_path, _running_in_bg, _progress_callback, watch_tasks, \`
		`arg_filter, linked_upload, list_task_output_all_volumes, \`
		`print_task_headers, print_task_recurse, download_file, watch_logs, \`
		`- error, greetings`
		`+ error, greetings, _list_tasks`


		`def _printable_unicode(s):`
		`@@ -6010,59 +6010,6 @@`
		`session.setTaskPriority(task_id, options.priority, options.recurse)`


		`- def _list_tasks(options, session):`
		`- "Retrieve a list of tasks"`
		`-`
		`- callopts = {`
		`- 'state' : [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`- 'decode' : True,`
		`- }`
		`-`
		`- if options.mine:`
		`- user = session.getLoggedInUser()`
		`- if not user:`
		`- print("Unable to determine user")`
		`- sys.exit(1)`
		`- callopts['owner'] = user['id']`
		`- if options.user:`
		`- user = session.getUser(options.user)`
		`- if not user:`
		`- print("No such user: %s" % options.user)`
		`- sys.exit(1)`
		`- callopts['owner'] = user['id']`
		`- if options.arch:`
		`- callopts['arch'] = parse_arches(options.arch, to_list=True)`
		`- if options.method:`
		`- callopts['method'] = options.method`
		`- if options.channel:`
		`- chan = session.getChannel(options.channel)`
		`- if not chan:`
		`- print("No such channel: %s" % options.channel)`
		`- sys.exit(1)`
		`- callopts['channel_id'] = chan['id']`
		`- if options.host:`
		`- host = session.getHost(options.host)`
		`- if not host:`
		`- print("No such host: %s" % options.host)`
		`- sys.exit(1)`
		`- callopts['host_id'] = host['id']`
		`-`
		`- qopts = {'order' : 'priority,create_time'}`
		`- tasklist = session.listTasks(callopts, qopts)`
		`- tasks = dict([(x['id'], x) for x in tasklist])`
		`-`
		`- #thread the tasks`
		`- for t in tasklist:`
		`- if t['parent'] is not None:`
		`- parent = tasks.get(t['parent'])`
		`- if parent:`
		`- parent.setdefault('children',[])`
		`- parent['children'].append(t)`
		`- t['sub'] = True`
		`-`
		`- return tasklist`
		`-`
		`-`
		`def handle_list_tasks(goptions, session, args):`
		`"[info] Print the list of tasks"`
		`usage = _("usage: %prog list-tasks [options]")`
		`@@ -6240,15 +6187,21 @@`
		`usage += _("\n(Specify the --help global option for a list of other help options)")`
		`parser = OptionParser(usage=usage)`
		`parser.add_option("--log", help=_("Watch only a specific log"))`
		`+ parser.add_option("--mine", action="store_true", help=_("Watch logs for all your tasks"))`
		`+ parser.add_option("--follow", action="store_true", help=_("Follow spawned child tasks"))`
		`(options, args) = parser.parse_args(args)`
		`activate_session(session, goptions)`

		`- tasks = []`
		`- for task in args:`
		`- try:`
		`- tasks.append(int(task))`
		`- except ValueError:`
		`- parser.error(_("task id must be an integer"))`
		`+ if options.mine:`
		`+ tasks = _list_tasks(options, session)`
		`+ tasks = [t['id'] for t in tasks]`
		`+ else:`
		`+ tasks = []`
		`+ for task in args:`
		`+ try:`
		`+ tasks.append(int(task))`
		`+ except ValueError:`
		`+ parser.error(_("task id must be an integer"))`
		`if not tasks:`
		`parser.error(_("at least one task id must be specified"))`

cli/koji_cli/lib.py

file modified

+61

		`@@ -377,6 +377,12 @@`
		`lastlog = currlog`
		`sys.stdout.write(contents.decode('utf8'))`

		`+ if opts.follow:`
		`+ for child in session.getTaskChildren(task_id):`
		`+ if child['id'] not in tasklist:`
		`+ tasklist.append(child['id'])`
		`+ offsets[child['id']] = {}`
		`+`
		`if not tasklist:`
		`break`

		`@@ -571,3 +577,58 @@`
		`ensure_connection(session)`
		`if options.debug:`
		`print("successfully connected to hub")`
		`+`
		`+`
		`+ def _list_tasks(options, session):`
		`+ "Retrieve a list of tasks"`
		`+`
		`+ callopts = {`
		`+ 'state' : [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode' : True,`
		`+ }`
		`+`
		`+ if getattr(options, 'mine'):`
		`+ if getattr(options, 'user'):`
		`+ raise koji.GenericError("Can't specify 'mine' and 'user' in same time")`
		`+ user = session.getLoggedInUser()`
		`+ if not user:`
		`+ print("Unable to determine user")`
		`+ sys.exit(1)`
		`+ callopts['owner'] = user['id']`
		`+ if getattr(options, 'user'):`
		`+ user = session.getUser(options.user)`
		`+ if not user:`
		`+ print("No such user: %s" % options.user)`
		`+ sys.exit(1)`
		`+ callopts['owner'] = user['id']`
		`+ if getattr(options, 'arch'):`
		`+ callopts['arch'] = parse_arches(options.arch, to_list=True)`
		`+ if getattr(options, 'method'):`
		`+ callopts['method'] = options.method`
		`+ if getattr(options, 'channel'):`
		`+ chan = session.getChannel(options.channel)`
		`+ if not chan:`
		`+ print("No such channel: %s" % options.channel)`
		`+ sys.exit(1)`
		`+ callopts['channel_id'] = chan['id']`
		`+ if getattr(options, 'host'):`
		`+ host = session.getHost(options.host)`
		`+ if not host:`
		`+ print("No such host: %s" % options.host)`
		`+ sys.exit(1)`
		`+ callopts['host_id'] = host['id']`
		`+`
		`+ qopts = {'order' : 'priority,create_time'}`
		`+ tasklist = session.listTasks(callopts, qopts)`
		`+ tasks = dict([(x['id'], x) for x in tasklist])`
		`+`
		`+ #thread the tasks`
		`+ for t in tasklist:`
		`+ if t['parent'] is not None:`
		`+ parent = tasks.get(t['parent'])`
		`+ if parent:`
		`+ parent.setdefault('children',[])`
		`+ parent['children'].append(t)`
		`+ t['sub'] = True`
		`+`
		`+ return tasklist`

tests/test_cli/test_list_tasks.py

file added

+150

		`@@ -0,0 +1,150 @@`
		`+ import mock`
		`+ import unittest`
		`+`
		`+ import koji`
		`+ from koji_cli.lib import _list_tasks`
		`+`
		`+ class TestListTasks(unittest.TestCase):`
		`+ def setUp(self):`
		`+ pass`
		`+`
		`+ @mock.patch('sys.exit')`
		`+ def test_list_tasks(self, sys_exit):`
		`+ options = mock.MagicMock(name='options')`
		`+ options.mine = True`
		`+ options.user = None`
		`+ options.arch = None`
		`+ options.method = None`
		`+ options.channel = None`
		`+ options.host = None`
		`+ session = mock.MagicMock(name='session')`
		`+ session.getLoggedInUser.return_value = {'id': 1, 'username': 'name'}`
		`+ session.listTasks.return_value = []`
		`+ sys_exit.side_effect = RuntimeError`
		`+`
		`+ # mine`
		`+ r = _list_tasks(options, session)`
		`+ self.assertEqual(r, [])`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'owner': 1,`
		`+ }, {'order' : 'priority,create_time'})`
		`+`
		`+ # invalid me`
		`+ session.getLoggedInUser.return_value = None`
		`+ with self.assertRaises(RuntimeError):`
		`+ _list_tasks(options, session)`
		`+`
		`+ # mine + user -> error`
		`+ options.user = 2`
		`+ with self.assertRaises(koji.GenericError):`
		`+ _list_tasks(options, session)`
		`+`
		`+ # only user`
		`+ session.listTasks.reset_mock()`
		`+ options.mine = None`
		`+ session.getUser.return_value = {'id': 2, 'username': 'name'}`
		`+ _list_tasks(options, session)`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'owner': 2,`
		`+ }, {'order' : 'priority,create_time'})`
		`+`
		`+ # invalid user`
		`+ session.getUser.return_value = None`
		`+ with self.assertRaises(RuntimeError):`
		`+ _list_tasks(options, session)`
		`+`
		`+ # only arch`
		`+ session.listTasks.reset_mock()`
		`+ options.user = None`
		`+ options.arch = 'x86_64,i386'`
		`+ _list_tasks(options, session)`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'arch': ['x86_64', 'i386'],`
		`+ }, {'order' : 'priority,create_time'})`
		`+`
		`+ # only method`
		`+ session.listTasks.reset_mock()`
		`+ options.arch = None`
		`+ options.method = 'method'`
		`+ _list_tasks(options, session)`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'method': 'method',`
		`+ }, {'order' : 'priority,create_time'})`
		`+`
		`+ # only channel`
		`+ session.listTasks.reset_mock()`
		`+ options.method = None`
		`+ options.channel = 'channel'`
		`+ session.getChannel.return_value = {'id': 123, 'name': 'channel'}`
		`+ _list_tasks(options, session)`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'channel_id': 123,`
		`+ }, {'order' : 'priority,create_time'})`
		`+ session.getChannel.assert_called_once_with('channel')`
		`+`
		`+ # invalid channel`
		`+ session.getChannel.return_value = None`
		`+ with self.assertRaises(RuntimeError):`
		`+ _list_tasks(options, session)`
		`+`
		`+ # only host`
		`+ session.listTasks.reset_mock()`
		`+ options.channel = None`
		`+ options.host = 'host'`
		`+ session.getHost.return_value = {'id': 234}`
		`+ _list_tasks(options, session)`
		`+ session.listTasks.assert_called_once_with({`
		`+ 'state': [koji.TASK_STATES[s] for s in ('FREE', 'OPEN', 'ASSIGNED')],`
		`+ 'decode': True,`
		`+ 'host_id': 234,`
		`+ }, {'order' : 'priority,create_time'})`
		`+ session.getHost.assert_called_once_with('host')`
		`+`
		`+ # invalid host`
		`+ session.getHost.return_value = None`
		`+ with self.assertRaises(RuntimeError):`
		`+ _list_tasks(options, session)`
		`+`
		`+ # parent/children threading`
		`+ options.host = None`
		`+ session.listTasks.return_value = [`
		`+ {'id': 1, 'parent': None},`
		`+ {'id': 2, 'parent': 1},`
		`+ {'id': 3, 'parent': 2},`
		`+ ]`
		`+ r = _list_tasks(options, session)`
		`+ self.assertEqual(r, [`
		`+ {`
		`+ 'children': [`
		`+ {`
		`+ 'children': [ {'id': 3, 'parent': 2, 'sub': True}],`
		`+ 'id': 2,`
		`+ 'parent': 1,`
		`+ 'sub': True`
		`+ }`
		`+ ],`
		`+ 'id': 1,`
		`+ 'parent': None`
		`+ },`
		`+ {`
		`+ 'children': [{'id': 3, 'parent': 2, 'sub': True}],`
		`+ 'id': 2,`
		`+ 'parent': 1,`
		`+ 'sub': True`
		`+ },`
		`+ {`
		`+ 'id': 3,`
		`+ 'parent': 2,`
		`+ 'sub': True}`
		`+ ])`
		`+`

tkopecek commented 6 years ago

mikem commented on line 21 of cli/koji_cli/commands.py 6 years ago

I wonder about setting these options here. Would it be better for _list_tasks to use getattr?
At least worth an explanatory comment.

mikem commented 6 years ago

Also, this approach will break if we add a new option to the list tasks command without also adding it here. That's not necessarily a problem with this approach, but we should probably have a unit test that will catch this. Right now, if I delete one of these options the command fails, but the unit tests pass.

tkopecek commented 6 years ago

I've moved _list_tasks to lib (as it is now used by more functions) and changed option retrieval to getattr.

1 new commit added