)]}'
{"neutron/db/extraroute_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"75e0d6ce4dc901d05ac9e75ee00c87ee81b7c86f","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # NOTE(bence romsics): The input validation is delayed until"},{"line_number":199,"context_line":"        # update_router() validates the whole set of routes. Until then"},{"line_number":200,"context_line":"        # do not trust \u0027routes\u0027."},{"line_number":201,"context_line":"        routes \u003d body[\u0027router\u0027][\u0027routes\u0027]"},{"line_number":202,"context_line":"        with db_api.CONTEXT_WRITER.using(context):"},{"line_number":203,"context_line":"            old_routes \u003d self._get_extra_routes_by_router_id("},{"line_number":204,"context_line":"                context, router_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_42a6ec11","line":201,"updated":"2019-08-12 14:38:40.000000000","message":"I guess if {} is passed we can\u0027t return early because the API is returning the router?","commit_id":"6ec0721b2ed2a0ca1f5fc8e5bcaa7a7b747b4e7b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"099637a938fb2141723e958ea4bb76bf85dd7881","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # NOTE(bence romsics): The input validation is delayed until"},{"line_number":199,"context_line":"        # update_router() validates the whole set of routes. Until then"},{"line_number":200,"context_line":"        # do not trust \u0027routes\u0027."},{"line_number":201,"context_line":"        routes \u003d body[\u0027router\u0027][\u0027routes\u0027]"},{"line_number":202,"context_line":"        with db_api.CONTEXT_WRITER.using(context):"},{"line_number":203,"context_line":"            old_routes \u003d self._get_extra_routes_by_router_id("},{"line_number":204,"context_line":"                context, router_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_c16ba81c","line":201,"in_reply_to":"7faddb67_42a6ec11","updated":"2019-08-13 12:34:42.000000000","message":"Please see below.","commit_id":"6ec0721b2ed2a0ca1f5fc8e5bcaa7a7b747b4e7b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"75e0d6ce4dc901d05ac9e75ee00c87ee81b7c86f","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                router_id,"},{"line_number":208,"context_line":"                {\u0027router\u0027:"},{"line_number":209,"context_line":"                    {\u0027routes\u0027:"},{"line_number":210,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":211,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    @db_api.retry_if_session_inactive()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_223f3088","line":210,"updated":"2019-08-12 14:38:40.000000000","message":"Similar question here - if the \u0027add\u0027 didn\u0027t actually add a new route do we still need to update the router?  Corner case I know but there\u0027s going to be an update sent to the l3-agent\u0027s that could be avoided.\n\nSame questions for delete.","commit_id":"6ec0721b2ed2a0ca1f5fc8e5bcaa7a7b747b4e7b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"099637a938fb2141723e958ea4bb76bf85dd7881","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                router_id,"},{"line_number":208,"context_line":"                {\u0027router\u0027:"},{"line_number":209,"context_line":"                    {\u0027routes\u0027:"},{"line_number":210,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":211,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    @db_api.retry_if_session_inactive()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_c10c0844","line":210,"in_reply_to":"7faddb67_223f3088","updated":"2019-08-13 12:34:42.000000000","message":"I understand you want to optimize the \"empty update\" because that can be replaced with a \"get\", so we don\u0027t trigger the deeper side effects (e.g. I/O to the agents), right?\n\nThis optimization makes sense. But I think this should be done in update_router(). Because there a single change will optimize all calls to update_router(). Why would we want to optimize at all places where we call update_router() instead of doing it once in update_router() itself? Especially that update_router() already contains a get_router() so there\u0027s no extra db cost there to do this.","commit_id":"6ec0721b2ed2a0ca1f5fc8e5bcaa7a7b747b4e7b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2add9a8ccda55669880793ffe21f3c2dcbb00835","unresolved":false,"context_lines":[{"line_number":207,"context_line":"                router_id,"},{"line_number":208,"context_line":"                {\u0027router\u0027:"},{"line_number":209,"context_line":"                    {\u0027routes\u0027:"},{"line_number":210,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":211,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    @db_api.retry_if_session_inactive()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_599d7064","line":210,"in_reply_to":"7faddb67_c10c0844","updated":"2019-08-13 14:31:53.000000000","message":"Yes, just trying to avoid work.\n\nLooking at update_router/_update_router_db, it does handle the [] case by not doing a DB update, but will still emit a event, which I guess is OK.\n\nI guess this case of \u0027routes\u0027 being the same before/after is a subset of that - nothing has changed, but we do a DB update anyways.  The router_update code doesn\u0027t attempt to handle that, and it doesn\u0027t look trivial to change it.\n\nIn the end it\u0027s an edge case, and the user made the API call so knows what they\u0027re doing....","commit_id":"6ec0721b2ed2a0ca1f5fc8e5bcaa7a7b747b4e7b"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"871fd82d84a60f7702a50c59f70f11a03812be4b","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        them are returned."},{"line_number":162,"context_line":"        Overlapping destinations are accepted and all of them are returned."},{"line_number":163,"context_line":"        \"\"\""},{"line_number":164,"context_line":"        routes_dict \u003d {}  # its values are sets of nexthops"},{"line_number":165,"context_line":"        for r in old + add:"},{"line_number":166,"context_line":"            dst \u003d r[\u0027destination\u0027]"},{"line_number":167,"context_line":"            nexthop \u003d r[\u0027nexthop\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_2c3c57e0","line":164,"range":{"start_line":164,"start_character":8,"end_line":164,"end_character":59},"updated":"2019-08-22 11:45:13.000000000","message":"nice/elegant idea with the set","commit_id":"5b81c2181248b0ec8f0d80a7a3d738194897e173"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        for r in old:"},{"line_number":186,"context_line":"            dst \u003d r[\u0027destination\u0027]"},{"line_number":187,"context_line":"            nexthop \u003d r[\u0027nexthop\u0027]"},{"line_number":188,"context_line":"            result[dst] \u003d nexthop"},{"line_number":189,"context_line":"        for r in remove:"},{"line_number":190,"context_line":"            dst \u003d r[\u0027destination\u0027]"},{"line_number":191,"context_line":"            nexthop \u003d r[\u0027nexthop\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_41810c4a","line":188,"range":{"start_line":188,"start_character":12,"end_line":188,"end_character":33},"updated":"2019-09-10 16:07:51.000000000","message":"If there are two entries with same destination and different nexthops, the one entry will be overridden by the other.\nDon\u0027t we consider (dest, nexthop) pair as unique entry?","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        for r in old:"},{"line_number":186,"context_line":"            dst \u003d r[\u0027destination\u0027]"},{"line_number":187,"context_line":"            nexthop \u003d r[\u0027nexthop\u0027]"},{"line_number":188,"context_line":"            result[dst] \u003d nexthop"},{"line_number":189,"context_line":"        for r in remove:"},{"line_number":190,"context_line":"            dst \u003d r[\u0027destination\u0027]"},{"line_number":191,"context_line":"            nexthop \u003d r[\u0027nexthop\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_00153c4a","line":188,"range":{"start_line":188,"start_character":12,"end_line":188,"end_character":33},"in_reply_to":"5faad753_41810c4a","updated":"2019-09-11 12:04:23.000000000","message":"Good catch. I don\u0027t what I had been thinking when I fixed this problem in _add_extra_routes() but did not fix it here.","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e6b5c09472610e191275dbc0a649cf98904160a0","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_1a15a89c","line":217,"updated":"2019-09-11 13:16:49.000000000","message":"for me this new method isn\u0027t atomic at all. Imagine that You have 2 neutron-servers, now You are doing two calls to add some routes to router. First request is going to first neutron-server where You have old\u003d[] and second goes to second neutron-server where You also have old\u003d[]. Now each of them adds one new extraroute and saves it in db. Finally only one of those 2 extraroutes will be added.\n\nAm I missing something here?","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"99ab1730c7e67c6e18749d267bca6111d6b6c910","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_9c28a234","line":217,"in_reply_to":"5faad753_04af8794","updated":"2019-09-12 06:53:15.000000000","message":"Ok, You\u0027re totally right. It\u0027s fine. Sorry for the noise.","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8a31ea820bc618115accf2a9a52c07d883d8855d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_f0254dac","line":217,"in_reply_to":"5faad753_1a15a89c","updated":"2019-09-11 14:02:17.000000000","message":"We should be in a DB transaction. Am I completely misunderstanding the db_api.CONTEXT_WRITER context manager?","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"4635bfec26f9215133595bf827a9f0d4dc5f4ff8","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_04af8794","line":217,"in_reply_to":"5faad753_69f41487","updated":"2019-09-11 16:31:50.000000000","message":"The use case that\u0027s actually affected is when multiple clients claim their need for the exact same route (and the route should be there from the first add until all of them removed/released it) - that\u0027s the one we discussed at the beginning of this cycle and said we don\u0027t want to support it. So I\u0027m baffled.","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"5dd29d741d2b35963b6b600d279c79da7ee52479","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_f792114b","line":217,"in_reply_to":"5faad753_9c28a234","updated":"2019-09-12 07:53:51.000000000","message":"No problem. :-)","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"0fee71adb49ed123cd1bd3b08ec569d48ee0cffb","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                {\u0027router\u0027:"},{"line_number":215,"context_line":"                    {\u0027routes\u0027:"},{"line_number":216,"context_line":"                        self._add_extra_routes(old_routes, routes)}})"},{"line_number":217,"context_line":"            return {\u0027router\u0027: router}"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    @db_api.retry_if_session_inactive()"},{"line_number":220,"context_line":"    def remove_extraroutes(self, context, router_id, body\u003dNone):"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_69f41487","line":217,"in_reply_to":"5faad753_f0254dac","updated":"2019-09-11 16:00:19.000000000","message":"There\u0027s no compare and swap here - that\u0027s intentional. But the add and remove diffs/requests (multiple routes in each diff) sent by multiple clients should be applied serially because of the DB transactions. The order of the diffs is not guaranteed by anything, but as long as multiple clients don\u0027t try to add/remove the *exact same* route what kind of use case would be broken by that?","commit_id":"ab07b91b1adf9c7fe3d94a67aeb689788dfee09a"}],"neutron/tests/unit/db/test_extraroute_db.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    def assertEqualRoutes(self, a, b):"},{"line_number":78,"context_line":"        \"\"\"Compare a list of routes without caring for the list order.\"\"\""},{"line_number":79,"context_line":"        return self.assertEqual("},{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_9efd233c","line":79,"range":{"start_line":79,"start_character":20,"end_line":79,"end_character":31},"updated":"2019-09-10 16:07:51.000000000","message":"assertSetEqual might be used. If so, the failure message would be a bit more human-friendly.","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    def assertEqualRoutes(self, a, b):"},{"line_number":78,"context_line":"        \"\"\"Compare a list of routes without caring for the list order.\"\"\""},{"line_number":79,"context_line":"        return self.assertEqual("},{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_05b30eec","line":79,"range":{"start_line":79,"start_character":20,"end_line":79,"end_character":31},"in_reply_to":"5faad753_9efd233c","updated":"2019-09-11 12:04:23.000000000","message":"Done","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def assertEqualRoutes(self, a, b):"},{"line_number":78,"context_line":"        \"\"\"Compare a list of routes without caring for the list order.\"\"\""},{"line_number":79,"context_line":"        return self.assertEqual("},{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def test_add_extra_routes(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_be71ff0e","line":80,"range":{"start_line":80,"start_character":12,"end_line":80,"end_character":25},"updated":"2019-09-10 16:07:51.000000000","message":"Why do we need to convert frozenset into set?","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def assertEqualRoutes(self, a, b):"},{"line_number":78,"context_line":"        \"\"\"Compare a list of routes without caring for the list order.\"\"\""},{"line_number":79,"context_line":"        return self.assertEqual("},{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def test_add_extra_routes(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_c5fad673","line":80,"range":{"start_line":80,"start_character":12,"end_line":80,"end_character":25},"in_reply_to":"5faad753_be71ff0e","updated":"2019-09-11 12:04:23.000000000","message":"We don\u0027t convert. It\u0027s a set of frozensets. The inner set has to be frozen otherwise it\u0027s not hashable, so it cannot be put in the outer set.\n\nThe inner set is needed so we can ignore the dict order of a single route (dst-nexthop or nexthop-dst).\nThe outer set is needed so we can ignore the order of routes.","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"57fb4ca67bf7247fa4e2dbb5fa6a5fd9a7ac2991","unresolved":false,"context_lines":[{"line_number":77,"context_line":"    def assertEqualRoutes(self, a, b):"},{"line_number":78,"context_line":"        \"\"\"Compare a list of routes without caring for the list order.\"\"\""},{"line_number":79,"context_line":"        return self.assertEqual("},{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def test_add_extra_routes(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_2b4a997f","line":80,"range":{"start_line":80,"start_character":12,"end_line":80,"end_character":25},"in_reply_to":"5faad753_c5fad673","updated":"2019-09-11 12:10:18.000000000","message":"Ah.. I see. Thanks for the explanation. I seemed to be confused.","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"7ecbe0c7818c4c541f16890ee15e29bc5dff750c","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def test_add_extra_routes(self):"},{"line_number":84,"context_line":"        self.assertEqual("},{"line_number":85,"context_line":"            [],"},{"line_number":86,"context_line":"            self._plugin._add_extra_routes([], []),"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_9e91a39c","line":83,"updated":"2019-09-10 15:34:29.000000000","message":"nice tests :-)","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            set(frozenset(r.items()) for r in a),"},{"line_number":81,"context_line":"            set(frozenset(r.items()) for r in b))"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def test_add_extra_routes(self):"},{"line_number":84,"context_line":"        self.assertEqual("},{"line_number":85,"context_line":"            [],"},{"line_number":86,"context_line":"            self._plugin._add_extra_routes([], []),"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_80504c5f","line":83,"in_reply_to":"5faad753_9e91a39c","updated":"2019-09-11 12:04:23.000000000","message":"thanks","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.assertEqual("},{"line_number":90,"context_line":"            [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":91,"context_line":"            self._plugin._add_extra_routes("},{"line_number":92,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":93,"context_line":"                [],"},{"line_number":94,"context_line":"            ),"},{"line_number":95,"context_line":"        )"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_c1173c45","line":93,"range":{"start_line":92,"start_character":15,"end_line":93,"end_character":18},"updated":"2019-09-10 16:07:51.000000000","message":"From the readability, it is better to introduce \"old\" and \"add\" variables.\n\n  old \u003d [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}]\n  add \u003d []\n  self._plugin._add_extra_routes(old, add)\n\nor\n\n  self._plugin._add_extra_routes(\n    old\u003d[{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],\n    add\u003d[])\n\nSame comment for the remove_route test below.","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.assertEqual("},{"line_number":90,"context_line":"            [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":91,"context_line":"            self._plugin._add_extra_routes("},{"line_number":92,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":93,"context_line":"                [],"},{"line_number":94,"context_line":"            ),"},{"line_number":95,"context_line":"        )"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_e0510064","line":93,"range":{"start_line":92,"start_character":15,"end_line":93,"end_character":18},"in_reply_to":"5faad753_c1173c45","updated":"2019-09-11 12:04:23.000000000","message":"Done","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            [],"},{"line_number":125,"context_line":"            self._plugin._remove_extra_routes([], []),"},{"line_number":126,"context_line":"        )"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        self.assertEqual("},{"line_number":129,"context_line":"            [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":130,"context_line":"            self._plugin._remove_extra_routes("}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_5e632b1b","line":127,"updated":"2019-09-10 16:07:51.000000000","message":"I think a test for normal removal is missing. Special cases are covered.\n\n  self.assertEqual(\n      [],\n      self._plugin._remove_extra_routes(\n          [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],\n          [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],\n      ),\n  )\n\nand/or\n\n  self.assertEqual(\n      [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],\n      self._plugin._remove_extra_routes(\n          [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"},\n            {\"destination\": \"10.0.20.0/24\", \"nexthop\": \"10.0.0.11\"}],\n          [{\"destination\": \"10.0.20.0/24\", \"nexthop\": \"10.0.0.11\"}],\n      ),\n  )","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            [],"},{"line_number":125,"context_line":"            self._plugin._remove_extra_routes([], []),"},{"line_number":126,"context_line":"        )"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        self.assertEqual("},{"line_number":129,"context_line":"            [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":130,"context_line":"            self._plugin._remove_extra_routes("}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_c056445d","line":127,"in_reply_to":"5faad753_5e632b1b","updated":"2019-09-11 12:04:23.000000000","message":"Done","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"6cec0b500cefa42a4d940db58ae79adf1c42313c","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":148,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.11\"}],"},{"line_number":149,"context_line":"            ),"},{"line_number":150,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_410cac3a","line":150,"updated":"2019-09-10 16:07:51.000000000","message":"The following test is worth added: two entries with same destination and different nexthops.\n\n  old \u003d [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"},\n         {\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.11\"}]\n  remove \u003d [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}]\n\nI expect the return value would be:\n\n  [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}]","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1f579c665dc5814b02609b6e1c26747a455608dd","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.10\"}],"},{"line_number":148,"context_line":"                [{\"destination\": \"10.0.10.0/24\", \"nexthop\": \"10.0.0.11\"}],"},{"line_number":149,"context_line":"            ),"},{"line_number":150,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faad753_a055481f","line":150,"in_reply_to":"5faad753_410cac3a","updated":"2019-09-11 12:04:23.000000000","message":"Done","commit_id":"e07f7b05e03b67a2b48780b6c42e01a8b394848b"}]}