PR#981: cli: add a param in watch_tasks to override KeyboardInterrupt output - koji

koji

#981 cli: add a param in watch_tasks to override KeyboardInterrupt output

Merged 5 years ago by mikem. Opened 5 years ago by julian8628.

julian8628/koji issue/976 into master

whitespace

Mike McLean • 5 years ago

8923181

move inner function out of except clause

Mike McLean • 5 years ago

0c2b094

move quiet option into ki_handler

Yuming Zhu • 5 years ago

f1fd867

cli: add a param in watch_tasks to override KeyboardInterrupt output

Yuming Zhu • 5 years ago

9d3cd4c

cli/koji_cli/lib.py

file modified

+12 -8

		`@@ -276,11 +276,20 @@`
		`print('%s has not completed' % task_label)`


		`- def watch_tasks(session, tasklist, quiet=False, poll_interval=60):`
		`+ def watch_tasks(session, tasklist, quiet=False, poll_interval=60, ki_handler=None):`
		`if not tasklist:`
		`return`
		`if not quiet:`
		`print("Watching tasks (this may be safely interrupted)...")`
		`+ if ki_handler is None:`
		`+ def ki_handler(progname, tasks, quiet):`
		`+ if not quiet:`
		`+ tlist = ['%s: %s' % (t.str(), t.display_state(t.info))`
		`+ for t in tasks.values() if not t.is_done()]`
		`+ print(`
		`+ """Tasks still running. You can continue to watch with the '%s watch-task' command.`
		`+ Running Tasks:`
		`+ %s""" % (progname, '\n'.join(tlist)))`
		`sys.stdout.flush()`
		`rv = 0`
		`try:`
		`@@ -317,14 +326,9 @@`
		`sys.stdout.flush()`
		`time.sleep(poll_interval)`
		`except KeyboardInterrupt:`
		`- if tasks and not quiet:`
		`+ if tasks:`
		`progname = os.path.basename(sys.argv[0]) or 'koji'`
		`- tlist = ['%s: %s' % (t.str(), t.display_state(t.info))`
		`- for t in tasks.values() if not t.is_done()]`
		`- print( \`
		`- """Tasks still running. You can continue to watch with the '%s watch-task' command.`
		`- Running Tasks:`
		`- %s""" % (progname, '\n'.join(tlist)))`
		`+ ki_handler(progname, tasks, quiet)`
		`raise`
		`return rv`

tests/test_cli/test_watch_tasks.py

file modified

+26

		`@@ -121,6 +121,32 @@`
		`''' % (os.path.basename(sys.argv[0]) or 'koji'))`
		`self.assertMultiLineEqual(stdout.getvalue(), expected)`

		`+ @mock.patch('time.sleep')`
		`+ @mock.patch('sys.stdout', new_callable=six.StringIO)`
		`+ def test_watch_tasks_with_keyboardinterrupt_handler(self, stdout, sleep):`
		`+ """Raise KeyboardInterrupt inner watch_tasks with a ki_handler"""`
		`+ cfile = os.path.dirname(__file__) + '/data/calls/watchtasks2.json'`
		`+ with open(cfile) as fp:`
		`+ cdata = json.load(fp)`
		`+ self.session.load_calls(cdata)`
		`+ sleep.side_effect = [None] * 10 + [KeyboardInterrupt]`
		`+`
		`+ def customized_handler(progname, tasks, quiet):`
		`+ print('some output')`
		`+`
		`+ with self.assertRaises(KeyboardInterrupt):`
		`+ # watch_tasks catches and re-raises it to display a message`
		`+ watch_tasks(self.session, [1208], quiet=False, poll_interval=5,`
		`+ ki_handler=customized_handler)`
		`+ expected = ('''Watching tasks (this may be safely interrupted)...`
		`+ 1208 build (f24, /users/mikem/fake.git:master): free`
		`+ 1208 build (f24, /users/mikem/fake.git:master): free -> open (builder-01)`
		`+ 1209 buildSRPMFromSCM (/users/mikem/fake.git:master): free`
		`+ 1209 buildSRPMFromSCM (/users/mikem/fake.git:master): free -> open (builder-01)`
		`+ some output`
		`+ ''')`
		`+ self.assertMultiLineEqual(stdout.getvalue(), expected)`
		`+`

		`if __name__ == '__main__':`
		`unittest.main()`

julian8628 commented 5 years ago

fixes #976

tkopecek commented on line 18 of cli/koji_cli/lib.py 5 years ago

Maybe adding quiet option and calling it in if tasks branch would make it slightly more usable. In default implementation it will do nothing, but in some client it can do some emergency action even in quite mode.

1 new commit added

move quiet option into ki_handler

5 years ago

julian8628 commented 5 years ago

Maybe adding quiet option and calling it in if tasks branch would make it slightly more usable. In default implementation it will do nothing, but in some client it can do some emergency action even in quite mode.

updated

tkopecek commented 5 years ago

:thumbsup:

mikem commented 5 years ago

Seems odd to define an inner function at that point. Clearer to define it at the beginning.

https://pagure.io/fork/mikem/koji/commits/pagure/pr/981

rebased onto 9d3cd4c

5 years ago

julian8628 commented 5 years ago