)]}'
{"kolla/cmd/build.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"49ffee91fcb460c85352c079dc6a1e75afaf9bf8","unresolved":false,"context_lines":[{"line_number":53,"context_line":"from kolla import version"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def handle_ctrlc(single, frame):"},{"line_number":57,"context_line":"    kollaobj \u003d frame.f_locals[\u0027kolla\u0027]"},{"line_number":58,"context_line":"    kollaobj.cleanup()"},{"line_number":59,"context_line":"    sys.exit(1)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"signal.signal(signal.SIGINT, handle_ctrlc)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"class KollaDirNotFoundException(Exception):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9abb7d3a_b697e046","side":"PARENT","line":62,"range":{"start_line":56,"start_character":0,"end_line":62,"end_character":42},"updated":"2016-06-01 01:39:21.000000000","message":"would it not have been better to keep this and just have it set self.needs_to_stop to true instead of calling sys.exit(1) to avoid catching the exception on line 914.\n\npythons exception handling is fast but its probably still better to never raise the exception in the first place.","commit_id":"6abc8f5e37eb81227c23d144bea21ca6e5378a8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f584ce89f37ec05444fbc0cce278f6836ec97655","unresolved":false,"context_lines":[{"line_number":53,"context_line":"from kolla import version"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def handle_ctrlc(single, frame):"},{"line_number":57,"context_line":"    kollaobj \u003d frame.f_locals[\u0027kolla\u0027]"},{"line_number":58,"context_line":"    kollaobj.cleanup()"},{"line_number":59,"context_line":"    sys.exit(1)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"signal.signal(signal.SIGINT, handle_ctrlc)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"class KollaDirNotFoundException(Exception):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9abb7d3a_ced56d43","side":"PARENT","line":62,"range":{"start_line":56,"start_character":0,"end_line":62,"end_character":42},"in_reply_to":"9abb7d3a_676b04f2","updated":"2016-06-02 00:11:59.000000000","message":"oh cool thanks for the pointer.\nthe post looks interesting ill add it to my reading list.","commit_id":"6abc8f5e37eb81227c23d144bea21ca6e5378a8c"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"7fae1d56f53727c32ef847e9c71ef50ff9b4efe3","unresolved":false,"context_lines":[{"line_number":53,"context_line":"from kolla import version"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def handle_ctrlc(single, frame):"},{"line_number":57,"context_line":"    kollaobj \u003d frame.f_locals[\u0027kolla\u0027]"},{"line_number":58,"context_line":"    kollaobj.cleanup()"},{"line_number":59,"context_line":"    sys.exit(1)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"signal.signal(signal.SIGINT, handle_ctrlc)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"class KollaDirNotFoundException(Exception):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9abb7d3a_676b04f2","side":"PARENT","line":62,"range":{"start_line":56,"start_character":0,"end_line":62,"end_character":42},"in_reply_to":"9abb7d3a_b697e046","updated":"2016-06-01 04:32:52.000000000","message":"What this is doing is installing a non-standard handler of that exception, instead of using the built-in way this is handled. \n\nhttps://docs.python.org/2/library/exceptions.html#exceptions.KeyboardInterrupt\n\nThis program also uses threads, so IMHO `sys.exit` is never really a good answer to much of anything in a program that is written to have concurrent things happening at the same time (because it does not allow for orderly shutdown and cleanup of resources those concurrent threads are working on).\n\nThis signal handler is basically duplicating something that the python runtime is already handling, and for no real good reason, so ya, shouldn\u0027t be needing to do this.\n\nProbably good to read over: http://snakesthatbite.blogspot.com/2010/09/cpython-threading-interrupting.html","commit_id":"6abc8f5e37eb81227c23d144bea21ca6e5378a8c"},{"author":{"_account_id":3098,"name":"Jeff Peeler","email":"jpeeler@redhat.com","username":"jpeeler-z"},"change_message_id":"4fca5007d1ff42ff33a8444b072f04b9fe87e688","unresolved":false,"context_lines":[{"line_number":454,"context_line":"                        LOG.debug(\"Attempting to run task %s for the first\""},{"line_number":455,"context_line":"                                  \" time\", task.name)"},{"line_number":456,"context_line":"                    try:"},{"line_number":457,"context_line":"                        task.run()"},{"line_number":458,"context_line":"                        if task.success:"},{"line_number":459,"context_line":"                            break"},{"line_number":460,"context_line":"                    except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9abb7d3a_5af7c058","line":457,"updated":"2016-05-31 20:45:38.000000000","message":"This means now the operation won\u0027t halt until after an image is completely built, right?","commit_id":"82972b4e6bd834e5c4f242f9a3f2d3804e3c1547"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"5c9cab905974cf9618af50896729b0eb395535f5","unresolved":false,"context_lines":[{"line_number":454,"context_line":"                        LOG.debug(\"Attempting to run task %s for the first\""},{"line_number":455,"context_line":"                                  \" time\", task.name)"},{"line_number":456,"context_line":"                    try:"},{"line_number":457,"context_line":"                        task.run()"},{"line_number":458,"context_line":"                        if task.success:"},{"line_number":459,"context_line":"                            break"},{"line_number":460,"context_line":"                    except Exception:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9abb7d3a_93762ed5","line":457,"in_reply_to":"9abb7d3a_5af7c058","updated":"2016-06-01 01:11:08.000000000","message":"Yup that\u0027s correct.\n\nOptions IMHO for making this better are the following.\n\n* Split up the build image task into smaller pieces (allowing for stopping in between those pieces); ie a big upfront task is the processing of the image pieces (that likely shouldn\u0027t all be combined together into the same task).\n\n* Add a way to stop the main blocking operation that I assume takes the longest, the call into the docker client \u0027build\u0027 api.\n\nDoes that client itself support some type of abort api, or async execution where u first request a build to start, then that start command returns some identifier that can be polled on (or aborted) at some point in the future.\n\nThen if ctrl-c is triggered, stop in between the tasks that are up to the build() call, and if the stop is \u0027to late\u0027 and the client is already building the image, figure out how to use that async api (assuming it exists) to tell the underlying docker service to abort the build.","commit_id":"82972b4e6bd834e5c4f242f9a3f2d3804e3c1547"}]}