)]}'
{"neutron/services/trunk/drivers/openvswitch/agent/driver.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0998e114541c7e4c8f598cb25f377447b8aca6fe","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                        ctx, trunk_id, subports)"},{"line_number":59,"context_line":"                elif event_type \u003d\u003d events.DELETED:"},{"line_number":60,"context_line":"                    subport_ids \u003d [subport.port_id for subport in subports]"},{"line_number":61,"context_line":"                    self.ovsdb_handler.unwire_subports_for_trunk("},{"line_number":62,"context_line":"                        trunk_id, subport_ids)"},{"line_number":63,"context_line":"                else:"},{"line_number":64,"context_line":"                    LOG.error(_LE(\"Unknown event type %s\"), event_type)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_8cac25ef","line":61,"updated":"2016-09-14 00:09:38.000000000","message":"unwire_subports_for_trunk does not raise this exception, at least not right now. I wonder if you want to be more defensive.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"159694a050f771de7648f860b090104f138fc049","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                        ctx, trunk_id, subports)"},{"line_number":59,"context_line":"                elif event_type \u003d\u003d events.DELETED:"},{"line_number":60,"context_line":"                    subport_ids \u003d [subport.port_id for subport in subports]"},{"line_number":61,"context_line":"                    self.ovsdb_handler.unwire_subports_for_trunk("},{"line_number":62,"context_line":"                        trunk_id, subport_ids)"},{"line_number":63,"context_line":"                else:"},{"line_number":64,"context_line":"                    LOG.error(_LE(\"Unknown event type %s\"), event_type)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_2c04e613","line":61,"in_reply_to":"7a8ec9b2_8cac25ef","updated":"2016-09-14 08:51:26.000000000","message":"No, I meant to wrap only wire_subports_for_trunk()","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6e672e19e3f19656e2c286e0325178fb259db5f9","unresolved":false,"context_lines":[{"line_number":62,"context_line":"                        trunk_id, subport_ids)"},{"line_number":63,"context_line":"                else:"},{"line_number":64,"context_line":"                    LOG.error(_LE(\"Unknown event type %s\"), event_type)"},{"line_number":65,"context_line":"            except oslo_messaging.MessagingException as e:"},{"line_number":66,"context_line":"                LOG.error(_LE(\"Got error from trunk rpc: %s\"), e)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_953bd4d0","line":65,"updated":"2016-09-12 14:50:40.000000000","message":"Are you saying that this is just for extra precaution, in case the underlying method does not catch this error? I thought we got it handled everywhere where we were doing RPC communication.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0c04b2d3a31adcc30b274cf6e8a8647eb7caaf6","unresolved":false,"context_lines":[{"line_number":62,"context_line":"                        trunk_id, subport_ids)"},{"line_number":63,"context_line":"                else:"},{"line_number":64,"context_line":"                    LOG.error(_LE(\"Unknown event type %s\"), event_type)"},{"line_number":65,"context_line":"            except oslo_messaging.MessagingException as e:"},{"line_number":66,"context_line":"                LOG.error(_LE(\"Got error from trunk rpc: %s\"), e)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_780e458d","line":65,"in_reply_to":"9a89bdaa_953bd4d0","updated":"2016-09-12 15:06:58.000000000","message":"We had it everywhere in previous patchsets. But it looked all like try: except, try: except ... so we moved it to higher levels, as there was no actual error handling other than simply logging the error and returning.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"94ce71ee437c797313b21a4536d8d5a9c8f4c927","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                    self.ovsdb_handler.wire_subports_for_trunk("},{"line_number":58,"context_line":"                        ctx, trunk_id, subports)"},{"line_number":59,"context_line":"                except oslo_messaging.MessagingException as e:"},{"line_number":60,"context_line":"                    LOG.error(_LE("},{"line_number":61,"context_line":"                        \"Got an error from trunk rpc when wiring subports \""},{"line_number":62,"context_line":"                        \"%(subports)s for trunk %(trunk_id)s: %(err)s\"),"},{"line_number":63,"context_line":"                        {\u0027subports\u0027: subports,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_78093a2c","line":60,"range":{"start_line":60,"start_character":20,"end_line":60,"end_character":34},"updated":"2016-09-14 12:14:16.000000000","message":"Is it possible, that operator can do anything with it?\nIf not, probably this should be just debug.","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c715fb74b0158a8d67264e2fee1afbf51b8a8d4a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                    self.ovsdb_handler.wire_subports_for_trunk("},{"line_number":58,"context_line":"                        ctx, trunk_id, subports)"},{"line_number":59,"context_line":"                except oslo_messaging.MessagingException as e:"},{"line_number":60,"context_line":"                    LOG.error(_LE("},{"line_number":61,"context_line":"                        \"Got an error from trunk rpc when wiring subports \""},{"line_number":62,"context_line":"                        \"%(subports)s for trunk %(trunk_id)s: %(err)s\"),"},{"line_number":63,"context_line":"                        {\u0027subports\u0027: subports,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_d3514f64","line":60,"range":{"start_line":60,"start_character":20,"end_line":60,"end_character":34},"in_reply_to":"7a8ec9b2_78093a2c","updated":"2016-09-14 12:27:30.000000000","message":"I think it should be error, something is wrong with RPC layer (rabbit down, full queues, no listener for given topic). You think it\u0027s not?","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbd0ab449526dde66eb332cdbe653dd5519f0b96","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                    self.ovsdb_handler.wire_subports_for_trunk("},{"line_number":58,"context_line":"                        ctx, trunk_id, subports)"},{"line_number":59,"context_line":"                except oslo_messaging.MessagingException as e:"},{"line_number":60,"context_line":"                    LOG.error(_LE("},{"line_number":61,"context_line":"                        \"Got an error from trunk rpc when wiring subports \""},{"line_number":62,"context_line":"                        \"%(subports)s for trunk %(trunk_id)s: %(err)s\"),"},{"line_number":63,"context_line":"                        {\u0027subports\u0027: subports,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_d809472e","line":60,"range":{"start_line":60,"start_character":20,"end_line":60,"end_character":34},"in_reply_to":"7a8ec9b2_d3514f64","updated":"2016-09-14 21:45:49.000000000","message":"I think error is appropriate according to logging guidelines:\n\nhttp://docs.openstack.org/developer/neutron/devref/effective_neutron.html#translation-and-logging","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"}],"neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_driver.py":[{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"b1ae632f063c0771ea517e15403f8b3229119e93","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_a73ee088","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"updated":"2016-09-12 19:44:53.000000000","message":"What are you testing here?","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"159694a050f771de7648f860b090104f138fc049","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_c2f8a67f","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"in_reply_to":"7a8ec9b2_4c813d4a","updated":"2016-09-14 08:51:26.000000000","message":"I\u0027ll fix the scope of try block","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"611313ae6fd9b291d61d84f6fa35db9e560785a3","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_65043c15","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"in_reply_to":"7a8ec9b2_d816f5a6","updated":"2016-09-14 08:58:04.000000000","message":"The validation is implicit, if exception won\u0027t be handled then handle_subports will fail. Thus we set that exception will be raised, call the method we are testing and we make sure we hit the point, where exception was raised.\n\nThis way we get triggered the error handling and make sure there are no mistakes in logging.\n\nAny suggestions how to test it better are welcome :)","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0998e114541c7e4c8f598cb25f377447b8aca6fe","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_4c813d4a","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"in_reply_to":"7a8ec9b2_d816f5a6","updated":"2016-09-14 00:09:38.000000000","message":"the remove even does not trigger any update_subport_bindings operation, so this is a bit implicit than I would like.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6958329430139db4d3dcc8361bbf8f16327179fc","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a89bdaa_ba4a6181","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"in_reply_to":"9a89bdaa_a73ee088","updated":"2016-09-13 07:02:29.000000000","message":"Validating we managed to call the method and thus exception was raised within the method.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"98b9217644e583fb3d08cb956f0ac0477f755608","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8ec9b2_d816f5a6","line":99,"range":{"start_line":99,"start_character":7,"end_line":99,"end_character":65},"in_reply_to":"9a89bdaa_ba4a6181","updated":"2016-09-13 19:29:36.000000000","message":"OK. I see, you\u0027re validating that method was called, but I don\u0027t see where\u0027s validation for Exception.","commit_id":"3c0184c1c03ee7f5ee01e6407b45b4387fcdd664"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbd0ab449526dde66eb332cdbe653dd5519f0b96","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        self.assertFalse(self.trunk_manager.wire_subports_for_trunk.called)"},{"line_number":92,"context_line":"        self.assertFalse(self.trunk_manager.unwire_subports_for_trunk.called)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def test_handle_subports_trunk_rpc_error(self):"},{"line_number":95,"context_line":"        trunk_rpc \u003d self.skeleton.ovsdb_handler.trunk_rpc"},{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_d820679e","line":94,"updated":"2016-09-14 21:45:49.000000000","message":"I\u0027d rename this test_handle_subports_created_trunk_rpc_error, but ok.","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"94ce71ee437c797313b21a4536d8d5a9c8f4c927","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_f8ad4a89","line":99,"range":{"start_line":99,"start_character":8,"end_line":99,"end_character":65},"updated":"2016-09-14 12:14:16.000000000","message":"One suggestion. Instead of verifying `update_subport_bindings` which will be called, and then Exception thrown, you can verify if `self.trunk_rpc.update_trunk_status` was not called.\nIn the case of Exception on `subport_bindings` flow will be interrupted. In case of RuntimeError, `update_trunk` will be called with DEGRADED_STATUS. And, if everything will be OK, then it\u0027ll be called at the end of `wiring` function.\n\n \u003e\u003e\u003e klass \u003d mock.Mock(spec\u003dKlass)\n \u003e\u003e\u003e klass.func.side_effect \u003d Exception\n \u003e\u003e\u003e klass.func()\n Traceback (most recent call last):\n   File \"\u003cinput\u003e\", line 1, in \u003cmodule\u003e\n     klass.func()\n   File \"/usr/local/lib/python2.7/dist-packages/mock/mock.py\", line 1062, in __call__\n     return _mock_self._mock_call(*args, **kwargs)\n   File \"/usr/local/lib/python2.7/dist-packages/mock/mock.py\", line 1118, in _mock_call\n     raise effect\n Exception\n \u003e\u003e\u003e klass.func.called\n True\n \u003e\u003e\u003e","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"},{"author":{"_account_id":14611,"name":"dasm","display_name":"dasm","email":"dsmigiel@redhat.com","username":"dasm"},"change_message_id":"c7ecaa3fcb4341afbb98d863a9e030db34b54624","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        trunk_rpc.update_subport_bindings.side_effect \u003d ("},{"line_number":97,"context_line":"            oslo_messaging.MessagingException)"},{"line_number":98,"context_line":"        self.skeleton.handle_subports(self.subports, events.CREATED)"},{"line_number":99,"context_line":"        self.assertTrue(trunk_rpc.update_subport_bindings.called)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8ec9b2_5091b298","line":99,"range":{"start_line":99,"start_character":8,"end_line":99,"end_character":65},"in_reply_to":"7a8ec9b2_f8ad4a89","updated":"2016-09-14 12:36:54.000000000","message":"OK, according to IRC discussion, it\u0027s better to leave this test, as is. Without depending on internal implementation of function.","commit_id":"9776d93f5849a02a6f4f13335d3f8ff97124fa8c"}]}