)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"99683950e23bd664fd6ef00e39243490b257858c","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6399be_0a3ef5d9","line":1846,"updated":"2016-10-19 09:17:52.000000000","message":"The only difference between these two except blocks is that one uses e.format_message() in the LOG call, the other throws away the potentially useful exception message and passes in this fixed message string.\n\nIMHO, instead of pulling this code out into a method we should just merge these two except blocks into one, and always use e.format_message().","commit_id":"a484143401d496abe047459b2141def57784abd3"},{"author":{"_account_id":18603,"name":"Béla Vancsics","email":"vancsics@inf.u-szeged.hu","username":"vancsics"},"change_message_id":"2e17cde1b1581c93f707e02a3c8ec7be497dbf8b","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_c15ddc16","line":1846,"in_reply_to":"7a77a97e_fedfd909","updated":"2016-11-22 10:46:36.000000000","message":"Hi!\nI agree with Gabor and Ken\u0027ichis. I think this is good","commit_id":"a484143401d496abe047459b2141def57784abd3"},{"author":{"_account_id":18602,"name":"Gábor Antal","email":"antal@inf.u-szeged.hu","username":"antal"},"change_message_id":"06d30b4944dddfc791be9551bb21659cd7bd4d68","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_fedfd909","line":1846,"in_reply_to":"ba5da102_bef57acc","updated":"2016-11-22 10:35:56.000000000","message":"@Artom: Unfortunately, the log message is already merged :( \n\nI also agree with Ken\u0027ichis opinion. We except exception.BuildAbortException and it is explicitely in the code. Also, catching the generic exception is also good, and if we want keep both of them, this refactoring is correct, as it removes some copy-paste code snippet into a method, that makes this part more readable imho.","commit_id":"a484143401d496abe047459b2141def57784abd3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ca4748cfa11bccc7ed039e34a80667537a93d536","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_bef57acc","line":1846,"in_reply_to":"da6895a0_5b7c8520","updated":"2016-11-05 20:11:35.000000000","message":"I\u0027m going with Ken\u0027ichi on this one. The BuildAbortException is expected, so we print a message, clean up and move on.\n\nThe last catch-all exception is if there\u0027s a mistake in the code somewhere and a hitherto-unhandled exception is thrown. We still want to cleanup, but I\u0027d print a stacktrace in the logs (not sure at what level) to aid debugging.\n\nSo I think I\u0027m +1 the code cleanup into _build_exception(), but -1 on the log message in the last Exception block.","commit_id":"a484143401d496abe047459b2141def57784abd3"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"cff4af7779ee82dc03c08f1c9789b91b83925508","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6895a0_5b7c8520","line":1846,"in_reply_to":"da6895a0_a294a4e8","updated":"2016-10-21 07:51:51.000000000","message":"I agree that logging the \"Unexpected build failure\" is useful, as it allows to check for possible errors in the code. But Daniel\u0027s argument is also valid, so maybe make this a LOG.error() with the fixed msg and do the same LOG.exception() as in the BuildAbortException after that?","commit_id":"a484143401d496abe047459b2141def57784abd3"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"e320901c2c6d951884e1d0bfafdb38481cbe6561","unresolved":false,"context_lines":[{"line_number":1843,"context_line":"        except Exception as e:"},{"line_number":1844,"context_line":"            # Should not reach here."},{"line_number":1845,"context_line":"            msg \u003d _LE(\u0027Unexpected build failure, not rescheduling build.\u0027)"},{"line_number":1846,"context_line":"            LOG.exception(msg, instance\u003dinstance)"},{"line_number":1847,"context_line":"            return self._build_exception(context, instance,"},{"line_number":1848,"context_line":"                                         requested_networks,"},{"line_number":1849,"context_line":"                                         block_device_mapping, e)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6895a0_a294a4e8","line":1846,"in_reply_to":"fa6399be_0a3ef5d9","updated":"2016-10-20 20:05:59.000000000","message":"Daniel,\n\nThat was I also did think before.\nHowever, the merger will be the same as just removal of exception.BuildAbortException block because the later is \"except Exception as e:\".\n\nSo I felt that is not so good because exception.BuildAbortException seems expected.\nDo we really need to merge them into a single?","commit_id":"a484143401d496abe047459b2141def57784abd3"}]}