)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"18c1a7e5ee316fb9f49488c81d31d5fd5cb485b7","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        - https://review.openstack.org/#/c/227821/"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Change-Id: I81fdd2c971ee4ae5133126b6887ba6ad855ef138"},{"line_number":25,"context_line":"Closes-Bug: #1523780"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1a7b0d38_1266aeed","line":25,"range":{"start_line":25,"start_character":13,"end_line":25,"end_character":20},"updated":"2015-12-23 08:12:07.000000000","message":"Forgot to add Related-Bug: #1510757...","commit_id":"b1fe344f5ffdcec237f45d9c2055e59b7b1f6765"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8bd11544f9dd12142346e01858a82176433bc904","unresolved":false,"context_lines":[{"line_number":13,"context_line":"So this patch slightly catch and ignore some exceptions in race between"},{"line_number":14,"context_line":"HA router create and HA router delete."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Dependencies:"},{"line_number":17,"context_line":"    \"Add UniqueConstraint in L3HARouterAgentPortBinding\""},{"line_number":18,"context_line":"        - https://review.openstack.org/#/c/230481/"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1a7b0d38_1e5c754a","line":16,"updated":"2015-12-23 10:01:28.000000000","message":"I am not really sure that I understand this.What is the purpose the links below","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"9cf2225c1056a96ac72c6156f7daef877016b5ea","unresolved":false,"context_lines":[{"line_number":13,"context_line":"So this patch slightly catch and ignore some exceptions in race between"},{"line_number":14,"context_line":"HA router create and HA router delete."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Dependencies:"},{"line_number":17,"context_line":"    \"Add UniqueConstraint in L3HARouterAgentPortBinding\""},{"line_number":18,"context_line":"        - https://review.openstack.org/#/c/230481/"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1a7b0d38_090d213e","line":16,"in_reply_to":"1a7b0d38_1e5c754a","updated":"2015-12-23 10:40:47.000000000","message":"To reduce the race may relay on that.","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b6fc3861ccdabcbbb5f2cbca9ca40af2af81f9e8","unresolved":false,"context_lines":[{"line_number":12,"context_line":"then foreign key constraint error(Integrity Error) raised."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I81fdd2c971ee4ae5133126b6887ba6ad855ef138"},{"line_number":15,"context_line":"Closes-Bug: #1523780"},{"line_number":16,"context_line":"Related-Bug: #1510757"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"da6ed579_5794fce0","line":15,"range":{"start_line":15,"start_character":13,"end_line":15,"end_character":20},"updated":"2016-01-12 23:02:16.000000000","message":"I\u0027ve got a bunch of reviews that claim to close the same bug [1-7].  In general, we can only have one review close a bug.  If it needs other patches then it needs to depend on them and then wait for them all to get merged and then close the bug.\n\nAs it is now, the first one of these to merge will close the bug and then we\u0027ll be reviewing the rest on a bug marked closed.  Something needs to be done.  Here are some options.\n\n1.  You could line them all up properly with most of them marked with related-bug instead of closes-bug with the last one coming in a closing it.\n2.  You could keep them independent and file a bug for each independent fix.\n3.  Some mix of 1 and 2.\n4.  Tell me that they are all independent fixes for the same bug and which one is the best one to review (probably abandon most of the others).\n\n[1] https://review.openstack.org/#/c/265672/\n[2] https://review.openstack.org/#/c/265676/\n[3] https://review.openstack.org/#/c/265680/\n[4] https://review.openstack.org/#/c/265682/\n[5] https://review.openstack.org/#/c/265685/\n[6] https://review.openstack.org/#/c/266109/\n[7] https://review.openstack.org/#/c/260303/","commit_id":"45f265caece8b1a2234314b91d1ebf1f9bae2efe"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"b69e0fb7a660511be97fd8fcdd4af21570254675","unresolved":false,"context_lines":[{"line_number":12,"context_line":"then foreign key constraint error(Integrity Error) raised."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I81fdd2c971ee4ae5133126b6887ba6ad855ef138"},{"line_number":15,"context_line":"Closes-Bug: #1523780"},{"line_number":16,"context_line":"Related-Bug: #1510757"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"da6ed579_73ab4b1e","line":15,"range":{"start_line":15,"start_character":13,"end_line":15,"end_character":20},"in_reply_to":"da6ed579_46e1a8cf","updated":"2016-01-13 03:34:30.000000000","message":"New LP bug #1533460","commit_id":"45f265caece8b1a2234314b91d1ebf1f9bae2efe"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"bfc04a750fa6e5acb35053e68f92e7e686eaa164","unresolved":false,"context_lines":[{"line_number":12,"context_line":"then foreign key constraint error(Integrity Error) raised."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I81fdd2c971ee4ae5133126b6887ba6ad855ef138"},{"line_number":15,"context_line":"Closes-Bug: #1523780"},{"line_number":16,"context_line":"Related-Bug: #1510757"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"da6ed579_46e1a8cf","line":15,"range":{"start_line":15,"start_character":13,"end_line":15,"end_character":20},"in_reply_to":"da6ed579_5794fce0","updated":"2016-01-13 01:31:55.000000000","message":"Thank you, as Kevin Benton mentioned in Patch Set 10 https://review.openstack.org/#/c/260303/10:\n\"to elaborate, the commit message identifies 4 different race conditions. please fix one in each patch so it\u0027s clear what is being done\". Different people have different views, so now each patch deal with each one race.","commit_id":"45f265caece8b1a2234314b91d1ebf1f9bae2efe"}],"neutron/agent/l3/ha_router.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7578ea84da10d453c8aceda86b5af0ac9ba95f00","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def delete(self, agent):"},{"line_number":380,"context_line":"        super(HaRouter, self).delete(agent)"},{"line_number":381,"context_line":"        if self.keepalived_manager:"},{"line_number":382,"context_line":"            self.destroy_state_change_monitor(self.process_monitor)"},{"line_number":383,"context_line":"            if self.ha_port:"},{"line_number":384,"context_line":"                self.ha_network_removed()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_751a8119","line":381,"range":{"start_line":381,"start_character":16,"end_line":381,"end_character":34},"updated":"2015-12-23 05:41:06.000000000","message":"Checking keepalived_manager here may not achieve what you want. If keepalived_manager is null, the code will fail at line 380, and not hit this line. Code executing in line 380 will eventually hit line 184. And failed at there. \n\nI think you want to resolve the bug [1] here. I have a patch for it, but long time no review :(\n\n[1] https://bugs.launchpad.net/neutron/+bug/1510757","commit_id":"1d81812e6129b9400b3f43060e6d2882e62fb0ae"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"c3601d329fc784e7284e36d8293e221ad3985b49","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def delete(self, agent):"},{"line_number":380,"context_line":"        super(HaRouter, self).delete(agent)"},{"line_number":381,"context_line":"        if self.keepalived_manager:"},{"line_number":382,"context_line":"            self.destroy_state_change_monitor(self.process_monitor)"},{"line_number":383,"context_line":"            if self.ha_port:"},{"line_number":384,"context_line":"                self.ha_network_removed()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_759361fb","line":381,"range":{"start_line":381,"start_character":16,"end_line":381,"end_character":34},"in_reply_to":"1a7b0d38_751a8119","updated":"2015-12-23 06:08:32.000000000","message":"OK, thank you very much, I will add this keepalived_manager check at #L184 or somewhere else. LP bug #1510757 do not have any fix in the comments. I will add partial/related-bug: 1510757 at commit msg.","commit_id":"1d81812e6129b9400b3f43060e6d2882e62fb0ae"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"23c866e40f81db634cc71ae9dcb542b4511a1b4f","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def delete(self, agent):"},{"line_number":380,"context_line":"        super(HaRouter, self).delete(agent)"},{"line_number":381,"context_line":"        if self.keepalived_manager:"},{"line_number":382,"context_line":"            self.destroy_state_change_monitor(self.process_monitor)"},{"line_number":383,"context_line":"            if self.ha_port:"},{"line_number":384,"context_line":"                self.ha_network_removed()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_d03ee3b7","line":381,"range":{"start_line":381,"start_character":16,"end_line":381,"end_character":34},"in_reply_to":"1a7b0d38_759361fb","updated":"2015-12-23 06:19:01.000000000","message":"It is related to bug 1510757, but can\u0027t resolve all the problem that HA router without ha_port. Maybe you can at least resolve the issue of race between add/delete here.\n\nI would suggest you add some detail information about it in commit message, so that others will understand more about the change.","commit_id":"1d81812e6129b9400b3f43060e6d2882e62fb0ae"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"1b0c820f92277b7c281bc604fdcf3786b20d4c5e","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"    def delete(self, agent):"},{"line_number":380,"context_line":"        super(HaRouter, self).delete(agent)"},{"line_number":381,"context_line":"        if self.keepalived_manager:"},{"line_number":382,"context_line":"            self.destroy_state_change_monitor(self.process_monitor)"},{"line_number":383,"context_line":"            if self.ha_port:"},{"line_number":384,"context_line":"                self.ha_network_removed()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_8b310eef","line":381,"range":{"start_line":381,"start_character":16,"end_line":381,"end_character":34},"in_reply_to":"1a7b0d38_d03ee3b7","updated":"2015-12-23 07:00:19.000000000","message":"OK, done.","commit_id":"1d81812e6129b9400b3f43060e6d2882e62fb0ae"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8bd11544f9dd12142346e01858a82176433bc904","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        except AttributeError:"},{"line_number":133,"context_line":"            LOG.error(_LE(\u0027Unable to get router %s keepalived instance.\u0027),"},{"line_number":134,"context_line":"                      self.router_id)"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _get_primary_vip(self):"},{"line_number":138,"context_line":"        instance \u003d self._get_keepalived_instance()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_7e181985","line":135,"updated":"2015-12-23 10:01:28.000000000","message":"why the return here. sorry for the nit picking","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8bd11544f9dd12142346e01858a82176433bc904","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _get_primary_vip(self):"},{"line_number":138,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":139,"context_line":"        return instance.get_primary_vip()"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_697385b2","line":138,"updated":"2015-12-23 10:01:28.000000000","message":"this will cause an exception if None is returned","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"9cf2225c1056a96ac72c6156f7daef877016b5ea","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _get_primary_vip(self):"},{"line_number":138,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":139,"context_line":"        return instance.get_primary_vip()"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_c9fcf940","line":138,"in_reply_to":"1a7b0d38_697385b2","updated":"2015-12-23 10:40:47.000000000","message":"Actually I just want to make the code style here is consistent with other _get_keepalived_instance() calls, such as _remove_vip, _clear_vips ect. And maybe in a next patch, catch the instance is None in it.","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"9cf2225c1056a96ac72c6156f7daef877016b5ea","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        new_routes \u003d self.router[\u0027routes\u0027]"},{"line_number":194,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":195,"context_line":"        if not instance:"},{"line_number":196,"context_line":"            return"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        instance.virtual_routes.extra_routes \u003d ["}],"source_content_type":"text/x-python","patch_set":5,"id":"1a7b0d38_09b1a18f","line":195,"range":{"start_line":195,"start_character":15,"end_line":195,"end_character":23},"updated":"2015-12-23 10:40:47.000000000","message":"Some further work like this.","commit_id":"d4e21df57c5c26f98140d223ee66a8f5123d92ec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def initialize(self, process_monitor):"},{"line_number":77,"context_line":"        super(HaRouter, self).initialize(process_monitor)"},{"line_number":78,"context_line":"        ha_port \u003d self.router.get(n_consts.HA_INTERFACE_KEY)"},{"line_number":79,"context_line":"        if not ha_port:"},{"line_number":80,"context_line":"            LOG.error(_LE(\u0027Unable to process HA router %s without HA port\u0027),"},{"line_number":81,"context_line":"                      self.router_id)"},{"line_number":82,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_c1fedf6a","line":79,"range":{"start_line":79,"start_character":15,"end_line":79,"end_character":22},"updated":"2015-12-24 15:30:44.000000000","message":"Race may cause get HA router without ha interface attrs, so it\u0027s keepalived manager may stay None.","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def _get_keepalived_instance(self):"},{"line_number":130,"context_line":"        try:"},{"line_number":131,"context_line":"            return self.keepalived_manager.config.get_instance(self.ha_vr_id)"},{"line_number":132,"context_line":"        except AttributeError:"},{"line_number":133,"context_line":"            LOG.error(_LE(\u0027Unable to get router %s keepalived instance.\u0027),"},{"line_number":134,"context_line":"                      self.router_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_012ce700","line":131,"range":{"start_line":131,"start_character":19,"end_line":131,"end_character":23},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":187,"context_line":"    def routes_updated(self):"},{"line_number":188,"context_line":"        new_routes \u003d self.router[\u0027routes\u0027]"},{"line_number":189,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":190,"context_line":"        if not instance:"},{"line_number":191,"context_line":"            LOG.warning(_LW(\u0027Failed to update HA router %s routes because \u0027"},{"line_number":192,"context_line":"                            \u0027HA router keepalived instance is None.\u0027),"},{"line_number":193,"context_line":"                        self.router_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_212fa3f7","line":190,"range":{"start_line":190,"start_character":15,"end_line":190,"end_character":23},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    def delete(self, agent):"},{"line_number":391,"context_line":"        super(HaRouter, self).delete(agent)"},{"line_number":392,"context_line":"        if self.keepalived_manager:"},{"line_number":393,"context_line":"            self.destroy_state_change_monitor(self.process_monitor)"},{"line_number":394,"context_line":"            if self.ha_port:"},{"line_number":395,"context_line":"                self.ha_network_removed()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_a13b9337","line":392,"range":{"start_line":392,"start_character":11,"end_line":392,"end_character":15},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d511a691533fc15913f3149e151d54f5909dc65c","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_ec2c264c","line":139,"updated":"2016-01-04 09:17:25.000000000","message":"why do you need this change?","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"be0f2295dbbdd20e97003be160c4b8a554842143","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_6c5fd641","line":139,"in_reply_to":"fa69d971_ec2c264c","updated":"2016-01-04 09:34:11.000000000","message":"Thank you for reviewing this, some info about this I\u0027ve replied you in the patch set 5 : https://review.openstack.org/#/c/260303/5/neutron/agent/l3/ha_router.py.","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        except AttributeError:"},{"line_number":134,"context_line":"            LOG.error(_LE(\u0027Unable to get router %s keepalived instance.\u0027),"},{"line_number":135,"context_line":"                      self.router_id)"},{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_8b9d22be","line":136,"range":{"start_line":136,"start_character":12,"end_line":136,"end_character":18},"updated":"2016-01-05 07:00:46.000000000","message":"This return is not needed.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"280dff4542ab76b1fd1c452bba54988270571bbe","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        except AttributeError:"},{"line_number":134,"context_line":"            LOG.error(_LE(\u0027Unable to get router %s keepalived instance.\u0027),"},{"line_number":135,"context_line":"                      self.router_id)"},{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_683f9187","line":136,"range":{"start_line":136,"start_character":12,"end_line":136,"end_character":18},"in_reply_to":"fa69d971_8b9d22be","updated":"2016-01-05 08:07:13.000000000","message":"Done","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b5a2a9a1778b809e3f65c1b8964c1e15a8338b7f","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_8bf67ff7","line":139,"updated":"2016-01-05 08:07:21.000000000","message":"+1. unrelated change","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_0b4e123b","line":139,"updated":"2016-01-05 07:00:46.000000000","message":"What is the benefit of splitting it to 2 lines?","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"280dff4542ab76b1fd1c452bba54988270571bbe","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_08edb5ee","line":139,"in_reply_to":"fa69d971_0b4e123b","updated":"2016-01-05 08:07:13.000000000","message":"To be consistent with other functions like, _add_vip, _remove_vip etc., so maybe in a next patch we could try to check the keepalived  instance value to avoid some unexpected exceptions. OK, since you and garyk consider that this is not necessary, I will remove this change in next patch.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"bc3b77bde0a19d654f145c1da841d5ccddf85515","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    def _get_primary_vip(self):"},{"line_number":139,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":140,"context_line":"        return instance.get_primary_vip()"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def get_ha_device_name(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_4e4bb52a","line":139,"in_reply_to":"fa69d971_8bf67ff7","updated":"2016-01-05 08:42:44.000000000","message":"Done","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"}],"neutron/db/common_db_mixin.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"ecf5c7ed5436b9eda6a988879d45a0a90f177a74","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        an object. It is called with object\u0027s id field as an argument."},{"line_number":52,"context_line":"    \u0027\u0027\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def _delete_obj(obj_id):"},{"line_number":55,"context_line":"        try:"},{"line_number":56,"context_line":"            delete_fn(obj_id)"},{"line_number":57,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":21,"id":"bae84128_add99c85","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":19},"updated":"2016-02-22 16:45:05.000000000","message":"Please rename to _safe_delete_fn","commit_id":"32b4953e635f533e77c10bf3bf4d798589e8074f"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"400bf4953d87106dcae57d97d5c292725f6d363b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        an object. It is called with object\u0027s id field as an argument."},{"line_number":52,"context_line":"    \u0027\u0027\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def _delete_obj(obj_id):"},{"line_number":55,"context_line":"        try:"},{"line_number":56,"context_line":"            delete_fn(obj_id)"},{"line_number":57,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":21,"id":"bae84128_412d2594","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":19},"in_reply_to":"bae84128_add99c85","updated":"2016-02-23 01:42:29.000000000","message":"Done","commit_id":"32b4953e635f533e77c10bf3bf4d798589e8074f"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"13f0c01eb97509cefa1d7fc93bd365ddb420311b","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":72,"context_line":"        obj \u003d create_fn()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        if obj is None:"},{"line_number":75,"context_line":"            # Failed to create object"},{"line_number":76,"context_line":"            return None, None"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_90119eaf","line":74,"updated":"2016-02-26 07:58:12.000000000","message":"@Kevin,  \"The changes to safe_creation and returning None are not necessary.\", let\u0027s discuss about that here.\nFor now, the safe_creation could not handle any return None creations. If create_fn return a None, the safe_creation will no longer safe, so much exceptions raised.","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"af4dfde9fbeb7cd6f356e296de99070831d8bde5","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":72,"context_line":"        obj \u003d create_fn()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        if obj is None:"},{"line_number":75,"context_line":"            # Failed to create object"},{"line_number":76,"context_line":"            return None, None"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_988260ff","line":74,"in_reply_to":"9aed3d3a_7d5aee84","updated":"2016-02-26 09:37:30.000000000","message":"OK. Then in order to speed this patch (remember we need to get this merged by M-3 next week), I propose you move this change to the patch you referenced to. This will (on top of letting us get this patch in much faster) put the actual change in context and make it easier to review :)","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2fae5175b897e8490226886d5b7ecba8df5c9e89","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":72,"context_line":"        obj \u003d create_fn()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        if obj is None:"},{"line_number":75,"context_line":"            # Failed to create object"},{"line_number":76,"context_line":"            return None, None"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_9a6a5819","line":74,"in_reply_to":"9aed3d3a_90119eaf","updated":"2016-02-26 09:03:36.000000000","message":"What\u0027s wrong with using exceptions to tell something went wrong?","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"8f9e6c529a4cb6fe7a5cfb98084984b4ad8a0ffa","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":72,"context_line":"        obj \u003d create_fn()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        if obj is None:"},{"line_number":75,"context_line":"            # Failed to create object"},{"line_number":76,"context_line":"            return None, None"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_7d5aee84","line":74,"in_reply_to":"9aed3d3a_9a6a5819","updated":"2016-02-26 09:22:35.000000000","message":"Actually this change is for a future use just like [1], there already realized that it has a race condition between last HA router deletion and a new HA router creation API call. So after catch known exceptions a None value will be return. Here just handle that scenario.\nAnd I just added some comments about the create_fn and create_bindings return value considerations.\n\n\n[1]: https://review.openstack.org/#/c/265682/12/neutron/db/l3_hamode_db.py#L340","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"}],"neutron/db/l3_hamode_db.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        return num_agents"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    def _create_ha_port_binding(self, context, port_id, router_id):"},{"line_number":313,"context_line":"        try:"},{"line_number":314,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":315,"context_line":"                portbinding \u003d L3HARouterAgentPortBinding(port_id\u003dport_id,"},{"line_number":316,"context_line":"                                                         router_id\u003drouter_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_e1443b1c","line":313,"range":{"start_line":313,"start_character":11,"end_line":313,"end_character":12},"updated":"2015-12-24 15:30:44.000000000","message":"Race may cause the router_id foreign key constraint error, but no harm.","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"f5c24d8deb338a05b2706673d2758eefe4441994","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                \u0027device_id\u0027: router_id,"},{"line_number":329,"context_line":"                \u0027device_owner\u0027: constants.DEVICE_OWNER_ROUTER_HA_INTF,"},{"line_number":330,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":331,"context_line":"        port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":332,"context_line":"                                 {\u0027port\u0027: args})"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_755a4e52","line":331,"range":{"start_line":331,"start_character":23,"end_line":331,"end_character":34},"updated":"2015-12-24 11:03:41.000000000","message":"catch (n_exc.NetworkNotFound, n_exc.IpAddressGenerationFailure)","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                \u0027device_id\u0027: router_id,"},{"line_number":329,"context_line":"                \u0027device_owner\u0027: constants.DEVICE_OWNER_ROUTER_HA_INTF,"},{"line_number":330,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":331,"context_line":"        port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":332,"context_line":"                                 {\u0027port\u0027: args})"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_41c98f77","line":331,"range":{"start_line":331,"start_character":23,"end_line":331,"end_character":34},"in_reply_to":"1a7b0d38_755a4e52","updated":"2015-12-24 15:30:44.000000000","message":"Race may also happened here, because the last router was delete, and HA network was removed too, the auto_scheduler (create_ha_port_and_bind) may got errors here.","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":578,"context_line":"                                                    host)"},{"line_number":579,"context_line":"        for binding in bindings:"},{"line_number":580,"context_line":"            router \u003d routers_dict.get(binding.router_id)"},{"line_number":581,"context_line":"            if binding.port:"},{"line_number":582,"context_line":"                port_dict \u003d self._core_plugin._make_port_dict(binding.port)"},{"line_number":583,"context_line":"                router[constants.HA_INTERFACE_KEY] \u003d port_dict"},{"line_number":584,"context_line":"            if binding.state:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_61728b4b","line":581,"range":{"start_line":581,"start_character":0,"end_line":581,"end_character":28},"updated":"2015-12-24 15:30:44.000000000","message":"Race happend here because the deleting of a router\u0027s HA interface may also remove the ha port bindings.","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d511a691533fc15913f3149e151d54f5909dc65c","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                context.session.add(portbinding)"},{"line_number":318,"context_line":"                return portbinding"},{"line_number":319,"context_line":"        except db_exc.DBReferenceError:"},{"line_number":320,"context_line":"            LOG.warning(_LW(\u0027Failed to create HA router agent PortBinding,\u0027"},{"line_number":321,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":322,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_4c1692f2","line":320,"updated":"2016-01-04 09:17:25.000000000","message":"missing space at the end of the line","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"be0f2295dbbdd20e97003be160c4b8a554842143","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                context.session.add(portbinding)"},{"line_number":318,"context_line":"                return portbinding"},{"line_number":319,"context_line":"        except db_exc.DBReferenceError:"},{"line_number":320,"context_line":"            LOG.warning(_LW(\u0027Failed to create HA router agent PortBinding,\u0027"},{"line_number":321,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":322,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_8c080a2f","line":320,"in_reply_to":"fa69d971_4c1692f2","updated":"2016-01-04 09:34:11.000000000","message":"Done","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d511a691533fc15913f3149e151d54f5909dc65c","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":334,"context_line":"                                       {\u0027port\u0027: args})"},{"line_number":335,"context_line":"        except n_exc.NetworkNotFound:"},{"line_number":336,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network\""},{"line_number":337,"context_line":"                            \" was deleted concurrently.\"))"},{"line_number":338,"context_line":"            return"},{"line_number":339,"context_line":"        except n_exc.IpAddressGenerationFailure:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_acab8e88","line":336,"updated":"2016-01-04 09:17:25.000000000","message":"why the space at the beginning of the sentence. please put at the end","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"be0f2295dbbdd20e97003be160c4b8a554842143","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":334,"context_line":"                                       {\u0027port\u0027: args})"},{"line_number":335,"context_line":"        except n_exc.NetworkNotFound:"},{"line_number":336,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network\""},{"line_number":337,"context_line":"                            \" was deleted concurrently.\"))"},{"line_number":338,"context_line":"            return"},{"line_number":339,"context_line":"        except n_exc.IpAddressGenerationFailure:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_ac12ae60","line":336,"in_reply_to":"fa69d971_acab8e88","updated":"2016-01-04 09:34:11.000000000","message":"Done","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"d511a691533fc15913f3149e151d54f5909dc65c","unresolved":false,"context_lines":[{"line_number":343,"context_line":"            return"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"        try:"},{"line_number":346,"context_line":"            portbonding \u003d self._create_ha_port_binding(context,"},{"line_number":347,"context_line":"                                                       port[\u0027id\u0027],"},{"line_number":348,"context_line":"                                                       router_id)"},{"line_number":349,"context_line":"            if not portbonding:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_ccae0298","line":346,"range":{"start_line":346,"start_character":12,"end_line":346,"end_character":23},"updated":"2016-01-04 09:17:25.000000000","message":"please change this to portbinding","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"be0f2295dbbdd20e97003be160c4b8a554842143","unresolved":false,"context_lines":[{"line_number":343,"context_line":"            return"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"        try:"},{"line_number":346,"context_line":"            portbonding \u003d self._create_ha_port_binding(context,"},{"line_number":347,"context_line":"                                                       port[\u0027id\u0027],"},{"line_number":348,"context_line":"                                                       router_id)"},{"line_number":349,"context_line":"            if not portbonding:"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_6c0d963d","line":346,"range":{"start_line":346,"start_character":12,"end_line":346,"end_character":23},"in_reply_to":"fa69d971_ccae0298","updated":"2016-01-04 09:34:11.000000000","message":"Done","commit_id":"08bbeae95a13b88fa678bfd592d0fc481f828924"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network \""},{"line_number":337,"context_line":"                            \"was deleted concurrently.\"))"},{"line_number":338,"context_line":"            return"},{"line_number":339,"context_line":"        except n_exc.IpAddressGenerationFailure:"},{"line_number":340,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network \""},{"line_number":341,"context_line":"                            \"has no subnet, or the subnet of HA network \""},{"line_number":342,"context_line":"                            \"was deleted concurrently.\"))"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_6bd46ebc","line":339,"updated":"2016-01-05 07:00:46.000000000","message":"These exceptions seem coming from your experiment. But what if there are other exceptions(in future)? I think as long as there is exception when creating port, it should be logged. So, to keep code simple, just catch Exception might be better.\n\nAnd it actually fails the \"add_ha_port\", consider using LOG.error.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"280dff4542ab76b1fd1c452bba54988270571bbe","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network \""},{"line_number":337,"context_line":"                            \"was deleted concurrently.\"))"},{"line_number":338,"context_line":"            return"},{"line_number":339,"context_line":"        except n_exc.IpAddressGenerationFailure:"},{"line_number":340,"context_line":"            LOG.warning(_LW(\"Failed to create HA port because HA network \""},{"line_number":341,"context_line":"                            \"has no subnet, or the subnet of HA network \""},{"line_number":342,"context_line":"                            \"was deleted concurrently.\"))"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_6880f1f7","line":339,"in_reply_to":"fa69d971_6bd46ebc","updated":"2016-01-05 08:07:13.000000000","message":"NetworkNotFound and IpAddressGenerationFailure are caused by the race between tenant last HA router delete and create. The exception do not comes a lot, for now, and no other exceptions raised. What about using excutils to LOG error about these two exceptions, and raising others?","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            portbinding \u003d self._create_ha_port_binding(context,"},{"line_number":347,"context_line":"                                                       port[\u0027id\u0027],"},{"line_number":348,"context_line":"                                                       router_id)"},{"line_number":349,"context_line":"            if not portbinding:"},{"line_number":350,"context_line":"                self._core_plugin.delete_port(context, port[\u0027id\u0027],"},{"line_number":351,"context_line":"                                              l3_port_check\u003dFalse)"},{"line_number":352,"context_line":"                return"},{"line_number":353,"context_line":"            return portbinding"},{"line_number":354,"context_line":"        except Exception:"},{"line_number":355,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_cba8fa22","line":352,"range":{"start_line":349,"start_character":4,"end_line":352,"end_character":22},"updated":"2016-01-05 07:00:46.000000000","message":"Why not move the the exception handler in line 320~322 to here and do the port clean in the same exception handler.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"280dff4542ab76b1fd1c452bba54988270571bbe","unresolved":false,"context_lines":[{"line_number":352,"context_line":"                return"},{"line_number":353,"context_line":"            return portbinding"},{"line_number":354,"context_line":"        except Exception:"},{"line_number":355,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":356,"context_line":"                self._core_plugin.delete_port(context, port[\u0027id\u0027],"},{"line_number":357,"context_line":"                                              l3_port_check\u003dFalse)"},{"line_number":358,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_287af9bd","line":355,"range":{"start_line":355,"start_character":26,"end_line":355,"end_character":52},"updated":"2016-01-05 08:07:13.000000000","message":"What about remove the try..catch here? And move this save_and_reraise_exception to L319-322, and add raise condition with exception type not equal to  IntegrityError?","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"cdd478221f4f41912a7b404e5ee31ebde57bd051","unresolved":false,"context_lines":[{"line_number":345,"context_line":"        except Exception as e:"},{"line_number":346,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":347,"context_line":"                if isinstance(e, db_exc.DBError):"},{"line_number":348,"context_line":"                    if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":349,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":350,"context_line":"                        LOG.warning("},{"line_number":351,"context_line":"                            _LW(\u0027Failed to create HA router agent PortBinding,\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"da6ed579_dd0fa62c","line":348,"range":{"start_line":348,"start_character":36,"end_line":348,"end_character":51},"updated":"2016-01-13 02:19:50.000000000","message":"I will remove this to _create_ha_port_binding in order to avoid the conflicts with Ann Kamyshnikova\u0027s patch https://review.openstack.org/#/c/230481","commit_id":"45f265caece8b1a2234314b91d1ebf1f9bae2efe"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9fe61ba6b41194c408967d400a69976b932b625d","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                                     l3_port_check\u003dFalse)"},{"line_number":355,"context_line":"        port, bindings \u003d common_db_mixin.safe_creation(context, creation,"},{"line_number":356,"context_line":"                                                       deletion, content)"},{"line_number":357,"context_line":"        if not bindings and port:"},{"line_number":358,"context_line":"            # Handle the IntegrityError may get empty bindings"},{"line_number":359,"context_line":"            # so remove the stale port."},{"line_number":360,"context_line":"            self._core_plugin.delete_port(context, port[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a68dd71_feda2016","line":357,"updated":"2016-01-25 07:53:00.000000000","message":"Why not add this in safe_creation? Previously, there was not an option that bindings will be None, so I do not check this there, but now I think this logic will be related.","commit_id":"13d7ee5ab7d3ae30269f7704bcd8146b25339517"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7b23f828a0c99d896ffa04a3149876f6bc57f58a","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                                     l3_port_check\u003dFalse)"},{"line_number":355,"context_line":"        port, bindings \u003d common_db_mixin.safe_creation(context, creation,"},{"line_number":356,"context_line":"                                                       deletion, content)"},{"line_number":357,"context_line":"        if not bindings and port:"},{"line_number":358,"context_line":"            # Handle the IntegrityError may get empty bindings"},{"line_number":359,"context_line":"            # so remove the stale port."},{"line_number":360,"context_line":"            self._core_plugin.delete_port(context, port[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":18,"id":"9a68dd71_41583188","line":357,"in_reply_to":"9a68dd71_feda2016","updated":"2016-01-25 08:15:48.000000000","message":"OK, done.","commit_id":"13d7ee5ab7d3ae30269f7704bcd8146b25339517"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"ecf5c7ed5436b9eda6a988879d45a0a90f177a74","unresolved":false,"context_lines":[{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding,\u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":336,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"},{"line_number":337,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"bae84128_205ab44f","line":334,"range":{"start_line":334,"start_character":73,"end_line":334,"end_character":75},"updated":"2016-02-22 16:45:05.000000000","message":"there should be a space after this word.","commit_id":"32b4953e635f533e77c10bf3bf4d798589e8074f"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"400bf4953d87106dcae57d97d5c292725f6d363b","unresolved":false,"context_lines":[{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding,\u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":336,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"},{"line_number":337,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"bae84128_c13935d9","line":334,"range":{"start_line":334,"start_character":73,"end_line":334,"end_character":75},"in_reply_to":"bae84128_205ab44f","updated":"2016-02-23 01:42:29.000000000","message":"Done","commit_id":"32b4953e635f533e77c10bf3bf4d798589e8074f"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b93c76d23f68153264234fdfac95a880b2ef4a6a","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        except db_exc.DBReferenceError as e:"},{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_c4c7f043","line":332,"updated":"2016-02-26 06:16:55.000000000","message":"It\u0027s completely pointless to continue the process of ha port creation once this error has been encountered. The changes to safe_creation and returning None are not necessary. Just raise a RouterNotFound exception right here and catch that in the create_ha_port_and_bind function in the scheduler.","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"13f0c01eb97509cefa1d7fc93bd365ddb420311b","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        except db_exc.DBReferenceError as e:"},{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_9517d050","line":332,"in_reply_to":"9aed3d3a_0afa7f42","updated":"2016-02-26 07:58:12.000000000","message":"If router is not existed, retry is no use. And in your perspective if here raised a RouterNoutFound Exception, then the patch(282876) you paste[1] will catch all kinds of Exceptions then go retry creation next time, because create_ha_port_and_bind() is ONLY placed in l3_agent_scheduler.py, it will not break the retry in [1].\n\nThen for my next patch, if test OK, I will let here raise the RouterNotFound, and only catch it in l3_agent_scheduler.py create_ha_port_and_bind().\n\n[1]: https://review.openstack.org/#/c/282876/","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3b66384f042742405efb93df6c37ecdb88fc13ed","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        except db_exc.DBReferenceError as e:"},{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_0afa7f42","line":332,"in_reply_to":"9aed3d3a_6f26bdbb","updated":"2016-02-26 07:23:46.000000000","message":"I\u0027m all for throwing an exception here, but catching it in create_ha_port_and_bind() is not a good idea IMO as it goes against [1]. In short, catching the exception will disable the retry mechanism [1] implements and will make test_create_and_delete_router rally test fail nondeterministicly again.\n\nFor this reason I think that this patch should be rebased on [1]. @Kevin, this means that would [2] have not been abandoned, I would\u0027ve asked that it would have been rebased on [1] as well (and I was mistaken saying otherwise yesterday on IRC).\n\n[1]: https://review.openstack.org/#/c/282876/14/neutron/db/l3_hamode_db.py\n[2]: https://review.openstack.org/#/c/283031/","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2fae5175b897e8490226886d5b7ecba8df5c9e89","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        except db_exc.DBReferenceError as e:"},{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_a76aa918","line":332,"in_reply_to":"9aed3d3a_9517d050","updated":"2016-02-26 09:03:36.000000000","message":"Liu, you\u0027re correct and my previous comment should be ignored.","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"c4c073c08d34840d3a20a91456237bdafaf06948","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        except db_exc.DBReferenceError as e:"},{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_6f26bdbb","line":332,"in_reply_to":"9aed3d3a_c4c7f043","updated":"2016-02-26 06:43:54.000000000","message":"Good idea, let me test that RouterNotFound exception raised here to see if it will break the safe_creation transaction.","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b93c76d23f68153264234fdfac95a880b2ef4a6a","unresolved":false,"context_lines":[{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":336,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_24c5fc3b","line":333,"updated":"2016-02-26 06:16:55.000000000","message":"this is just debug information. We don\u0027t print warnings when a tenant deletes their router.","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"c4c073c08d34840d3a20a91456237bdafaf06948","unresolved":false,"context_lines":[{"line_number":330,"context_line":"            with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":331,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":332,"context_line":"                    ctxt.reraise \u003d False"},{"line_number":333,"context_line":"                    LOG.warning("},{"line_number":334,"context_line":"                        _LW(\u0027Failed to create HA router agent PortBinding, \u0027"},{"line_number":335,"context_line":"                            \u0027Router %s has already been removed \u0027"},{"line_number":336,"context_line":"                            \u0027by concurrent operation\u0027), router_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_4f2bf982","line":333,"in_reply_to":"9aed3d3a_24c5fc3b","updated":"2016-02-26 06:43:54.000000000","message":"Done","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"}],"neutron/plugins/ml2/rpc.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":74,"context_line":"        port_id \u003d plugin._device_to_port_id(rpc_context, device)"},{"line_number":75,"context_line":"        port_context \u003d plugin.get_bound_port_context(rpc_context,"},{"line_number":76,"context_line":"                                                     port_id,"},{"line_number":77,"context_line":"                                                     host,"},{"line_number":78,"context_line":"                                                     cached_networks)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_01de2711","line":75,"range":{"start_line":75,"start_character":23,"end_line":75,"end_character":29},"updated":"2015-12-24 15:30:44.000000000","message":"Race happened here because of the deleting of a router\u0027s HA port.","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                          else n_const.PORT_STATUS_DOWN)"},{"line_number":106,"context_line":"            if port[\u0027status\u0027] !\u003d new_status:"},{"line_number":107,"context_line":"                try:"},{"line_number":108,"context_line":"                    plugin.update_port_status(rpc_context,"},{"line_number":109,"context_line":"                                              port_id,"},{"line_number":110,"context_line":"                                              new_status,"},{"line_number":111,"context_line":"                                              host,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_21e1e3d0","line":108,"range":{"start_line":108,"start_character":27,"end_line":108,"end_character":45},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                    \u0027exists\u0027: port_exists}"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        try:"},{"line_number":199,"context_line":"            port_exists \u003d bool(plugin.update_port_status("},{"line_number":200,"context_line":"                rpc_context, port_id, n_const.PORT_STATUS_DOWN, host))"},{"line_number":201,"context_line":"        except exc.StaleDataError:"},{"line_number":202,"context_line":"            port_exists \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_c1e7bfe2","line":199,"range":{"start_line":199,"start_character":37,"end_line":199,"end_character":38},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            return"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            port_id \u003d plugin.update_port_status(rpc_context, port_id,"},{"line_number":235,"context_line":"                                                n_const.PORT_STATUS_ACTIVE,"},{"line_number":236,"context_line":"                                                host)"},{"line_number":237,"context_line":"        except os_db_exception.DBError:"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_e1eafbab","line":234,"range":{"start_line":234,"start_character":0,"end_line":234,"end_character":69},"updated":"2015-12-24 15:30:44.000000000","message":"ditto","commit_id":"bf311f26cb539421a93f39dedaec37aee89f0240"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                                              port_id,"},{"line_number":115,"context_line":"                                              new_status,"},{"line_number":116,"context_line":"                                              host,"},{"line_number":117,"context_line":"                                              port_context.network.current)"},{"line_number":118,"context_line":"                except os_db_exception.DBError:"},{"line_number":119,"context_line":"                    LOG.warning("},{"line_number":120,"context_line":"                        _LW(\"Failed to update port %(port_id)s with status \""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_86019707","line":117,"updated":"2016-01-05 07:00:46.000000000","message":"It looks like you are handling the same exception for the same method, why not just handling it in the plugin, so that duplicating code can be avoided.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"280dff4542ab76b1fd1c452bba54988270571bbe","unresolved":false,"context_lines":[{"line_number":114,"context_line":"                                              port_id,"},{"line_number":115,"context_line":"                                              new_status,"},{"line_number":116,"context_line":"                                              host,"},{"line_number":117,"context_line":"                                              port_context.network.current)"},{"line_number":118,"context_line":"                except os_db_exception.DBError:"},{"line_number":119,"context_line":"                    LOG.warning("},{"line_number":120,"context_line":"                        _LW(\"Failed to update port %(port_id)s with status \""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_e804e118","line":117,"in_reply_to":"fa69d971_86019707","updated":"2016-01-05 08:07:13.000000000","message":"The function has db retry mechanism, let it retry enough times, and just let it failed happy.","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            port_exists \u003d False"},{"line_number":208,"context_line":"            LOG.debug(\"delete_port and update_device_down are being executed \""},{"line_number":209,"context_line":"                      \"concurrently. Ignoring StaleDataError.\")"},{"line_number":210,"context_line":"        except os_db_exception.DBError:"},{"line_number":211,"context_line":"            port_exists \u003d False"},{"line_number":212,"context_line":"            LOG.warning("},{"line_number":213,"context_line":"                _LW(\"Failed to update port %(port_id)s with status \""}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_8628b787","line":210,"updated":"2016-01-05 07:00:46.000000000","message":"ditto","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86862ade4176b1dafb99f506d7c2a2f32e64f101","unresolved":false,"context_lines":[{"line_number":239,"context_line":"            port_id \u003d plugin.update_port_status(rpc_context, port_id,"},{"line_number":240,"context_line":"                                                n_const.PORT_STATUS_ACTIVE,"},{"line_number":241,"context_line":"                                                host)"},{"line_number":242,"context_line":"        except os_db_exception.DBError:"},{"line_number":243,"context_line":"            LOG.warning("},{"line_number":244,"context_line":"                _LW(\"Failed to update port %(port_id)s with status \""},{"line_number":245,"context_line":"                    \"%(status)s on host %(host)s.\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"fa69d971_461e3f60","line":242,"updated":"2016-01-05 07:00:46.000000000","message":"ditto","commit_id":"17a25c3d80cfa4b4a9c9b7c31537fe262f7acbcf"}],"neutron/scheduler/l3_agent_scheduler.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"aceed1eafa9efef46379e7df0cbe41665d88ffc8","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a7e1126_29637643","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"updated":"2015-12-22 06:26:09.000000000","message":"Transaction here was added here to avoid having extra L3HARouterAgentPortBinding with l3_agent as\nNone, also as duplications L3HARouterAgentPortBinding, that can appeared even in normal usage neutron with L3 HA. Operation of setting l3_agent should be atomic.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"1b0c820f92277b7c281bc604fdcf3786b20d4c5e","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_70060f22","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_30dde714","updated":"2015-12-23 07:00:19.000000000","message":"Hmm, yes, what about also add port_id to unique_constraint too?","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"efef65433c0ebfd461420514ac0f2fe43db131d4","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_cbe4f60e","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_70060f22","updated":"2015-12-23 07:13:55.000000000","message":"port_id is unique itself without any constraints. I\u0027m talking about situation like this http://paste.openstack.org/show/482572/ It is described in the \nhttps://bugs.launchpad.net/neutron/+bug/1499647","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"90ce6f95f9b4c2ae14d432e7a5a0e8a6691d22e7","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_f2ce8282","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_afadf589","updated":"2015-12-23 08:19:11.000000000","message":"I see that we are not getting understanding here. max_l3_agents_per_router and min_l3_agents_per_router was set correctly for 3 and 2. This is not about this. When I was working on this issues I get the result that for stabilization of work we need adding transaction here, do changes that are done in \"Make object creation methods in l3_hamode_db atomic\" and add unique constraint. This three things. Without each other this won\u0027t make any sense.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"fc761de2893dc4cfcbd8ffa425bdefab6926e90e","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_afadf589","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_cbe4f60e","updated":"2015-12-23 07:57:55.000000000","message":"I don\u0027t know the network node number of the the environment you paste in LP bug #1499647, and what about the settings about the max_l3_agents_per_router and min_l3_agents_per_router?\nIf the patch you submit \"\nMake object creation methods in l3_hamode_db atomic\" can ensure that there is no chance to create more than the number of max_l3_agents_per_router HA interface for a HA router. Then there is no chance to insert a total same record like: port_id_aaa, router_id_bbb, \u003cnull\u003e.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3e49034e425c5c12cbae7ea289616abad9e1452c","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_30dde714","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_cc6c190b","updated":"2015-12-23 06:42:12.000000000","message":"adding unique constraint won\u0027t make operation of setting bindings atomic, because l3_agent as None is a possible with unique constraint, so reverting this you return this problem.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"832e0b9b12043f8a6b583216eccc90130349a505","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_64b96ce9","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"1a7b0d38_f2ce8282","updated":"2015-12-23 10:40:56.000000000","message":"Because the race of router delete api call will delete original HA interfaces and portbindings, if the auto_scheduler is true, then here recreate some. So here may only relay on the \"add unique constraint\" patch. Transaction here is not needed because new HA port will not have a same in DB, even l3_agent is None.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6d605e993244c58a611756b3fa36e50036bdadf7","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a7b0d38_cc6c190b","side":"PARENT","line":295,"range":{"start_line":295,"start_character":8,"end_line":295,"end_character":56},"in_reply_to":"3a7e1126_29637643","updated":"2015-12-23 00:15:22.000000000","message":"This patch may relay on the patch you submit \"Add UniqueConstraint in L3HARouterAgentPortBinding\".","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"00558cf4c963d9a56081f8131209704c44f71e46","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a7b0d38_c1a57f04","side":"PARENT","line":295,"range":{"start_line":295,"start_character":35,"end_line":295,"end_character":50},"updated":"2015-12-24 15:30:44.000000000","message":"If everything was processed OK in sub-function calls, session begin here may not need anymore. Maybe the revert can be separated as a new patch.","commit_id":"aa285ff82c781a69c4b2172a7f7b83bcf30d7fc0"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7d9b6fb09b27cfa94a448677773044afd924f5bf","unresolved":false,"context_lines":[{"line_number":292,"context_line":"                                tenant_id, agent):"},{"line_number":293,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":294,"context_line":"        ha_network \u003d plugin.get_ha_network(context, tenant_id)"},{"line_number":295,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":296,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":297,"context_line":"                                              ha_network.network.id, tenant_id)"},{"line_number":298,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"da6ed579_5590afec","side":"PARENT","line":295,"range":{"start_line":295,"start_character":55,"end_line":295,"end_character":57},"updated":"2016-01-12 03:30:48.000000000","message":"Maybe this revert can be more clearly solved by this https://review.openstack.org/#/c/261238/","commit_id":"89f96e627fc1e402ca456d60127ccde5bd534415"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b93c76d23f68153264234fdfac95a880b2ef4a6a","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":263,"context_line":"                                          ha_network.network.id, tenant_id)"},{"line_number":264,"context_line":"            if not port_binding:"},{"line_number":265,"context_line":"                LOG.warning(_LW(\"Failed to add router %s HA port binding\"),"},{"line_number":266,"context_line":"                            router_id)"},{"line_number":267,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_e4caf44e","line":264,"updated":"2016-02-26 06:16:55.000000000","message":"just debug","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"c4c073c08d34840d3a20a91456237bdafaf06948","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        try:"},{"line_number":262,"context_line":"            port_binding \u003d plugin.add_ha_port(context.elevated(), router_id,"},{"line_number":263,"context_line":"                                          ha_network.network.id, tenant_id)"},{"line_number":264,"context_line":"            if not port_binding:"},{"line_number":265,"context_line":"                LOG.warning(_LW(\"Failed to add router %s HA port binding\"),"},{"line_number":266,"context_line":"                            router_id)"},{"line_number":267,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":24,"id":"9aed3d3a_8f5ce126","line":264,"in_reply_to":"9aed3d3a_e4caf44e","updated":"2016-02-26 06:43:54.000000000","message":"Done","commit_id":"0c5b9a2d41027d4ee241f7d7887e52f7aa4e9d61"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"6c487ad5e2379720582d57c58cb21097d64c5081","unresolved":false,"context_lines":[{"line_number":270,"context_line":"        except l3.RouterNotFound:"},{"line_number":271,"context_line":"            LOG.debug(\u0027Router %s has already been removed \u0027"},{"line_number":272,"context_line":"                      \u0027by concurrent operation\u0027, router_id)"},{"line_number":273,"context_line":"            return"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.bind_router(context, router_id, agent)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9aed3d3a_63e03afb","line":273,"updated":"2016-02-29 03:40:50.000000000","message":"Directly return here will stop bind_router be called, that makes sense. But will we take care of return value of its caller? Like \"return scheduled\" in method _schedule_ha_routers_to_additional_agent in this file. Now it will always return scheduled(True) even router isn\u0027t scheduled indeed. Inconsistency happens there.\n\nOne thing lucky is those return value don\u0027t get care by any caller in source code, but only in test code. So could you do something for that.","commit_id":"d1accc36689fdda5bb5e16d276d5f284bc80a6bc"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"fc5d074218b86f011a865d4f07c53bbce3e0077e","unresolved":false,"context_lines":[{"line_number":270,"context_line":"        except l3.RouterNotFound:"},{"line_number":271,"context_line":"            LOG.debug(\u0027Router %s has already been removed \u0027"},{"line_number":272,"context_line":"                      \u0027by concurrent operation\u0027, router_id)"},{"line_number":273,"context_line":"            return"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.bind_router(context, router_id, agent)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9aed3d3a_83e57edd","line":273,"in_reply_to":"9aed3d3a_63e03afb","updated":"2016-02-29 03:47:07.000000000","message":"I mean maybe you could add a return value for create_ha_port_and_bind to make all return value behavior keep consistent, or maybe just note something for source code or for test code.","commit_id":"d1accc36689fdda5bb5e16d276d5f284bc80a6bc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7a2cb099d730de74a6d6e4168caaad6d02c41cfa","unresolved":false,"context_lines":[{"line_number":270,"context_line":"        except l3.RouterNotFound:"},{"line_number":271,"context_line":"            LOG.debug(\u0027Router %s has already been removed \u0027"},{"line_number":272,"context_line":"                      \u0027by concurrent operation\u0027, router_id)"},{"line_number":273,"context_line":"            return"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.bind_router(context, router_id, agent)"},{"line_number":276,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"9aed3d3a_e98b8966","line":273,"in_reply_to":"9aed3d3a_83e57edd","updated":"2016-02-29 05:33:05.000000000","message":"Here return None is similar to Line 212 in function bind_router.","commit_id":"d1accc36689fdda5bb5e16d276d5f284bc80a6bc"}]}