Graceful shutdown on Ctrl+C in blender/yacat #134
Conversation
| @@ -119,7 +119,20 @@ async def worker(ctx: WorkContext, tasks): | |||
| task = loop.create_task(main(subnet_tag=args.subnet_tag)) | |||
| try: | |||
| loop.run_until_complete(task) | |||
| except (Exception, KeyboardInterrupt) as e: | |||
| print(e) | |||
| except KeyboardInterrupt: | |||
There was a problem hiding this comment.
I'd put this into some utility function... context manager in examples/utils.py maybe?
There was a problem hiding this comment.
Good idea, but I would create an issue for this instead of including it in the current PR
There was a problem hiding this comment.
I prefer the example to fit in one file and be as short as possible. I discussed this with @mfranciszkiewicz last week - Marek, can you share your opinion here?
There was a problem hiding this comment.
IMHO utils.py is not the way to go
- the argument parser is trivial enough to be copied between examples; we're copying the
sys.pathexpansion code instead - if there's a need for creating common worker launcher code / logging facility, maybe it's worth considering making those a part of the library
I'm worried that the initial purpose of examples will be obscured and they'll be better off being separate apps instead.
There was a problem hiding this comment.
@mfranciszkiewicz thanks for your remarks. Including the code for building the parser in blender.py/yacat.py was also my idea at the beginning but then a reviewer convinced me that that code should be shared between the example scripts. That's how we ended up with a shared examples/utils.py module.
I'd consider adding some boilerplate code for creating an asyncio event loop and running the main task + handling graceful termination to the yapapi itself (and maybe pulling the code for creating commandline parser back to b lender.py/yacat.py) but that's not in the scope of the current PR.
| @@ -119,7 +119,20 @@ async def worker(ctx: WorkContext, tasks): | |||
| task = loop.create_task(main(subnet_tag=args.subnet_tag)) | |||
| try: | |||
| loop.run_until_complete(task) | |||
| except (Exception, KeyboardInterrupt) as e: | |||
| print(e) | |||
| except KeyboardInterrupt: | |||
There was a problem hiding this comment.
I prefer the example to fit in one file and be as short as possible. I discussed this with @mfranciszkiewicz last week - Marek, can you share your opinion here?
Resolves #118
Improves #132
Also resolves some issues reported after the last testing session
All asyncio task are
cancelled andgathered at the end ofExecutor.submit()to ensure that no "Exception was never retrieved" errors occur at shutdownKeyboardInterruptis caught at the end ofblender.py/yacat.pyto ensure that pressing Ctrl+C for the second time does not print traceback