)]}'
{"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"9c7d969f9749cfad9e20dfb45c1a15fb9fc77255","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            initialize_connection.assert_called_once_with(volume_id, connector)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":311,"context_line":"    def test_initialize_connection_exception(self, mock_cinderclient):"},{"line_number":312,"context_line":"        mock_cinderclient.return_value.volumes. \\"},{"line_number":313,"context_line":"            initialize_connection.side_effect \u003d ("},{"line_number":314,"context_line":"                cinder_exception.ClientException(500, \"500\"))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_3609ba96","line":311,"range":{"start_line":311,"start_character":8,"end_line":311,"end_character":44},"updated":"2016-04-04 19:39:23.000000000","message":"Perhaps append \u0027_no_code\u0027 to the test name since the goal of this test is to make sure it still works when the exception raised doesn\u0027t have a \u0027code\u0027 attribute.\n\ntest_initialize_connection_exception_no_code","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"c58aad429d85a800131920347d591a0109e1bdda","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            initialize_connection.assert_called_once_with(volume_id, connector)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":311,"context_line":"    def test_initialize_connection_exception(self, mock_cinderclient):"},{"line_number":312,"context_line":"        mock_cinderclient.return_value.volumes. \\"},{"line_number":313,"context_line":"            initialize_connection.side_effect \u003d ("},{"line_number":314,"context_line":"                cinder_exception.ClientException(500, \"500\"))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_eb358b46","line":311,"range":{"start_line":311,"start_character":8,"end_line":311,"end_character":44},"in_reply_to":"9a061dce_3609ba96","updated":"2016-04-05 01:26:55.000000000","message":"good point","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"9c7d969f9749cfad9e20dfb45c1a15fb9fc77255","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                test.TestingException)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        connector \u003d {\u0027host\u0027: \u0027fakehost1\u0027}"},{"line_number":320,"context_line":"        exc \u003d self.assertRaises(test.TestingException,"},{"line_number":321,"context_line":"                                self.api.terminate_connection,"},{"line_number":322,"context_line":"                                self.ctx,"},{"line_number":323,"context_line":"                                \u0027id1\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_f91733eb","line":320,"range":{"start_line":320,"start_character":32,"end_line":320,"end_character":53},"updated":"2016-04-04 19:39:23.000000000","message":"You\u0027re not actually going to get this exception returned because the save_and_reraise_exception context manager will re-raise the first exception (cinder_exception.ClientException).","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"c58aad429d85a800131920347d591a0109e1bdda","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                test.TestingException)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        connector \u003d {\u0027host\u0027: \u0027fakehost1\u0027}"},{"line_number":320,"context_line":"        exc \u003d self.assertRaises(test.TestingException,"},{"line_number":321,"context_line":"                                self.api.terminate_connection,"},{"line_number":322,"context_line":"                                self.ctx,"},{"line_number":323,"context_line":"                                \u0027id1\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_6bf33b9f","line":320,"range":{"start_line":320,"start_character":32,"end_line":320,"end_character":53},"in_reply_to":"9a061dce_f91733eb","updated":"2016-04-05 01:26:55.000000000","message":"yes, you\u0027re right","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"9c7d969f9749cfad9e20dfb45c1a15fb9fc77255","unresolved":false,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        connector \u003d {\u0027host\u0027: \u0027fakehost1\u0027}"},{"line_number":320,"context_line":"        exc \u003d self.assertRaises(test.TestingException,"},{"line_number":321,"context_line":"                                self.api.terminate_connection,"},{"line_number":322,"context_line":"                                self.ctx,"},{"line_number":323,"context_line":"                                \u0027id1\u0027,"},{"line_number":324,"context_line":"                                connector)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_f6ef42ae","line":321,"range":{"start_line":321,"start_character":41,"end_line":321,"end_character":61},"updated":"2016-04-04 19:39:23.000000000","message":"I think you mean \u0027initialize_connection\u0027 here.","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0d0a052d78ff4e20f9e43982bc22c37c1bbae8d6","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                          connector)"},{"line_number":327,"context_line":"        mock_log.error.assert_called_with("},{"line_number":328,"context_line":"            mock.ANY,"},{"line_number":329,"context_line":"            {\u0027vol\u0027: \u0027id1\u0027, \u0027host\u0027: \u0027fakehost1\u0027, \u0027msg\u0027: \u0027\u0027, \u0027code\u0027: None}"},{"line_number":330,"context_line":"        )"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a061dce_e76e2995","line":329,"range":{"start_line":329,"start_character":12,"end_line":329,"end_character":72},"updated":"2016-04-05 20:28:37.000000000","message":"This makes me a bit nervous about comparing dicts with a random hashseed, I\u0027m hoping that mock handles this for us in a sane way.\n\nAlternatively we could use call_args or call_args_list and get the args from the 2nd call (there would be 2 args, one for the Message object and one for the kwargs), then check that \u0027code\u0027 is in the kwargs and it\u0027s value is None.","commit_id":"e1f580e04223974b4ead91b7713d9c13dad97ec0"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"78ce69020310a7a5feea87c0f2cc7a69fac7fa4e","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                          connector)"},{"line_number":327,"context_line":"        mock_log.error.assert_called_with("},{"line_number":328,"context_line":"            mock.ANY,"},{"line_number":329,"context_line":"            {\u0027vol\u0027: \u0027id1\u0027, \u0027host\u0027: \u0027fakehost1\u0027, \u0027msg\u0027: \u0027\u0027, \u0027code\u0027: None}"},{"line_number":330,"context_line":"        )"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a061dce_f3cf027d","line":329,"range":{"start_line":329,"start_character":12,"end_line":329,"end_character":72},"in_reply_to":"9a061dce_e76e2995","updated":"2016-04-06 01:55:54.000000000","message":"Done, thanks for your guidance, Matt.","commit_id":"e1f580e04223974b4ead91b7713d9c13dad97ec0"}],"nova/volume/cinder.py":[{"author":{"_account_id":16907,"name":"Diana Clarke","email":"diana.joan.clarke@gmail.com","username":"diana-clarke"},"change_message_id":"9c7d969f9749cfad9e20dfb45c1a15fb9fc77255","unresolved":false,"context_lines":[{"line_number":435,"context_line":"                               \u0027host\u0027: connector.get(\u0027host\u0027),"},{"line_number":436,"context_line":"                               \u0027msg\u0027: six.text_type(exc),"},{"line_number":437,"context_line":"                               \u0027code\u0027: ("},{"line_number":438,"context_line":"                                exc.code if hasattr(exc, \u0027code\u0027) else None)})"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    @translate_volume_exception"},{"line_number":441,"context_line":"    def terminate_connection(self, context, volume_id, connector):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_36689af5","line":438,"updated":"2016-04-04 19:39:23.000000000","message":"I would assign this to a variable named \u0027code\u0027 to avoid having to wrap the line:\n\n    code \u003d exc.code if hasattr(exc, \u0027code\u0027) else None\n    LOG.error(_LE(\u0027Connection between volume %(vol)s and host \u0027\n                  \u0027%(host)s might have succeeded, but attempt \u0027\n                  \u0027to terminate connection has failed. \u0027\n                  \u0027Validate the connection and determine if \u0027\n                  \u0027manual cleanup is needed. Error: %(msg)s \u0027\n                  \u0027Code: %(code)s.\u0027),\n              {\u0027vol\u0027: volume_id,\n               \u0027host\u0027: connector.get(\u0027host\u0027),\n               \u0027msg\u0027: six.text_type(exc),\n               \u0027code\u0027: code})","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"c58aad429d85a800131920347d591a0109e1bdda","unresolved":false,"context_lines":[{"line_number":435,"context_line":"                               \u0027host\u0027: connector.get(\u0027host\u0027),"},{"line_number":436,"context_line":"                               \u0027msg\u0027: six.text_type(exc),"},{"line_number":437,"context_line":"                               \u0027code\u0027: ("},{"line_number":438,"context_line":"                                exc.code if hasattr(exc, \u0027code\u0027) else None)})"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    @translate_volume_exception"},{"line_number":441,"context_line":"    def terminate_connection(self, context, volume_id, connector):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a061dce_cb5fe770","line":438,"in_reply_to":"9a061dce_36689af5","updated":"2016-04-05 01:26:55.000000000","message":"not sure if it worth introducing a variable to avoid wrapping the line, just leave it as it is now","commit_id":"ed11999a547538c6b4129539cbae003e01be6771"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0d0a052d78ff4e20f9e43982bc22c37c1bbae8d6","unresolved":false,"context_lines":[{"line_number":435,"context_line":"                               \u0027host\u0027: connector.get(\u0027host\u0027),"},{"line_number":436,"context_line":"                               \u0027msg\u0027: six.text_type(exc),"},{"line_number":437,"context_line":"                               \u0027code\u0027: ("},{"line_number":438,"context_line":"                                exc.code if hasattr(exc, \u0027code\u0027) else None)})"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    @translate_volume_exception"},{"line_number":441,"context_line":"    def terminate_connection(self, context, volume_id, connector):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a061dce_27e4a139","line":438,"updated":"2016-04-05 20:28:37.000000000","message":"I\u0027m inclined to just say remove the code part of the error message since we log the error anyway, and for CinderClientException that contains the code, if one is set:\n\nhttps://github.com/openstack/python-cinderclient/blob/master/cinderclient/exceptions.py#L91\n\nBut that changes the translated error message which messes up backporting this a bit, so I\u0027m OK with leaving it in here and just checking for it.","commit_id":"e1f580e04223974b4ead91b7713d9c13dad97ec0"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ccc0b092a33c5cfa466ebab483a1fe61c2a90696","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                           \u0027code\u0027: ex.code})"},{"line_number":425,"context_line":"                try:"},{"line_number":426,"context_line":"                    self.terminate_connection(context, volume_id, connector)"},{"line_number":427,"context_line":"                except Exception as exc:"},{"line_number":428,"context_line":"                    LOG.error(_LE(\u0027Connection between volume %(vol)s and host \u0027"},{"line_number":429,"context_line":"                                  \u0027%(host)s might have succeeded, but attempt \u0027"},{"line_number":430,"context_line":"                                  \u0027to terminate connection has failed. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a061dce_60542821","line":427,"range":{"start_line":427,"start_character":23,"end_line":427,"end_character":39},"updated":"2016-04-08 08:25:10.000000000","message":"I think the root cause is this exception catching was too generic, we should catch CinderClientException and Exception separately. I saw there is comment from Matt about backport, I\u0027m not very clear how effect that, so leave the comment at here.","commit_id":"2135a3f55ff2222ec9c0fe7064aa339a430af4ee"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"57986f7284a051c6a4bcf3b27e0e97de7c4984b9","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                           \u0027code\u0027: ex.code})"},{"line_number":425,"context_line":"                try:"},{"line_number":426,"context_line":"                    self.terminate_connection(context, volume_id, connector)"},{"line_number":427,"context_line":"                except Exception as exc:"},{"line_number":428,"context_line":"                    LOG.error(_LE(\u0027Connection between volume %(vol)s and host \u0027"},{"line_number":429,"context_line":"                                  \u0027%(host)s might have succeeded, but attempt \u0027"},{"line_number":430,"context_line":"                                  \u0027to terminate connection has failed. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a061dce_b23e7f27","line":427,"range":{"start_line":427,"start_character":23,"end_line":427,"end_character":39},"in_reply_to":"9a061dce_6012635a","updated":"2016-04-08 14:03:17.000000000","message":"Alex, I\u0027d have to dig up the original change here, but it was made generic on purpose in case initialize_connection failed due to some connection error or rpc timeout on the cinder side, we\u0027d always at least attempt to terminate the connection rather than leave it hanging, but since we expect it might fail since initialize_connection failed, we use a generic Exception handler. We just can\u0027t assume \u0027code\u0027 is in the exception since it might not be a CinderClientException or NovaException per this bug.","commit_id":"2135a3f55ff2222ec9c0fe7064aa339a430af4ee"},{"author":{"_account_id":13248,"name":"Wenzhi Yu","email":"wenzhi_yu@163.com","username":"yuywz"},"change_message_id":"de1b0b901e77c5d44a673d9877c67f1396cb17b5","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                           \u0027code\u0027: ex.code})"},{"line_number":425,"context_line":"                try:"},{"line_number":426,"context_line":"                    self.terminate_connection(context, volume_id, connector)"},{"line_number":427,"context_line":"                except Exception as exc:"},{"line_number":428,"context_line":"                    LOG.error(_LE(\u0027Connection between volume %(vol)s and host \u0027"},{"line_number":429,"context_line":"                                  \u0027%(host)s might have succeeded, but attempt \u0027"},{"line_number":430,"context_line":"                                  \u0027to terminate connection has failed. \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a061dce_6012635a","line":427,"range":{"start_line":427,"start_character":23,"end_line":427,"end_character":39},"in_reply_to":"9a061dce_60542821","updated":"2016-04-08 09:36:38.000000000","message":"@Alex, thanks for your comment. It seems \u0027terminate_connection\u0027 method would also throw NovaException in some cases, not sure if is it appropriate to treat NovaException as Exception here, so maybe we should keep going with the generic exception catching(or catch CinderClientException, NovaException and Exception separately?).\n\nMatt\u0027s concern is that if we drop the \u0027code\u0027 attribute when\nlogging the error, the translated error message would be changed.","commit_id":"2135a3f55ff2222ec9c0fe7064aa339a430af4ee"}]}