)]}'
{"trove/taskmanager/models.py":[{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"77cdb72e048ac31b389f910d42b2d69ade7733f4","unresolved":false,"context_lines":[{"line_number":376,"context_line":"            msg \u003d _(\"An unexpected error occurred while getting \""},{"line_number":377,"context_line":"                    \"the replication source snapshot from %s\" %"},{"line_number":378,"context_line":"                    slave_of_id)"},{"line_number":379,"context_line":"        if delete_on_error:"},{"line_number":380,"context_line":"            err \u003d inst_models.InstanceTasks.BUILDING_ERROR_REPLICA"},{"line_number":381,"context_line":"            try:"},{"line_number":382,"context_line":"                Backup.delete(context, snapshot_info[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_2f3569f7","line":379,"updated":"2014-10-28 22:36:37.000000000","message":"You have to define in first before using it. If exception wouldn\u0027t take its place you wouldn\u0027t be able to address given variable.","commit_id":"503177f7b7d6f44a8c164e58195979c49146cf8b"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"77cdb72e048ac31b389f910d42b2d69ade7733f4","unresolved":false,"context_lines":[{"line_number":386,"context_line":"                                \"instance %(source)s\" %"},{"line_number":387,"context_line":"                                {\u0027source\u0027: slave_of_id}))"},{"line_number":388,"context_line":"                # Make sure we log any unexpected errors"},{"line_number":389,"context_line":"                if type(delete_on_error) is not TroveError:"},{"line_number":390,"context_line":"                    LOG.error(delete_on_error)"},{"line_number":391,"context_line":"                self._log_and_raise(e, msg, err)"},{"line_number":392,"context_line":"            self._log_and_raise(delete_on_error, msg, err)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_affdd93d","line":389,"updated":"2014-10-28 22:36:37.000000000","message":"can changed to: if not isinstance(...)","commit_id":"503177f7b7d6f44a8c164e58195979c49146cf8b"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"778d489e61407fd43d5a0ca46aae915cfff91ba6","unresolved":false,"context_lines":[{"line_number":388,"context_line":"                      \"a bad replication snapshot from instance %(source)s\") %"},{"line_number":389,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":390,"context_line":"                # Make sure we log any unexpected errors"},{"line_number":391,"context_line":"                if type(delete_on_error) is not TroveError:"},{"line_number":392,"context_line":"                    LOG.error(delete_on_error)"},{"line_number":393,"context_line":"                self._log_and_raise(e, msg, err)"},{"line_number":394,"context_line":"            self._log_and_raise(delete_on_error, msg, err)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_32930e51","line":391,"updated":"2014-10-28 22:56:54.000000000","message":"should be changed to: if not isinstance(...)","commit_id":"f4c33a671d74ea97f6ec34ff81e62ac2f02db561"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"82b1581e6934587afdf09732faf81ad619212026","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                     \"the replication source snapshot from %s\") %"},{"line_number":378,"context_line":"                   slave_of_id)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        if delete_on_error:"},{"line_number":381,"context_line":"            err \u003d inst_models.InstanceTasks.BUILDING_ERROR_REPLICA"},{"line_number":382,"context_line":"            try:"},{"line_number":383,"context_line":"                Backup.delete(context, snapshot_info[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_0e26ebba","line":380,"updated":"2014-10-29 21:02:10.000000000","message":"Why not just encapsulate all of this code in a method and call the method for each exception type. You can pass the msg as a parameter. That way you don\u0027t have to create the delete_on_error variable and add this branching.\n\nFor example:\n\ndef _delete_on_error(msg)\n   # add new code here\n\nexcept TroveError as te:\n    delete_on_error \u003d te\n    msg \u003d (_(\"Error creating replication snapshot from \"\n                   \"instance %(source)s for new replica %(replica)s.\") %\n                 {\u0027source\u0027: slave_of_id, \u0027replica\u0027: self.id})\n    _delete_on_error(msg)","commit_id":"255f010becb37fa0d01119677f17c47b110f8d6a"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"617f794568eb68c1f15aba2a38a476adae894fd5","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                     \"the replication source snapshot from %s\") %"},{"line_number":378,"context_line":"                   slave_of_id)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        if delete_on_error:"},{"line_number":381,"context_line":"            err \u003d inst_models.InstanceTasks.BUILDING_ERROR_REPLICA"},{"line_number":382,"context_line":"            try:"},{"line_number":383,"context_line":"                Backup.delete(context, snapshot_info[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_94958c8a","line":380,"in_reply_to":"7a8c0949_0e26ebba","updated":"2014-10-29 23:34:21.000000000","message":"Meant to not include delete_on_error \u003d te in example\n\ndef _delete_on_error(msg):\n   # add new code here\n\nexcept TroveError as te:\n    msg \u003d (_(\"Error creating replication snapshot from \"\n                   \"instance %(source)s for new replica %(replica)s.\") %\n                 {\u0027source\u0027: slave_of_id, \u0027replica\u0027: self.id})\n    _delete_on_error(msg)","commit_id":"255f010becb37fa0d01119677f17c47b110f8d6a"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"6523787830d5c9183e5ab71e30d03e447d3766b1","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                LOG.error(msg_create)"},{"line_number":379,"context_line":"                # Make sure we log any unexpected errors from the create"},{"line_number":380,"context_line":"                if not isinstance(e_create, TroveError):"},{"line_number":381,"context_line":"                    LOG.error(e_create)"},{"line_number":382,"context_line":"                msg_delete \u003d ("},{"line_number":383,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_a61de575","line":381,"updated":"2014-10-30 17:53:53.000000000","message":"Why is LOG.error(e_create) not done on line 370?","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"39feaa4f173ed10f6a0133e40fb543089ae74bb3","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                LOG.error(msg_create)"},{"line_number":379,"context_line":"                # Make sure we log any unexpected errors from the create"},{"line_number":380,"context_line":"                if not isinstance(e_create, TroveError):"},{"line_number":381,"context_line":"                    LOG.error(e_create)"},{"line_number":382,"context_line":"                msg_delete \u003d ("},{"line_number":383,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_d91496f7","line":381,"in_reply_to":"7a8c0949_a61de575","updated":"2014-10-30 18:04:36.000000000","message":"_log_and_raise will log the error (along with the exception) if it hits line 387.  This has to be last because it also re-throws the exception as a TroveError.  It\u0027s written to the log on line 381 because the delete caused an error, and that\u0027s the one that is now passed to _log_and_raise. If we don\u0027t print it out here, it will be lost.\n\nTo answer your question simply, if we print out the exception on line 370 and the delete is successful, it will be printed again on line 387.","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"6a424a9e3760cf92ec3eecc8f07952204a98558d","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                LOG.error(msg_create)"},{"line_number":379,"context_line":"                # Make sure we log any unexpected errors from the create"},{"line_number":380,"context_line":"                if not isinstance(e_create, TroveError):"},{"line_number":381,"context_line":"                    LOG.error(e_create)"},{"line_number":382,"context_line":"                msg_delete \u003d ("},{"line_number":383,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_f9d072f9","line":381,"in_reply_to":"7a8c0949_d91496f7","updated":"2014-10-30 18:13:26.000000000","message":"Gotcha, I missed the self._log_and_raise on 387.","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"2b41ff780402fc44ced18407352ce94d6174cb11","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                LOG.error(msg_create)"},{"line_number":379,"context_line":"                # Make sure we log any unexpected errors from the create"},{"line_number":380,"context_line":"                if not isinstance(e_create, TroveError):"},{"line_number":381,"context_line":"                    LOG.error(e_create)"},{"line_number":382,"context_line":"                msg_delete \u003d ("},{"line_number":383,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_07a137f8","line":381,"in_reply_to":"7a8c0949_f9d072f9","updated":"2014-11-01 19:25:16.000000000","message":"So the consequence of this construct is that if you are unlucky to get both an e_create exception on get_replication_snapshot and the delete exception on Backup.delete, you\u0027ll end up logging the delete exception (and raising that error) without ever logging the exception for e_create. Yes, you\u0027ll log the message at like 378, I get that. But what of the exception? Shouldn\u0027t you just log (Log.exception) at  378 and raise e_create and figure out which \u0027prize\u0027 you want to return to the winner if you do in fact receive two prizes?","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"cad809f7ad40b0030087da47252480aa86a3a9c2","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":385,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":386,"context_line":"                self._log_and_raise(e_delete, msg_delete, err)"},{"line_number":387,"context_line":"            self._log_and_raise(e_create, msg_create, err)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    def report_root_enabled(self):"},{"line_number":390,"context_line":"        mysql_models.RootHistory.create(self.context, self.id, \u0027root\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_8196e1ee","line":387,"updated":"2014-11-04 15:50:27.000000000","message":"How about wrapping the call to Backup.delete with try/catch with finally.\nLog and raise a_create exception in finally and just log a_delete exception in catch block.\n\nI think we have to remember that the interesting error here is the \"a_create\", \"a_delete\" is a side issue which I would like to see in the log but definitely not raise out to the caller.","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"36c49ab48e5e12dfd91b8b90d98e6fe756daf504","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":385,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":386,"context_line":"                self._log_and_raise(e_delete, msg_delete, err)"},{"line_number":387,"context_line":"            self._log_and_raise(e_create, msg_create, err)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    def report_root_enabled(self):"},{"line_number":390,"context_line":"        mysql_models.RootHistory.create(self.context, self.id, \u0027root\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_c507031c","line":387,"in_reply_to":"7a8c0949_0a8cba7e","updated":"2014-11-05 18:05:18.000000000","message":"If everyone thinks it\u0027s okay to log them the other way, I can change it (this case hopefully won\u0027t happen all that much).","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"1cd6889128ec01c41a76d37dde7847c32bf6afc5","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":385,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":386,"context_line":"                self._log_and_raise(e_delete, msg_delete, err)"},{"line_number":387,"context_line":"            self._log_and_raise(e_create, msg_create, err)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    def report_root_enabled(self):"},{"line_number":390,"context_line":"        mysql_models.RootHistory.create(self.context, self.id, \u0027root\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_0a8cba7e","line":387,"in_reply_to":"7a8c0949_8196e1ee","updated":"2014-11-05 16:04:34.000000000","message":"To address both concerns above:\n\nThe reason I chose to log the create first and then throw the delete (although I agree that the \u0027create\u0027 is more important) is that the log became confusing doing it the other way.  There\u0027s a message that basically says \u0027creating replication snapshot\u0027 and then immediately afterwards \u0027error deleting bad replication snapshot,\u0027 followed by \u0027error creating replication snapshot.\u0027 It wasn\u0027t obvious (to me) that the \u0027delete\u0027 error was caused by the create action, since it was reported first.\n\nIf I report the \u0027create\u0027 first, then the \u0027delete\u0027 and then throw e_create, then the create error is reported twice in the log (which I also found confusing).","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"861d318cf11c8c73be2ca5bb4f7099823a423c6c","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":385,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":386,"context_line":"                self._log_and_raise(e_delete, msg_delete, err)"},{"line_number":387,"context_line":"            self._log_and_raise(e_create, msg_create, err)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    def report_root_enabled(self):"},{"line_number":390,"context_line":"        mysql_models.RootHistory.create(self.context, self.id, \u0027root\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_6c3293f3","line":387,"in_reply_to":"7a8c0949_9e9f0950","updated":"2014-11-13 21:07:27.000000000","message":"The case I\u0027m trying to fix is where an exception is thrown on line 375.  Log messages on line 378 would never be executed.\n\nOne thing to remember with all this back-and-forth.  The users never see these messages.  The instance goes into ERROR state and that\u0027s all they know.  It\u0027s up to someone (Ops) to dig into the log to find out what went wrong.  I\u0027ve structured this code so that the error flow made sense to me when reading the log, since that\u0027s the only place they\u0027re ever going to be seen.","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"86e3095f1545a84057b671fd678160de35e1df44","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":385,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":386,"context_line":"                self._log_and_raise(e_delete, msg_delete, err)"},{"line_number":387,"context_line":"            self._log_and_raise(e_create, msg_create, err)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"    def report_root_enabled(self):"},{"line_number":390,"context_line":"        mysql_models.RootHistory.create(self.context, self.id, \u0027root\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_9e9f0950","line":387,"in_reply_to":"7a8c0949_c507031c","updated":"2014-11-06 08:42:02.000000000","message":"Just in case you consider to throw less important exception, please, consider just to add a log message in the original code (less code is needed to achieve the same goal):\n\nexcept Exception:\n     # log message here\n\t            \n     Backup.delete(context, snapshot_info[\u0027id\u0027])\n     raise","commit_id":"f2365b67aea9576eb6dea09eb41b865d6be13d5b"},{"author":{"_account_id":8415,"name":"Denis M.","email":"lildee1991@gmail.com","username":"dmakogon"},"change_message_id":"2a7e015afb2c3d407c2ae6651e8d19cffd63674c","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    LOG.error(e_create)"},{"line_number":384,"context_line":"                msg_delete \u003d ("},{"line_number":385,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":386,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":387,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":388,"context_line":"                # we\u0027ve already logged the create exception, so we\u0027ll raise"},{"line_number":389,"context_line":"                # the delete (otherwise the create will be logged twice)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_98326a5d","line":386,"updated":"2014-11-15 18:47:26.000000000","message":"missing period","commit_id":"1c284cbc0d83dbc1b3a9211596a5b3b188477b3c"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"8b6cd774708085d9fa4e0dc7ad9a2629ab5988e3","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                    LOG.error(e_create)"},{"line_number":384,"context_line":"                msg_delete \u003d ("},{"line_number":385,"context_line":"                    _(\"An error occurred while deleting a bad \""},{"line_number":386,"context_line":"                      \"replication snapshot from instance %(source)s\") %"},{"line_number":387,"context_line":"                    {\u0027source\u0027: slave_of_id})"},{"line_number":388,"context_line":"                # we\u0027ve already logged the create exception, so we\u0027ll raise"},{"line_number":389,"context_line":"                # the delete (otherwise the create will be logged twice)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_fdf501ed","line":386,"in_reply_to":"5a890539_98326a5d","updated":"2014-11-17 16:42:29.000000000","message":"Done","commit_id":"1c284cbc0d83dbc1b3a9211596a5b3b188477b3c"}]}