)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ac0bf7083773e6d08e66de2262f94bcc3f81878","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Allow concurrent schedule() to run on HA routers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Changeset I22ff5a5a74527366da8f82982232d4e70e455570 introduces a new"},{"line_number":10,"context_line":"status for routers, ALLOCATING, which was intended to not inform l3"},{"line_number":11,"context_line":"agents about HA routers that are currently being handled by a server"},{"line_number":12,"context_line":"(either allocating resources for, or scheduling a router). This"},{"line_number":13,"context_line":"approached turned out to have a few downsides, mainly the fact that a"},{"line_number":14,"context_line":"router could be \"locked\" in the ALLOCATING state (see bug report for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"fa7ab95a_8b603137","line":11,"range":{"start_line":10,"start_character":32,"end_line":11,"end_character":68},"updated":"2016-09-01 10:52:23.000000000","message":"how does new approach solves this problem?","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"61e91f4a7178ae993ac86508f2c968e9864cc310","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Allow concurrent schedule() to run on HA routers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Changeset I22ff5a5a74527366da8f82982232d4e70e455570 introduces a new"},{"line_number":10,"context_line":"status for routers, ALLOCATING, which was intended to not inform l3"},{"line_number":11,"context_line":"agents about HA routers that are currently being handled by a server"},{"line_number":12,"context_line":"(either allocating resources for, or scheduling a router). This"},{"line_number":13,"context_line":"approached turned out to have a few downsides, mainly the fact that a"},{"line_number":14,"context_line":"router could be \"locked\" in the ALLOCATING state (see bug report for"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"fa7ab95a_7dd65428","line":11,"range":{"start_line":10,"start_character":32,"end_line":11,"end_character":68},"in_reply_to":"fa7ab95a_8b603137","updated":"2016-09-01 10:59:37.000000000","message":"The second patch of the series, [1], adds in a check with a \"make sure the router returned to the host has the ha interfaces it expects\". This way, an agent won\u0027t receive a router\u0027s information that is incomplete.\n\n[1]: https://review.openstack.org/#/c/357966/9/neutron/db/l3_hamode_db.py@765","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ac0bf7083773e6d08e66de2262f94bcc3f81878","unresolved":false,"context_lines":[{"line_number":34,"context_line":"commit the transaction will know that he\u0027s too late and will stop the"},{"line_number":35,"context_line":"scheduling attempts."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Lastly, this patch removes the unneeded return value from"},{"line_number":38,"context_line":"auto_schedule_routers (and other functions invoked by it) and modifies"},{"line_number":39,"context_line":"tests to check if that function actually worked by checking actual"},{"line_number":40,"context_line":"router-to-agent bindings."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Partial-Bug: #1609738"},{"line_number":43,"context_line":"Change-Id: I1589ed9d6514647f4167e291b3016b511c670526"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"fa7ab95a_0b55014f","line":40,"range":{"start_line":37,"start_character":0,"end_line":40,"end_character":25},"updated":"2016-09-01 10:52:23.000000000","message":"a good candidate for a separate change","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"61e91f4a7178ae993ac86508f2c968e9864cc310","unresolved":false,"context_lines":[{"line_number":34,"context_line":"commit the transaction will know that he\u0027s too late and will stop the"},{"line_number":35,"context_line":"scheduling attempts."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Lastly, this patch removes the unneeded return value from"},{"line_number":38,"context_line":"auto_schedule_routers (and other functions invoked by it) and modifies"},{"line_number":39,"context_line":"tests to check if that function actually worked by checking actual"},{"line_number":40,"context_line":"router-to-agent bindings."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Partial-Bug: #1609738"},{"line_number":43,"context_line":"Change-Id: I1589ed9d6514647f4167e291b3016b511c670526"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"fa7ab95a_ddce8082","line":40,"range":{"start_line":37,"start_character":0,"end_line":40,"end_character":25},"in_reply_to":"fa7ab95a_0b55014f","updated":"2016-09-01 10:59:37.000000000","message":"Since we want to push this","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"c1ad0f3c97d960a1b813303bbc0d6dfe8da317ec","unresolved":false,"context_lines":[{"line_number":34,"context_line":"commit the transaction will know that he\u0027s too late and will stop the"},{"line_number":35,"context_line":"scheduling attempts."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Lastly, this patch removes the unneeded return value from"},{"line_number":38,"context_line":"auto_schedule_routers (and other functions invoked by it) and modifies"},{"line_number":39,"context_line":"tests to check if that function actually worked by checking actual"},{"line_number":40,"context_line":"router-to-agent bindings."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"Partial-Bug: #1609738"},{"line_number":43,"context_line":"Change-Id: I1589ed9d6514647f4167e291b3016b511c670526"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"fa7ab95a_20badf23","line":40,"range":{"start_line":37,"start_character":0,"end_line":40,"end_character":25},"in_reply_to":"fa7ab95a_ddce8082","updated":"2016-09-01 11:10:08.000000000","message":"I\u0027ll continue the sentence: since we want to push this for N, I\u0027d rather not split it off to a different patch this late in the cycle.","commit_id":"8619abb837ab367842324df1d1959a426feef5da"}],"neutron/db/l3_hamode_db.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9295cfc07c1953c36e0d0f30b45d26058fc02f64","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        sa.UniqueConstraint("},{"line_number":105,"context_line":"            \u0027router_id\u0027, \u0027binding_id\u0027,"},{"line_number":106,"context_line":"            name\u003d\u0027uniq_ha_router_agent_port_bindings0router_id0binding_id0\u0027),"},{"line_number":107,"context_line":"        model_base.BASEV2.__table_args__"},{"line_number":108,"context_line":"    )"},{"line_number":109,"context_line":"    port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027,"},{"line_number":110,"context_line":"                                                     ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_04da5078","line":107,"updated":"2016-08-22 12:01:57.000000000","message":"What about adding unique for l3_agent_id and binding_id as well?","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"076e38c920fe789cc64407e7f8bb6e3222ca1a4b","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        sa.UniqueConstraint("},{"line_number":105,"context_line":"            \u0027router_id\u0027, \u0027binding_id\u0027,"},{"line_number":106,"context_line":"            name\u003d\u0027uniq_ha_router_agent_port_bindings0router_id0binding_id0\u0027),"},{"line_number":107,"context_line":"        model_base.BASEV2.__table_args__"},{"line_number":108,"context_line":"    )"},{"line_number":109,"context_line":"    port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027,"},{"line_number":110,"context_line":"                                                     ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_24b2ec94","line":107,"in_reply_to":"1ac06dbe_04da5078","updated":"2016-08-22 12:12:50.000000000","message":"If you think about it, port_ids are unique anyways (being UUIDs) and each l3_agent_id can have multiple bindings to it (if there are more than a single HA router, an agent can have more than one router scheduled to it and this shouldn\u0027t change pending on some index that doesn\u0027t mean anything in this context).\n\nAnyway, adding the same (router_id, l3_agent_id) pair is impossible thanks to the previous constraint anyway (so this makes sure that the same binding is made only once.","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0c617444106dd9f700c85344dbda645ba16d7d96","unresolved":false,"context_lines":[{"line_number":662,"context_line":"        return query.first()"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"    def get_vacant_binding_index(self, context, router_id,"},{"line_number":665,"context_line":"                                 is_manual_scheduling\u003dFalse):"},{"line_number":666,"context_line":"        # Lets figure out how many agents there are. If there aren\u0027t enough,"},{"line_number":667,"context_line":"        # this will throw an exception and we\u0027ll save running the rest of the"},{"line_number":668,"context_line":"        # function."}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_4c3e8e2a","line":665,"updated":"2016-08-24 13:53:28.000000000","message":"Please add a docstring here to explain the parameters returned by the method. The \u0027vacant\u0027 index is understood but why the boolean?\nwhere is an exception thrown?","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9c554b5f409b79aec754ba6ed171635364652f8f","unresolved":false,"context_lines":[{"line_number":662,"context_line":"        return query.first()"},{"line_number":663,"context_line":""},{"line_number":664,"context_line":"    def get_vacant_binding_index(self, context, router_id,"},{"line_number":665,"context_line":"                                 is_manual_scheduling\u003dFalse):"},{"line_number":666,"context_line":"        # Lets figure out how many agents there are. If there aren\u0027t enough,"},{"line_number":667,"context_line":"        # this will throw an exception and we\u0027ll save running the rest of the"},{"line_number":668,"context_line":"        # function."}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_25ab65d8","line":665,"in_reply_to":"1ac06dbe_4c3e8e2a","updated":"2016-08-24 14:18:43.000000000","message":"Will do.","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":659,"context_line":"            L3HARouterAgentPortBinding.binding_index \u003d\u003d binding_index)"},{"line_number":660,"context_line":"        return query.first()"},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"    def get_vacant_binding_index(self, context, router_id,"},{"line_number":663,"context_line":"                                 is_manual_scheduling\u003dFalse):"},{"line_number":664,"context_line":"        \"\"\"Return a vacant binding_index to use and whether or not it exists."},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        Each L3HARouterAgentPortBinding has a binding_index which is unique"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_2b32b410","line":663,"range":{"start_line":662,"start_character":4,"end_line":663,"end_character":61},"updated":"2016-08-29 08:09:58.000000000","message":"This function is really hard for me to review. : (","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"214003f68286a3c7b8422648788cbf9fab753369","unresolved":false,"context_lines":[{"line_number":659,"context_line":"            L3HARouterAgentPortBinding.binding_index \u003d\u003d binding_index)"},{"line_number":660,"context_line":"        return query.first()"},{"line_number":661,"context_line":""},{"line_number":662,"context_line":"    def get_vacant_binding_index(self, context, router_id,"},{"line_number":663,"context_line":"                                 is_manual_scheduling\u003dFalse):"},{"line_number":664,"context_line":"        \"\"\"Return a vacant binding_index to use and whether or not it exists."},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        Each L3HARouterAgentPortBinding has a binding_index which is unique"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_f27c38b7","line":663,"range":{"start_line":662,"start_character":4,"end_line":663,"end_character":61},"in_reply_to":"fa7ab95a_2b32b410","updated":"2016-08-29 10:23:50.000000000","message":"What can make it easier? I deliberately added a very long docstring and comments before each block, so if there\u0027s anything else let me know :)","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b2d8e551a97da23f55e5ec57b37519b81de14c08","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":693,"context_line":"        if not bindings:"},{"line_number":694,"context_line":"            return 1, False"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        # If there\u0027s largest binding_id index not max_l3_agents_per_router,"},{"line_number":697,"context_line":"        # then there\u0027s still room to add the binding_id at the end"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_a3c892a9","line":694,"range":{"start_line":694,"start_character":19,"end_line":694,"end_character":20},"updated":"2016-08-29 14:25:50.000000000","message":"LOWEST_BINDING_INDEX","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2170cdf1bfbe2e8ffce36258df1759b3b6aeb898","unresolved":false,"context_lines":[{"line_number":691,"context_line":""},{"line_number":692,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":693,"context_line":"        if not bindings:"},{"line_number":694,"context_line":"            return 1, False"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        # If there\u0027s largest binding_id index not max_l3_agents_per_router,"},{"line_number":697,"context_line":"        # then there\u0027s still room to add the binding_id at the end"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_831f0e6b","line":694,"range":{"start_line":694,"start_character":19,"end_line":694,"end_character":20},"in_reply_to":"fa7ab95a_a3c892a9","updated":"2016-08-29 14:27:58.000000000","message":"Done","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b2d8e551a97da23f55e5ec57b37519b81de14c08","unresolved":false,"context_lines":[{"line_number":704,"context_line":"            return LOWEST_BINDING_INDEX, False"},{"line_number":705,"context_line":""},{"line_number":706,"context_line":"        for index in range(len(bindings)):"},{"line_number":707,"context_line":"            # If a router was manually unscheduled from an agent andf it\u0027s"},{"line_number":708,"context_line":"            # currently unused"},{"line_number":709,"context_line":"            if (index \u003e 0 and"},{"line_number":710,"context_line":"                binding_indices[index] - binding_indices[index - 1] \u003e 1):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_a7cc286b","line":707,"range":{"start_line":707,"start_character":68,"end_line":707,"end_character":69},"updated":"2016-08-29 14:25:50.000000000","message":"extra f","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2170cdf1bfbe2e8ffce36258df1759b3b6aeb898","unresolved":false,"context_lines":[{"line_number":704,"context_line":"            return LOWEST_BINDING_INDEX, False"},{"line_number":705,"context_line":""},{"line_number":706,"context_line":"        for index in range(len(bindings)):"},{"line_number":707,"context_line":"            # If a router was manually unscheduled from an agent andf it\u0027s"},{"line_number":708,"context_line":"            # currently unused"},{"line_number":709,"context_line":"            if (index \u003e 0 and"},{"line_number":710,"context_line":"                binding_indices[index] - binding_indices[index - 1] \u003e 1):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_e30e1a3a","line":707,"range":{"start_line":707,"start_character":68,"end_line":707,"end_character":69},"in_reply_to":"fa7ab95a_a7cc286b","updated":"2016-08-29 14:27:58.000000000","message":"Done","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b2d8e551a97da23f55e5ec57b37519b81de14c08","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            # If a router was manually unscheduled from an agent andf it\u0027s"},{"line_number":708,"context_line":"            # currently unused"},{"line_number":709,"context_line":"            if (index \u003e 0 and"},{"line_number":710,"context_line":"                binding_indices[index] - binding_indices[index - 1] \u003e 1):"},{"line_number":711,"context_line":"                return binding_indices[index - 1] + 1, False"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"            # If the binding was never used in the first place, return it to"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_83f0eeff","line":710,"range":{"start_line":710,"start_character":16,"end_line":710,"end_character":71},"updated":"2016-08-29 14:25:50.000000000","message":"Hm, how does this work without any sorting? I suppose binding_indices should be also sorted at line 689. I think this can simplify logic in some places.","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2170cdf1bfbe2e8ffce36258df1759b3b6aeb898","unresolved":false,"context_lines":[{"line_number":707,"context_line":"            # If a router was manually unscheduled from an agent andf it\u0027s"},{"line_number":708,"context_line":"            # currently unused"},{"line_number":709,"context_line":"            if (index \u003e 0 and"},{"line_number":710,"context_line":"                binding_indices[index] - binding_indices[index - 1] \u003e 1):"},{"line_number":711,"context_line":"                return binding_indices[index - 1] + 1, False"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"            # If the binding was never used in the first place, return it to"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_4302660c","line":710,"range":{"start_line":710,"start_character":16,"end_line":710,"end_character":71},"in_reply_to":"fa7ab95a_83f0eeff","updated":"2016-08-29 14:27:58.000000000","message":"Good catch! :)","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7831ad711a3721923ebe0d58bc7408e54536ff95","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        binding_indices \u003d list(map(lambda binding: binding.binding_index,"},{"line_number":690,"context_line":"                                   bindings))"},{"line_number":691,"context_line":"        binding_indices.sort()"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":694,"context_line":"        if not bindings:"},{"line_number":695,"context_line":"            return LOWEST_BINDING_INDEX, False"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_108fa22a","line":692,"updated":"2016-08-30 00:21:15.000000000","message":"Correct me if I\u0027m wrong, but I think we can get rid of a lot of the index calculation with some sets.\n\nThe following code would replace L684 - L725\n\nquery \u003d context.session.query(L3HARouterAgentPortBinding)\nquery \u003d query.filter(\n    L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)\nquery \u003d query.order_by(L3HARouterAgentPortBinding.binding_index.asc())\nbindings \u003d query.all()\nbinding_indicies \u003d [b.binding_index for b in bindings]\n\nall_indicies \u003d set(range(LOWEST_BINDING_INDEX, num_agents + 1))\nopen_slots \u003d sorted(list(all_indicies - set(binding_indices)))\nif open_slots:\n    return open_slots[0], False\n# look for existing binding without an agent\nfor binding in bindings:\n    if binding.l3_agent_id is None:\n        return binding.binding_index, True\n# Last change: if this is a manual...\nif is_manual_scheduling:\n    return max(all_indicies) + 1, False\nreturn -1, False\n\n\nIt\u0027s up to you, but I prefer that for readability over the -1/+1 calculations on L711 and it avoids special casing lowest and highest.","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d2cb13d82bd722fc0f5af3dd9ae77fbacd379541","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        binding_indices \u003d list(map(lambda binding: binding.binding_index,"},{"line_number":690,"context_line":"                                   bindings))"},{"line_number":691,"context_line":"        binding_indices.sort()"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":694,"context_line":"        if not bindings:"},{"line_number":695,"context_line":"            return LOWEST_BINDING_INDEX, False"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_c4b2cae4","line":692,"in_reply_to":"fa7ab95a_108fa22a","updated":"2016-08-30 08:49:56.000000000","message":"+1 for Kevin suggestion. \n\nAt least I recommend to change lambda for [b.binding_index for b in bindings].","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"96113e2fda3ba3479b3e3505ca050e2e34d8c1b9","unresolved":false,"context_lines":[{"line_number":689,"context_line":"        binding_indices \u003d list(map(lambda binding: binding.binding_index,"},{"line_number":690,"context_line":"                                   bindings))"},{"line_number":691,"context_line":"        binding_indices.sort()"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":694,"context_line":"        if not bindings:"},{"line_number":695,"context_line":"            return LOWEST_BINDING_INDEX, False"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_df0a8625","line":692,"in_reply_to":"fa7ab95a_c4b2cae4","updated":"2016-08-30 13:05:12.000000000","message":"+1.","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d2cb13d82bd722fc0f5af3dd9ae77fbacd379541","unresolved":false,"context_lines":[{"line_number":691,"context_line":"        binding_indices.sort()"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":694,"context_line":"        if not bindings:"},{"line_number":695,"context_line":"            return LOWEST_BINDING_INDEX, False"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"        # If there\u0027s largest binding_id index not max_l3_agents_per_router,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_26f1f095","line":694,"updated":"2016-08-30 08:49:56.000000000","message":"This check can be moved to 689","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"96113e2fda3ba3479b3e3505ca050e2e34d8c1b9","unresolved":false,"context_lines":[{"line_number":691,"context_line":"        binding_indices.sort()"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        # If there are no bindings at all - return the lowest possible binding."},{"line_number":694,"context_line":"        if not bindings:"},{"line_number":695,"context_line":"            return LOWEST_BINDING_INDEX, False"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"        # If there\u0027s largest binding_id index not max_l3_agents_per_router,"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_021d7165","line":694,"in_reply_to":"fa7ab95a_26f1f095","updated":"2016-08-30 13:05:12.000000000","message":"Don\u0027t think it\u0027s needed with Kevin\u0027s implementation. I did move some stuff around though :)","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"48e696f067ecec26c6f75b06efb261ba28d01312","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                    port_type\u003dconstants.DEVICE_OWNER_ROUTER_HA_INTF)"},{"line_number":352,"context_line":"                context.session.add(routerportbinding)"},{"line_number":353,"context_line":"                portbinding \u003d L3HARouterAgentPortBinding("},{"line_number":354,"context_line":"                    port_id\u003dport_id,"},{"line_number":355,"context_line":"                    router_id\u003drouter_id,"},{"line_number":356,"context_line":"                    binding_index\u003dbinding_index)"},{"line_number":357,"context_line":"                context.session.add(portbinding)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_b449df90","line":354,"updated":"2016-09-01 09:04:25.000000000","message":"in some countries changing the indentation of existing code is the death penalty!","commit_id":"74283725e680737b49ef5408df8417aa11921e88"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1fc766193cd91be206a61cf29c8196d3e99de854","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                    port_type\u003dconstants.DEVICE_OWNER_ROUTER_HA_INTF)"},{"line_number":352,"context_line":"                context.session.add(routerportbinding)"},{"line_number":353,"context_line":"                portbinding \u003d L3HARouterAgentPortBinding("},{"line_number":354,"context_line":"                    port_id\u003dport_id,"},{"line_number":355,"context_line":"                    router_id\u003drouter_id,"},{"line_number":356,"context_line":"                    binding_index\u003dbinding_index)"},{"line_number":357,"context_line":"                context.session.add(portbinding)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_191c8312","line":354,"in_reply_to":"fa7ab95a_b449df90","updated":"2016-09-01 09:10:57.000000000","message":"Please! Not the AXE!\n\n(Actaully, not changing the indentation will cause the assignment with \u0027binding_index\u0027 to exceed 80 chars, so this is a must)","commit_id":"74283725e680737b49ef5408df8417aa11921e88"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ac0bf7083773e6d08e66de2262f94bcc3f81878","unresolved":false,"context_lines":[{"line_number":368,"context_line":"                    raise l3.RouterNotFound(router_id\u003drouter_id)"},{"line_number":369,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":370,"context_line":"            LOG.debug(\"Failed to create HA router agent PortBinding with \""},{"line_number":371,"context_line":"                      \"index %d for router %s since it already exists\","},{"line_number":372,"context_line":"                      binding_index, router_id)"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def add_ha_port(self, context, router_id, binding_index,"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_0bb1818f","line":371,"range":{"start_line":371,"start_character":22,"end_line":371,"end_character":69},"updated":"2016-09-01 10:52:23.000000000","message":"we have 2 unique constraints; please add agent id to the log","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":368,"context_line":"                    raise l3.RouterNotFound(router_id\u003drouter_id)"},{"line_number":369,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":370,"context_line":"            LOG.debug(\"Failed to create HA router agent PortBinding with \""},{"line_number":371,"context_line":"                      \"index %d for router %s since it already exists\","},{"line_number":372,"context_line":"                      binding_index, router_id)"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def add_ha_port(self, context, router_id, binding_index,"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_c27133aa","line":371,"range":{"start_line":371,"start_character":22,"end_line":371,"end_character":69},"in_reply_to":"fa7ab95a_0bb1818f","updated":"2016-09-01 12:44:46.000000000","message":"The above block doesn\u0027t set the l3_agent_id though - not until later down the line. This only creates the actual L3HRouterPortBinding (and the l3_agent_id is filled out later).","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ac0bf7083773e6d08e66de2262f94bcc3f81878","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":370,"context_line":"            LOG.debug(\"Failed to create HA router agent PortBinding with \""},{"line_number":371,"context_line":"                      \"index %d for router %s since it already exists\","},{"line_number":372,"context_line":"                      binding_index, router_id)"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def add_ha_port(self, context, router_id, binding_index,"},{"line_number":375,"context_line":"                    network_id, tenant_id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_4b82a995","line":372,"range":{"start_line":372,"start_character":22,"end_line":372,"end_character":46},"updated":"2016-09-01 10:52:23.000000000","message":"nit: use dict when \u003e 1 args","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":369,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":370,"context_line":"            LOG.debug(\"Failed to create HA router agent PortBinding with \""},{"line_number":371,"context_line":"                      \"index %d for router %s since it already exists\","},{"line_number":372,"context_line":"                      binding_index, router_id)"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def add_ha_port(self, context, router_id, binding_index,"},{"line_number":375,"context_line":"                    network_id, tenant_id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_e268f7bc","line":372,"range":{"start_line":372,"start_character":22,"end_line":372,"end_character":46},"in_reply_to":"fa7ab95a_4b82a995","updated":"2016-09-01 12:44:46.000000000","message":"Done","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ac0bf7083773e6d08e66de2262f94bcc3f81878","unresolved":false,"context_lines":[{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        port_ids \u003d []"},{"line_number":408,"context_line":"        try:"},{"line_number":409,"context_line":"            for index in range(1, num_agents + 1):"},{"line_number":410,"context_line":"                binding \u003d self.add_ha_port(admin_ctx, router.id, index,"},{"line_number":411,"context_line":"                                           ha_network.network[\u0027id\u0027],"},{"line_number":412,"context_line":"                                           router.tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_2b5ac550","line":409,"range":{"start_line":409,"start_character":31,"end_line":409,"end_character":32},"updated":"2016-09-01 10:52:23.000000000","message":"LOWEST_BINDING_INDEX?","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        port_ids \u003d []"},{"line_number":408,"context_line":"        try:"},{"line_number":409,"context_line":"            for index in range(1, num_agents + 1):"},{"line_number":410,"context_line":"                binding \u003d self.add_ha_port(admin_ctx, router.id, index,"},{"line_number":411,"context_line":"                                           ha_network.network[\u0027id\u0027],"},{"line_number":412,"context_line":"                                           router.tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_42b5232f","line":409,"range":{"start_line":409,"start_character":31,"end_line":409,"end_character":32},"in_reply_to":"fa7ab95a_2b5ac550","updated":"2016-09-01 12:44:46.000000000","message":"Done","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"092d9da57336533aa3a6bb436798c538372ec67b","unresolved":false,"context_lines":[{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    def get_ha_router_port_binding(self, context, router_id, binding_index):"},{"line_number":655,"context_line":"        query \u003d context.session.query(L3HARouterAgentPortBinding)"},{"line_number":656,"context_line":"        query \u003d query.filter("},{"line_number":657,"context_line":"            L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":658,"context_line":"        query \u003d query.filter("},{"line_number":659,"context_line":"            L3HARouterAgentPortBinding.binding_index \u003d\u003d binding_index)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_715e7bea","line":656,"range":{"start_line":656,"start_character":22,"end_line":656,"end_character":29},"updated":"2016-09-01 12:00:42.000000000","message":"nit: filter_by(router_id \u003d\u003d router_id)","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    def get_ha_router_port_binding(self, context, router_id, binding_index):"},{"line_number":655,"context_line":"        query \u003d context.session.query(L3HARouterAgentPortBinding)"},{"line_number":656,"context_line":"        query \u003d query.filter("},{"line_number":657,"context_line":"            L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":658,"context_line":"        query \u003d query.filter("},{"line_number":659,"context_line":"            L3HARouterAgentPortBinding.binding_index \u003d\u003d binding_index)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_c2cdb3b8","line":656,"range":{"start_line":656,"start_character":22,"end_line":656,"end_character":29},"in_reply_to":"fa7ab95a_715e7bea","updated":"2016-09-01 12:44:46.000000000","message":"Done","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"092d9da57336533aa3a6bb436798c538372ec67b","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        Each L3HARouterAgentPortBinding has a binding_index which is unique"},{"line_number":667,"context_line":"        per router_id, and when creating a single binding we require to find a"},{"line_number":668,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example of we have"},{"line_number":669,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        However, it\u0027s possible that the some L3HARouterAgentPortBinding is"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_f1728b7b","line":668,"range":{"start_line":668,"start_character":66,"end_line":668,"end_character":68},"updated":"2016-09-01 12:00:42.000000000","message":"if","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        Each L3HARouterAgentPortBinding has a binding_index which is unique"},{"line_number":667,"context_line":"        per router_id, and when creating a single binding we require to find a"},{"line_number":668,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example of we have"},{"line_number":669,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        However, it\u0027s possible that the some L3HARouterAgentPortBinding is"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_42ba830b","line":668,"range":{"start_line":668,"start_character":66,"end_line":668,"end_character":68},"in_reply_to":"fa7ab95a_f1728b7b","updated":"2016-09-01 12:44:46.000000000","message":"Done","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"092d9da57336533aa3a6bb436798c538372ec67b","unresolved":false,"context_lines":[{"line_number":668,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example of we have"},{"line_number":669,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        However, it\u0027s possible that the some L3HARouterAgentPortBinding is"},{"line_number":672,"context_line":"        created but isn\u0027t assigned to an l3 agent (and is fact \u0027dead in the"},{"line_number":673,"context_line":"        water\u0027). in this case, we should return that binding\u0027s index with an"},{"line_number":674,"context_line":"        indication that it already exists (trying to re-create a binding with"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_b1264366","line":671,"range":{"start_line":671,"start_character":36,"end_line":671,"end_character":39},"updated":"2016-09-01 12:00:42.000000000","message":"nit: remove","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":668,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example of we have"},{"line_number":669,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        However, it\u0027s possible that the some L3HARouterAgentPortBinding is"},{"line_number":672,"context_line":"        created but isn\u0027t assigned to an l3 agent (and is fact \u0027dead in the"},{"line_number":673,"context_line":"        water\u0027). in this case, we should return that binding\u0027s index with an"},{"line_number":674,"context_line":"        indication that it already exists (trying to re-create a binding with"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_823acb96","line":671,"range":{"start_line":671,"start_character":36,"end_line":671,"end_character":39},"in_reply_to":"fa7ab95a_b1264366","updated":"2016-09-01 12:44:46.000000000","message":"Done","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"092d9da57336533aa3a6bb436798c538372ec67b","unresolved":false,"context_lines":[{"line_number":699,"context_line":"            return open_slots[0], False"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":"        # Last chance: if this is a manual scheduling, we\u0027re gonna allow"},{"line_number":702,"context_line":"        # creation of a binding_index even if it will exceed"},{"line_number":703,"context_line":"        # max_l3_agents_per_router."},{"line_number":704,"context_line":"        if is_manual_scheduling:"},{"line_number":705,"context_line":"            return max(all_indicies) + 1, False"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_d1b3c7e7","line":703,"range":{"start_line":702,"start_character":46,"end_line":703,"end_character":34},"updated":"2016-09-01 12:00:42.000000000","message":"is it allowed now? (or a behavior change)","commit_id":"8619abb837ab367842324df1d1959a426feef5da"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"470187b631dcb7f6c15fbe33874388475b461620","unresolved":false,"context_lines":[{"line_number":699,"context_line":"            return open_slots[0], False"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":"        # Last chance: if this is a manual scheduling, we\u0027re gonna allow"},{"line_number":702,"context_line":"        # creation of a binding_index even if it will exceed"},{"line_number":703,"context_line":"        # max_l3_agents_per_router."},{"line_number":704,"context_line":"        if is_manual_scheduling:"},{"line_number":705,"context_line":"            return max(all_indicies) + 1, False"},{"line_number":706,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa7ab95a_226d7f98","line":703,"range":{"start_line":702,"start_character":46,"end_line":703,"end_character":34},"in_reply_to":"fa7ab95a_d1b3c7e7","updated":"2016-09-01 12:44:46.000000000","message":"Yes, it is was allowed before (and will be allowed following this). No behaviour change is made here.","commit_id":"8619abb837ab367842324df1d1959a426feef5da"}],"neutron/db/migration/alembic_migrations/versions/newton/contract/95877c4c257a_add_binding_index_to_l3harouteragentportbinding.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a35e8cd6a85acb018095e4db147d62b672f65156","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        for ha_bindings in routers_to_ha_bindings.values():"},{"line_number":62,"context_line":"            for index, result in enumerate(ha_bindings):"},{"line_number":63,"context_line":"                session.execute(ha_bindings_table.update().values("},{"line_number":64,"context_line":"                    binding_index\u003dindex).where("},{"line_number":65,"context_line":"                    ha_bindings_table.c.router_id \u003d\u003d result.router_id).where("},{"line_number":66,"context_line":"                    ha_bindings_table.c.port_id \u003d\u003d result.port_id))"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_940ccd43","line":63,"updated":"2016-08-25 09:51:01.000000000","message":"Such migration needs test with data. Like this https://github.com/openstack/neutron/blob/master/neutron/tests/functional/db/migrations/test_a8b517cff8ab_add_routerport_bindings_for_ha.py","commit_id":"eff76af21235296cfe8a0ba9a6f817e56747d3aa"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"2c23019e462df6369f5e2208fcf7173ff8090c9e","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        for ha_bindings in routers_to_ha_bindings.values():"},{"line_number":62,"context_line":"            for index, result in enumerate(ha_bindings):"},{"line_number":63,"context_line":"                session.execute(ha_bindings_table.update().values("},{"line_number":64,"context_line":"                    binding_index\u003dindex).where("},{"line_number":65,"context_line":"                    ha_bindings_table.c.router_id \u003d\u003d result.router_id).where("},{"line_number":66,"context_line":"                    ha_bindings_table.c.port_id \u003d\u003d result.port_id))"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_f125465c","line":63,"in_reply_to":"1ac06dbe_940ccd43","updated":"2016-08-25 12:30:12.000000000","message":"So actually, the data added by [1] is being tested by this migration anyway. I know this for a fact because [1] failed locally until I fixed this up :)\n\nIf you still think we need more tests here though, I\u0027ll happily oblige.\n\n[1]: https://github.com/openstack/neutron/blob/master/neutron/tests/functional/db/migrations/test_a8b517cff8ab_add_routerport_bindings_for_ha.py","commit_id":"eff76af21235296cfe8a0ba9a6f817e56747d3aa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":"#"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"\"\"\"Add binding index to L3HARouterAgentPortBinding"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Revision ID: 95877c4c257a"},{"line_number":19,"context_line":"Revises: 3b935b28e7a0"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_0b46d0be","line":16,"range":{"start_line":16,"start_character":3,"end_line":16,"end_character":6},"updated":"2016-08-29 08:09:58.000000000","message":"ditto","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def upgrade():"},{"line_number":49,"context_line":"    op.add_column(HA_AGENT_BINDINGS,"},{"line_number":50,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":51,"context_line":"                           server_default\u003d\u00270\u0027))"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_ebae0c84","line":49,"range":{"start_line":49,"start_character":7,"end_line":49,"end_character":17},"updated":"2016-08-29 08:09:58.000000000","message":"Move to expand folder seems more proper.\nSome refs [1], [2]:\n\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/db/migration/alembic_migrations/versions/newton/expand/030a959ceafa_uniq_routerports0port_id.py\n\n[2] https://github.com/openstack/neutron/blob/master/neutron/db/migration/alembic_migrations/versions/mitaka/expand/1df244e556f5_add_unique_ha_router_agent_port_bindings.py","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b2d8e551a97da23f55e5ec57b37519b81de14c08","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def upgrade():"},{"line_number":49,"context_line":"    op.add_column(HA_AGENT_BINDINGS,"},{"line_number":50,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":51,"context_line":"                           server_default\u003d\u00270\u0027))"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_59a264fe","line":49,"range":{"start_line":49,"start_character":7,"end_line":49,"end_character":17},"in_reply_to":"fa7ab95a_55078e17","updated":"2016-08-29 14:25:50.000000000","message":"John is correct this migration should be in contract branch.","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"214003f68286a3c7b8422648788cbf9fab753369","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def upgrade():"},{"line_number":49,"context_line":"    op.add_column(HA_AGENT_BINDINGS,"},{"line_number":50,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":51,"context_line":"                           server_default\u003d\u00270\u0027))"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_55078e17","line":49,"range":{"start_line":49,"start_character":7,"end_line":49,"end_character":17},"in_reply_to":"fa7ab95a_ebae0c84","updated":"2016-08-29 10:23:50.000000000","message":"Actually, as per a conversation with Ann (and as per documentation) this should be in contract. The reason is that expand scripts can run while the server is still running (as the server won\u0027t crash if you add a new table, etc), whereas this script will crash the server when the second L3HARouterAgentPortBinding is created (since binding_index won\u0027t exist as a parameter and will get the value 0 which was already created), so it belongs in the contract folder.","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":77,"context_line":"                    ha_bindings_table.c.port_id \u003d\u003d result.port_id))"},{"line_number":78,"context_line":"    session.commit()"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    op.create_unique_constraint("},{"line_number":81,"context_line":"        \u0027uniq_ha_router_agent_port_bindings0router_id0binding_id0\u0027,"},{"line_number":82,"context_line":"        \u0027ha_router_agent_port_bindings\u0027, [\u0027router_id\u0027, \u0027binding_index\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_4bba98c1","line":80,"range":{"start_line":80,"start_character":7,"end_line":80,"end_character":31},"updated":"2016-08-29 08:09:58.000000000","message":"ditto","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"}],"neutron/db/migration/alembic_migrations/versions/newton/expand/95877c4c257a_add_binding_index_to_l3harouteragentportbinding.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9295cfc07c1953c36e0d0f30b45d26058fc02f64","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def upgrade():"},{"line_number":34,"context_line":"    ### commands auto generated by Alembic - please adjust! ###"},{"line_number":35,"context_line":"    op.add_column(\u0027ha_router_agent_port_bindings\u0027, sa.Column(\u0027binding_id\u0027, sa.Integer(), nullable\u003dFalse))"},{"line_number":36,"context_line":"    op.create_unique_constraint(\u0027uniq_ha_router_agent_port_bindings0router_id0binding_id0\u0027, \u0027ha_router_agent_port_bindings\u0027, [\u0027router_id\u0027, \u0027binding_id\u0027])"},{"line_number":37,"context_line":"    ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_b8b441d1","line":34,"updated":"2016-08-22 12:01:57.000000000","message":"We also need to update all existing bindings tables and set binding_id there as well. That would be contract migration.","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"076e38c920fe789cc64407e7f8bb6e3222ca1a4b","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def upgrade():"},{"line_number":34,"context_line":"    ### commands auto generated by Alembic - please adjust! ###"},{"line_number":35,"context_line":"    op.add_column(\u0027ha_router_agent_port_bindings\u0027, sa.Column(\u0027binding_id\u0027, sa.Integer(), nullable\u003dFalse))"},{"line_number":36,"context_line":"    op.create_unique_constraint(\u0027uniq_ha_router_agent_port_bindings0router_id0binding_id0\u0027, \u0027ha_router_agent_port_bindings\u0027, [\u0027router_id\u0027, \u0027binding_id\u0027])"},{"line_number":37,"context_line":"    ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_e4457470","line":34,"in_reply_to":"1ac06dbe_b8b441d1","updated":"2016-08-22 12:12:50.000000000","message":"I actually had a piece of code that updated the bindings, but it disappeared somewhere on the etherweb :\u003c\n\nAlready rewrote and will upload soon.","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"}],"neutron/extensions/l3.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0c617444106dd9f700c85344dbda645ba16d7d96","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                \"gateway to external network %(net_id)s is required by one or \""},{"line_number":83,"context_line":"                \"more floating IPs.\")"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"ROUTERS \u003d \u0027routers\u0027"},{"line_number":87,"context_line":"FLOATINGIP \u003d \u0027floatingip\u0027"},{"line_number":88,"context_line":"FLOATINGIPS \u003d \u0027%ss\u0027 % FLOATINGIP"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_4c0b2e2c","line":85,"updated":"2016-08-24 13:53:28.000000000","message":"please drop this - one less file to review :)","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9c554b5f409b79aec754ba6ed171635364652f8f","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                \"gateway to external network %(net_id)s is required by one or \""},{"line_number":83,"context_line":"                \"more floating IPs.\")"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"ROUTERS \u003d \u0027routers\u0027"},{"line_number":87,"context_line":"FLOATINGIP \u003d \u0027floatingip\u0027"},{"line_number":88,"context_line":"FLOATINGIPS \u003d \u0027%ss\u0027 % FLOATINGIP"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_e5a82dde","line":85,"in_reply_to":"1ac06dbe_4c0b2e2c","updated":"2016-08-24 14:18:43.000000000","message":"Oops :)","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"}],"neutron/scheduler/l3_agent_scheduler.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9d2c02afd7849b2e9147f9ebd895104f23ad88de","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                                tenant_id, agent):"},{"line_number":276,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":277,"context_line":"        binding_index \u003d ("},{"line_number":278,"context_line":"            plugin.get_highest_binding_index(context, router_id) + 1)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        ctxt \u003d context.elevated()"},{"line_number":281,"context_line":"        creator \u003d functools.partial(self._add_port_from_net,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_3eaa46d5","line":278,"range":{"start_line":278,"start_character":12,"end_line":278,"end_character":45},"updated":"2016-08-19 15:29:12.000000000","message":"Specifically, this is bad logic since we should look for a missing index somewhere in the [0, \u003cmax_l3_agents\u003e - 1] closed section, and not go over the top.","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"9295cfc07c1953c36e0d0f30b45d26058fc02f64","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                                tenant_id, agent):"},{"line_number":276,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":277,"context_line":"        binding_index \u003d ("},{"line_number":278,"context_line":"            plugin.get_highest_binding_index(context, router_id) + 1)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        ctxt \u003d context.elevated()"},{"line_number":281,"context_line":"        creator \u003d functools.partial(self._add_port_from_net,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_de7da56f","line":278,"range":{"start_line":278,"start_character":12,"end_line":278,"end_character":45},"in_reply_to":"1ac06dbe_3eaa46d5","updated":"2016-08-22 12:01:57.000000000","message":"I think this index should start from 1 to max_l3_agent.","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"076e38c920fe789cc64407e7f8bb6e3222ca1a4b","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                                tenant_id, agent):"},{"line_number":276,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":277,"context_line":"        binding_index \u003d ("},{"line_number":278,"context_line":"            plugin.get_highest_binding_index(context, router_id) + 1)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        ctxt \u003d context.elevated()"},{"line_number":281,"context_line":"        creator \u003d functools.partial(self._add_port_from_net,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ac06dbe_045b7055","line":278,"range":{"start_line":278,"start_character":12,"end_line":278,"end_character":45},"in_reply_to":"1ac06dbe_de7da56f","updated":"2016-08-22 12:12:50.000000000","message":"OK, will do :)","commit_id":"86a3f7979e31286ec64a82b9dcce6736354b1aad"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0c617444106dd9f700c85344dbda645ba16d7d96","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":301,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def create_ha_port_and_bind(self, plugin, context, router_id,"},{"line_number":304,"context_line":"                                tenant_id, agent, is_manual_scheduling\u003dFalse):"},{"line_number":305,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":306,"context_line":"        for attempts in range(1, db_api.MAX_RETRIES + 1):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_6f5edc13","line":303,"updated":"2016-08-24 13:53:28.000000000","message":"why the return code? please add explanations","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9c554b5f409b79aec754ba6ed171635364652f8f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        with db_api.autonested_transaction(context.session):"},{"line_number":301,"context_line":"            port_binding.l3_agent_id \u003d agent[\u0027id\u0027]"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def create_ha_port_and_bind(self, plugin, context, router_id,"},{"line_number":304,"context_line":"                                tenant_id, agent, is_manual_scheduling\u003dFalse):"},{"line_number":305,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":306,"context_line":"        for attempts in range(1, db_api.MAX_RETRIES + 1):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ac06dbe_e5f66df8","line":303,"in_reply_to":"1ac06dbe_6f5edc13","updated":"2016-08-24 14:18:43.000000000","message":"Will find a way to remove the return codes. This is bad, I agree.","commit_id":"63f91c6293b4246df4d88edc8a3cb4e15e7a2e26"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        Do not schedule the routers which are hosted already"},{"line_number":134,"context_line":"        by active l3 agents."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        :returns: True if routers have been successfully assigned to host"},{"line_number":137,"context_line":"        \"\"\""},{"line_number":138,"context_line":"        l3_agent \u003d plugin.get_enabled_agent_on_host("},{"line_number":139,"context_line":"            context, lib_const.AGENT_TYPE_L3, host)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_cb64e8e6","side":"PARENT","line":136,"updated":"2016-08-29 08:09:58.000000000","message":"They do like some unrelated change like this.","commit_id":"14ebfb527dfa8631db0112f605927449584050ad"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"214003f68286a3c7b8422648788cbf9fab753369","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        Do not schedule the routers which are hosted already"},{"line_number":134,"context_line":"        by active l3 agents."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        :returns: True if routers have been successfully assigned to host"},{"line_number":137,"context_line":"        \"\"\""},{"line_number":138,"context_line":"        l3_agent \u003d plugin.get_enabled_agent_on_host("},{"line_number":139,"context_line":"            context, lib_const.AGENT_TYPE_L3, host)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_15eb4614","side":"PARENT","line":136,"in_reply_to":"fa7ab95a_cb64e8e6","updated":"2016-08-29 10:23:50.000000000","message":"Yes, it\u0027s unrelated, but there are 2 points for making it here: the first is that create_ha_port_and_bind is a lot more complicated with these, and the second (main) point is that we are nearing the N-3 milestone and I want to get this in. Adding even more patches will only make this unlikely to merge.","commit_id":"14ebfb527dfa8631db0112f605927449584050ad"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        l3_agent \u003d plugin.get_enabled_agent_on_host("},{"line_number":139,"context_line":"            context, lib_const.AGENT_TYPE_L3, host)"},{"line_number":140,"context_line":"        if not l3_agent:"},{"line_number":141,"context_line":"            return False"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        unscheduled_routers \u003d self._get_routers_to_schedule("},{"line_number":144,"context_line":"            context, plugin, router_ids)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_eb636cda","side":"PARENT","line":141,"updated":"2016-08-29 08:09:58.000000000","message":"ditto","commit_id":"14ebfb527dfa8631db0112f605927449584050ad"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6e3734e02241662ad950807eed64f0b4e3622dc0","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    def create_ha_port_and_bind(self, plugin, context, router_id,"},{"line_number":305,"context_line":"                                tenant_id, agent, is_manual_scheduling\u003dFalse):"},{"line_number":306,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":307,"context_line":"        for attempts in range(1, db_api.MAX_RETRIES + 1):"},{"line_number":308,"context_line":"            binding_index, does_exist \u003d plugin.get_vacant_binding_index("},{"line_number":309,"context_line":"                context, router_id, is_manual_scheduling)"},{"line_number":310,"context_line":"            if binding_index \u003d\u003d -1:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_0e489e5a","line":307,"range":{"start_line":307,"start_character":12,"end_line":307,"end_character":57},"updated":"2016-08-29 08:09:58.000000000","message":"attempts is not used below, then the range is also needed start at 1 ?","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"214003f68286a3c7b8422648788cbf9fab753369","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    def create_ha_port_and_bind(self, plugin, context, router_id,"},{"line_number":305,"context_line":"                                tenant_id, agent, is_manual_scheduling\u003dFalse):"},{"line_number":306,"context_line":"        \"\"\"Creates and binds a new HA port for this agent.\"\"\""},{"line_number":307,"context_line":"        for attempts in range(1, db_api.MAX_RETRIES + 1):"},{"line_number":308,"context_line":"            binding_index, does_exist \u003d plugin.get_vacant_binding_index("},{"line_number":309,"context_line":"                context, router_id, is_manual_scheduling)"},{"line_number":310,"context_line":"            if binding_index \u003d\u003d -1:"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_f55d0216","line":307,"range":{"start_line":307,"start_character":12,"end_line":307,"end_character":57},"in_reply_to":"fa7ab95a_0e489e5a","updated":"2016-08-29 10:23:50.000000000","message":"This is deliberate, as per [1] (consistency).\n\n[1]: https://github.com/openstack/neutron/blob/d1227c2ff8774b913e51a44ceecaed417751f1a6/neutron/common/utils.py#L553","commit_id":"11236c6cccd31d1e40f63ab828639a0ee4ddb393"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7831ad711a3721923ebe0d58bc7408e54536ff95","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        creator \u003d functools.partial(self._add_port_from_net,"},{"line_number":281,"context_line":"                                    plugin, ctxt, router_id,"},{"line_number":282,"context_line":"                                    binding_index, tenant_id)"},{"line_number":283,"context_line":"        dep_getter \u003d functools.partial(plugin.get_ha_network, ctxt,"},{"line_number":284,"context_line":"                                       tenant_id)"},{"line_number":285,"context_line":"        dep_creator \u003d functools.partial(plugin._create_ha_network,"},{"line_number":286,"context_line":"                                        ctxt, tenant_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_d09a9a6d","line":283,"updated":"2016-08-30 00:21:15.000000000","message":"it looks like this line an L287 didn\u0027t change? Try to avoid adding extra changes to make it easier to review.","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"96113e2fda3ba3479b3e3505ca050e2e34d8c1b9","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        creator \u003d functools.partial(self._add_port_from_net,"},{"line_number":281,"context_line":"                                    plugin, ctxt, router_id,"},{"line_number":282,"context_line":"                                    binding_index, tenant_id)"},{"line_number":283,"context_line":"        dep_getter \u003d functools.partial(plugin.get_ha_network, ctxt,"},{"line_number":284,"context_line":"                                       tenant_id)"},{"line_number":285,"context_line":"        dep_creator \u003d functools.partial(plugin._create_ha_network,"},{"line_number":286,"context_line":"                                        ctxt, tenant_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_a2df6538","line":283,"in_reply_to":"fa7ab95a_d09a9a6d","updated":"2016-08-30 13:05:12.000000000","message":"Agreed - I overlooked this one.","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7831ad711a3721923ebe0d58bc7408e54536ff95","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                    if not self._create_ha_port_and_bind("},{"line_number":320,"context_line":"                        plugin, context, router_id,"},{"line_number":321,"context_line":"                        binding_index, tenant_id, agent):"},{"line_number":322,"context_line":"                        continue"},{"line_number":323,"context_line":"                self.bind_router(context, router_id, agent)"},{"line_number":324,"context_line":"                return"},{"line_number":325,"context_line":"            except (db_exc.DBDuplicateEntry, exc.StaleDataError):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_f0895e27","line":322,"updated":"2016-08-30 00:21:15.000000000","message":"nit: move the \u0027continue\u0027 over another 4 spaces so it doesn\u0027t line up with the line-wrapped args right above it.","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"96113e2fda3ba3479b3e3505ca050e2e34d8c1b9","unresolved":false,"context_lines":[{"line_number":319,"context_line":"                    if not self._create_ha_port_and_bind("},{"line_number":320,"context_line":"                        plugin, context, router_id,"},{"line_number":321,"context_line":"                        binding_index, tenant_id, agent):"},{"line_number":322,"context_line":"                        continue"},{"line_number":323,"context_line":"                self.bind_router(context, router_id, agent)"},{"line_number":324,"context_line":"                return"},{"line_number":325,"context_line":"            except (db_exc.DBDuplicateEntry, exc.StaleDataError):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_a29be564","line":322,"in_reply_to":"fa7ab95a_f0895e27","updated":"2016-08-30 13:05:12.000000000","message":"Didn\u0027t understand exactly what you meant (indenting with 4 more spaces? adding a space line for the \u0027continue\u0027?)\nInstead, I assigned the return value to a variable and checked that instead (this also improves readability).","commit_id":"c88afd239878edbbe29f04cc5085f6a2eaf28dc3"}],"neutron/tests/unit/db/test_l3_hamode_db.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"48e696f067ecec26c6f75b06efb261ba28d01312","unresolved":false,"context_lines":[{"line_number":988,"context_line":""},{"line_number":989,"context_line":""},{"line_number":990,"context_line":"class L3HAModeDbTestCase(L3HATestFramework):"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"    def _create_network(self, plugin, ctx, name\u003d\u0027net\u0027,"},{"line_number":993,"context_line":"                        tenant_id\u003d\u0027tenant1\u0027, external\u003dFalse):"},{"line_number":994,"context_line":"        network \u003d {\u0027network\u0027: {\u0027name\u0027: name,"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_76603c36","side":"PARENT","line":991,"updated":"2016-09-01 09:04:25.000000000","message":":(","commit_id":"b293bc2e44bf9ab23d9b95a5497316aafa4acf29"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1fc766193cd91be206a61cf29c8196d3e99de854","unresolved":false,"context_lines":[{"line_number":988,"context_line":""},{"line_number":989,"context_line":""},{"line_number":990,"context_line":"class L3HAModeDbTestCase(L3HATestFramework):"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"    def _create_network(self, plugin, ctx, name\u003d\u0027net\u0027,"},{"line_number":993,"context_line":"                        tenant_id\u003d\u0027tenant1\u0027, external\u003dFalse):"},{"line_number":994,"context_line":"        network \u003d {\u0027network\u0027: {\u0027name\u0027: name,"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_1992c39f","side":"PARENT","line":991,"in_reply_to":"fa7ab95a_76603c36","updated":"2016-09-01 09:10:57.000000000","message":"This might actually get me a death penalty.","commit_id":"b293bc2e44bf9ab23d9b95a5497316aafa4acf29"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"48e696f067ecec26c6f75b06efb261ba28d01312","unresolved":false,"context_lines":[{"line_number":1281,"context_line":"                # binding so we should always provide a new one."},{"line_number":1282,"context_line":"                self.assertEqual(cfg.CONF.max_l3_agents_per_router + 1,"},{"line_number":1283,"context_line":"                                 vacant_binding_index)"},{"line_number":1284,"context_line":"                self.assertFalse(does_exist)"},{"line_number":1285,"context_line":"            else:"},{"line_number":1286,"context_line":"                # Else, we already have 3 so -1 is the \u0027error\u0027 value."},{"line_number":1287,"context_line":"                self.assertEqual(-1, vacant_binding_index)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_f6252cc2","line":1284,"updated":"2016-09-01 09:04:25.000000000","message":"If this and 1288 are indeed the same, move them outside of the if/else","commit_id":"74283725e680737b49ef5408df8417aa11921e88"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"183e16b41e4ce0bab9af133ee8e0c1f4e31e9d8f","unresolved":false,"context_lines":[{"line_number":1281,"context_line":"                # binding so we should always provide a new one."},{"line_number":1282,"context_line":"                self.assertEqual(cfg.CONF.max_l3_agents_per_router + 1,"},{"line_number":1283,"context_line":"                                 vacant_binding_index)"},{"line_number":1284,"context_line":"                self.assertFalse(does_exist)"},{"line_number":1285,"context_line":"            else:"},{"line_number":1286,"context_line":"                # Else, we already have 3 so -1 is the \u0027error\u0027 value."},{"line_number":1287,"context_line":"                self.assertEqual(-1, vacant_binding_index)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_994df39b","line":1284,"in_reply_to":"fa7ab95a_b90f17e0","updated":"2016-09-01 09:20:09.000000000","message":"Actually not a bug after all (I misread that code). Will fix the duplicity though :)","commit_id":"74283725e680737b49ef5408df8417aa11921e88"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1fc766193cd91be206a61cf29c8196d3e99de854","unresolved":false,"context_lines":[{"line_number":1281,"context_line":"                # binding so we should always provide a new one."},{"line_number":1282,"context_line":"                self.assertEqual(cfg.CONF.max_l3_agents_per_router + 1,"},{"line_number":1283,"context_line":"                                 vacant_binding_index)"},{"line_number":1284,"context_line":"                self.assertFalse(does_exist)"},{"line_number":1285,"context_line":"            else:"},{"line_number":1286,"context_line":"                # Else, we already have 3 so -1 is the \u0027error\u0027 value."},{"line_number":1287,"context_line":"                self.assertEqual(-1, vacant_binding_index)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_b90f17e0","line":1284,"in_reply_to":"fa7ab95a_f6252cc2","updated":"2016-09-01 09:10:57.000000000","message":"This is a bug - the returned value should be True. Must have gone someplace in a rebase or something.","commit_id":"74283725e680737b49ef5408df8417aa11921e88"}]}