)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9ccca51c61e81d0263a9d77f2d075c6435475296","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_agents_per_network in neutron.conf. This prevents"},{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1a1ced50_3620d949","line":19,"range":{"start_line":19,"start_character":12,"end_line":19,"end_character":30},"updated":"2017-03-16 22:54:57.000000000","message":"manually schedules","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"ff82c69bd340251e02a646092cf4265b4f7c4c7b","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_agents_per_network in neutron.conf. This prevents"},{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1a1ced50_b29f675e","line":19,"range":{"start_line":19,"start_character":12,"end_line":19,"end_character":30},"in_reply_to":"1a1ced50_3620d949","updated":"2017-03-17 09:39:17.000000000","message":"Hmm, I am referring to users as plural form here.","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"9ccca51c61e81d0263a9d77f2d075c6435475296","unresolved":false,"context_lines":[{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I1bc3f8b69c337f7c1cf7375509a0da61def9baf1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1a1ced50_761a5119","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":6},"updated":"2017-03-16 22:54:57.000000000","message":"agent","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"ff82c69bd340251e02a646092cf4265b4f7c4c7b","unresolved":false,"context_lines":[{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I1bc3f8b69c337f7c1cf7375509a0da61def9baf1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1a1ced50_32829701","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":6},"in_reply_to":"1a1ced50_761a5119","updated":"2017-03-17 09:39:17.000000000","message":"Users can specify multiple DHCP agents to one network.","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lujin \u003cluo.lujin@jp.fujitsu.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-05-26 11:03:05 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add binding_index to NetworkDhcpAgentBinding"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The patch proposes adding a new binding_index to the"},{"line_number":10,"context_line":"NetworkDhcpAgentBinding table, with an additional Unique"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3f1d235d_6a7bbcbd","line":7,"updated":"2017-07-11 20:37:44.000000000","message":"The design of the fix seems to assume that there is no point in time when old neutron-servers run at the same time as new neutron-servers. This is not in line with the work around rolling-upgrades.\n\nI am also not sure what\u0027s the point of having unique indices per se. I wonder if we could somehow use revision numbers on agent models instead, in a way that would make binding updates trigger revision number bump, and so that each binding update expects a specific revision number on corresponding agent models. In that way, no two parallel threads should be able to create new bindings without resyncing their session with database. What do you think?","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lujin \u003cluo.lujin@jp.fujitsu.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-05-26 11:03:05 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add binding_index to NetworkDhcpAgentBinding"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The patch proposes adding a new binding_index to the"},{"line_number":10,"context_line":"NetworkDhcpAgentBinding table, with an additional Unique"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"7faddb67_e8ebd2d9","line":7,"in_reply_to":"3f1d235d_6a7bbcbd","updated":"2019-08-19 12:34:50.000000000","message":"Yeah, seems the fix does not account for old servers running along with new ones. However I don\u0027t think it\u0027s an issue - it just means that the bug will be fixed only after all servers are upgraded (and no over-scheduled networks exist).","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_agents_per_network in neutron.conf. This prevents"},{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3f1d235d_e7f5c5ee","line":19,"range":{"start_line":19,"start_character":12,"end_line":19,"end_character":22},"updated":"2017-07-11 20:37:44.000000000","message":"manually","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_agents_per_network in neutron.conf. This prevents"},{"line_number":17,"context_line":"too many DHCP agents from being scheduled in the first place."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"2. If users mannually schedule a network to specific DHCP"},{"line_number":20,"context_line":"agents, the binding_index increments to show the number of"},{"line_number":21,"context_line":"DHCP agents hosting this network."},{"line_number":22,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"7faddb67_08e98ee1","line":19,"range":{"start_line":19,"start_character":12,"end_line":19,"end_character":22},"in_reply_to":"3f1d235d_e7f5c5ee","updated":"2019-08-19 12:34:50.000000000","message":"Done","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"}],"neutron/db/agentschedulers_db.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                    raise dhcpagentscheduler.NetworkHostedByDHCPAgent("},{"line_number":414,"context_line":"                        network_id\u003dnetwork_id, agent_id\u003did)"},{"line_number":415,"context_line":"            self.network_scheduler.resource_filter.bind("},{"line_number":416,"context_line":"                context, [agent_db], network_id, True)"},{"line_number":417,"context_line":"        dhcp_notifier \u003d self.agent_notifiers.get(constants.AGENT_TYPE_DHCP)"},{"line_number":418,"context_line":"        if dhcp_notifier:"},{"line_number":419,"context_line":"            dhcp_notifier.network_added_to_agent("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_67e1b52a","line":416,"range":{"start_line":416,"start_character":49,"end_line":416,"end_character":53},"updated":"2017-07-11 20:37:44.000000000","message":"nit: it may help with reading the code if you specify the name of the argument you set to True here.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":413,"context_line":"                    raise dhcpagentscheduler.NetworkHostedByDHCPAgent("},{"line_number":414,"context_line":"                        network_id\u003dnetwork_id, agent_id\u003did)"},{"line_number":415,"context_line":"            self.network_scheduler.resource_filter.bind("},{"line_number":416,"context_line":"                context, [agent_db], network_id, True)"},{"line_number":417,"context_line":"        dhcp_notifier \u003d self.agent_notifiers.get(constants.AGENT_TYPE_DHCP)"},{"line_number":418,"context_line":"        if dhcp_notifier:"},{"line_number":419,"context_line":"            dhcp_notifier.network_added_to_agent("}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_a8bb9ae2","line":416,"range":{"start_line":416,"start_character":49,"end_line":416,"end_character":53},"in_reply_to":"3f1d235d_67e1b52a","updated":"2019-08-19 12:34:50.000000000","message":"Done","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"}],"neutron/db/migration/alembic_migrations/versions/pike/contract/c3e9d13c4367_add_binding_index_to_.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"50458092d486859a12694062d8401b6bbafcb26d","unresolved":false,"context_lines":[{"line_number":46,"context_line":"def upgrade():"},{"line_number":47,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":48,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":49,"context_line":"                           server_default\u003d\u00271\u0027))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":52,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":16,"id":"7ffa3b31_ecdcde19","line":49,"updated":"2017-04-14 23:19:06.000000000","message":"we need the server default to auto increment to deal with older neutron servers that don\u0027t specify the index and need multiple agents. Otherwise we end up only supporting one DHCP agent per network for old servers after the expand script has been run.","commit_id":"1f82c5dbf1086729e47eb8e70cbb9d21abb2b43a"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"b64ab6f0094b845606a48c49f7346f6c07a60621","unresolved":false,"context_lines":[{"line_number":46,"context_line":"def upgrade():"},{"line_number":47,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":48,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":49,"context_line":"                           server_default\u003d\u00271\u0027))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":52,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":16,"id":"ff0f0b1f_eea739a7","line":49,"in_reply_to":"7ffa3b31_ecdcde19","updated":"2017-05-26 02:03:56.000000000","message":"Done. Thanks!","commit_id":"1f82c5dbf1086729e47eb8e70cbb9d21abb2b43a"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","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 NetworkDhcpAgentBindings"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Revision ID: c3e9d13c4367"},{"line_number":19,"context_line":"Revises: 5c85685d616d"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_8ae61844","line":16,"updated":"2017-07-11 20:37:44.000000000","message":"This is a contract script that is now forbidden in neutron. I think we can make it work if moved to expand branch though, see below why I think we can do it safely.\n\nThis script does three separate things:\n1 create a new binding_index attribute;\n2 fill in existing binding models with new values;\n3 enforce constraint.\n\nThe first item should be ok for expand branch. The only problem with it is that it\u0027s nullable\u003dFalse, which means existing bindings would not fulfill the column requirement. But this would be ok if we also move step 2 to the same script, like we have it in this patch set.\n\nThe second step should also be ok for expand branch. It would fill in existing bindings with index values, and new bindings would get new values by virtue of autoincrement\u003dTrue. Of course, there is a danger here that an old neutron-server will create a binding while we are populating the new attribute. For what I understand, autoincremented value may then clash with the migration script. If we move the migration code to expand, we better handle such errors gracefully (maybe retry a bunch of times, and handle bindings one network at a time.\n\nThe value of the third step is not clear to me, because we already have autoincrement\u003dTrue that will give us the needed uniqueness of indexes. But if it\u0027s just for code strictness, we can also have it in expand without issues.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","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 NetworkDhcpAgentBindings"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Revision ID: c3e9d13c4367"},{"line_number":19,"context_line":"Revises: 5c85685d616d"}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_68b12201","line":16,"in_reply_to":"3f1d235d_8ae61844","updated":"2019-08-19 12:34:50.000000000","message":"agree, this can go to expand branch","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def contract_creation_exceptions():"},{"line_number":38,"context_line":"    \"\"\"Add a new binding_index to ensure that no over-creation of the bindings"},{"line_number":39,"context_line":"    is possible."},{"line_number":40,"context_line":"    \"\"\""},{"line_number":41,"context_line":"    return {"},{"line_number":42,"context_line":"        sa.Column: [\u0027%s.binding_index\u0027 % NETWORK_DHCP_AGENT_BINDING]"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_e7438512","line":39,"updated":"2017-07-11 20:37:44.000000000","message":"if you go with an exception, you should explain here in a comment why you think it\u0027s safe to have it in contract phase","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def contract_creation_exceptions():"},{"line_number":38,"context_line":"    \"\"\"Add a new binding_index to ensure that no over-creation of the bindings"},{"line_number":39,"context_line":"    is possible."},{"line_number":40,"context_line":"    \"\"\""},{"line_number":41,"context_line":"    return {"},{"line_number":42,"context_line":"        sa.Column: [\u0027%s.binding_index\u0027 % NETWORK_DHCP_AGENT_BINDING]"}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_88b6def8","line":39,"in_reply_to":"3f1d235d_e7438512","updated":"2019-08-19 12:34:50.000000000","message":"will go to expand","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":46,"context_line":"def upgrade():"},{"line_number":47,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":48,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":49,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":52,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_4706b988","line":49,"updated":"2017-07-11 20:37:44.000000000","message":"it\u0027s autoincrement here but not in the db model definition. any reason why?","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":46,"context_line":"def upgrade():"},{"line_number":47,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":48,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":49,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":52,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_4891c651","line":49,"in_reply_to":"3f1d235d_4706b988","updated":"2019-08-19 12:34:50.000000000","message":"Seems need to add to model as well","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    networks_to_bindings \u003d defaultdict(list)"},{"line_number":61,"context_line":"    session \u003d sa.orm.Session(bind\u003dop.get_bind())"},{"line_number":62,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":63,"context_line":"        for result in session.query(bindings_table):"},{"line_number":64,"context_line":"            networks_to_bindings[result.network_id].append(result)"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_471a397d","line":62,"updated":"2017-07-11 20:37:44.000000000","message":"Old neutron-servers still running in the cluster will create new binding models without binding_index explicitly set, which will trigger autoincrement mechanism that will fill it in for them. It means that old neutron-servers will still be able to overcommit dhcp agents to networks. While there is no clear way to stop them from doing it on database level, and we probably can live with the limitation for the time of rolling upgrade, we may want to deal with those bindings afterwards. Or you think it\u0027s ok to leave those running?\n\nIf we decide to tackle that, we probably want to have code that would not only avoid scheduling new agents to a network like the current PS does when binding to a new agent, but that also would unschedule existing overcommitted agents.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    networks_to_bindings \u003d defaultdict(list)"},{"line_number":61,"context_line":"    session \u003d sa.orm.Session(bind\u003dop.get_bind())"},{"line_number":62,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":63,"context_line":"        for result in session.query(bindings_table):"},{"line_number":64,"context_line":"            networks_to_bindings[result.network_id].append(result)"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_88849e0c","line":62,"in_reply_to":"3f1d235d_471a397d","updated":"2019-08-19 12:34:50.000000000","message":"I think it\u0027s ok to consider fix is working only after all servers are upgraded. I\u0027d like to not overload the patch with unscheduling logic.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    session.commit()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    op.create_unique_constraint("},{"line_number":75,"context_line":"        \u0027uniq_network_dhcp_agent_binding0network_id0binding_index0\u0027,"},{"line_number":76,"context_line":"        NETWORK_DHCP_AGENT_BINDING, [\u0027network_id\u0027, \u0027binding_index\u0027])"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_277cdde9","line":75,"updated":"2017-07-11 20:37:44.000000000","message":"Am I right to read it as you are going to enforce it, but due to autoincrement\u003dTrue, old neutron-servers that don\u0027t have access to binding_index will still get it populated by server? If so, what\u0027s the point of the constraint?","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":72,"context_line":"    session.commit()"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    op.create_unique_constraint("},{"line_number":75,"context_line":"        \u0027uniq_network_dhcp_agent_binding0network_id0binding_index0\u0027,"},{"line_number":76,"context_line":"        NETWORK_DHCP_AGENT_BINDING, [\u0027network_id\u0027, \u0027binding_index\u0027])"}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_28a24a9b","line":75,"in_reply_to":"3f1d235d_277cdde9","updated":"2019-08-19 12:34:50.000000000","message":"The point is to fix the bug when all neutron servers are upgraded and all over-scheduled networks are handled (for example manually or by a script)","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"}],"neutron/db/migration/alembic_migrations/versions/train/expand/c3e9d13c4367_add_binding_index_to_.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9dfd730b03b561bae4b1c9685ec5b0cd574c58ae","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_5408987e","line":41,"range":{"start_line":41,"start_character":48,"end_line":41,"end_character":66},"updated":"2019-09-02 15:28:18.000000000","message":"is this really necessary? I don\u0027t see it later in db:\n\nmysql\u003e show create table networkdhcpagentbindings;\n+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+\n| Table                    | Create Table                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    |\n+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+\n| networkdhcpagentbindings | CREATE TABLE `networkdhcpagentbindings` (\n  `network_id` varchar(36) NOT NULL,\n  `dhcp_agent_id` varchar(36) NOT NULL,\n  `binding_index` int(11) NOT NULL DEFAULT \u00271\u0027,\n  PRIMARY KEY (`network_id`,`dhcp_agent_id`),\n  UNIQUE KEY `uniq_network_dhcp_agent_binding0network_id0binding_index0` (`network_id`,`binding_index`),\n  KEY `dhcp_agent_id` (`dhcp_agent_id`),\n  CONSTRAINT `networkdhcpagentbindings_ibfk_1` FOREIGN KEY (`dhcp_agent_id`) REFERENCES `agents` (`id`) ON DELETE CASCADE,\n  CONSTRAINT `networkdhcpagentbindings_ibfk_2` FOREIGN KEY (`network_id`) REFERENCES `networks` (`id`) ON DELETE CASCADE\n) ENGINE\u003dInnoDB DEFAULT CHARSET\u003dutf8 |\n+--------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"67816daa87f4db8fff3c90422c716e337c0e8fca","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_bf3750ff","line":41,"range":{"start_line":41,"start_character":43,"end_line":41,"end_character":46},"updated":"2019-09-16 07:43:55.000000000","message":"one more question: should default value be string if type of column is int?","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b82b8f46c7b21e28c425494c1b7da9143604546c","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3fa7e38b_1ab81bc6","line":41,"range":{"start_line":41,"start_character":48,"end_line":41,"end_character":66},"in_reply_to":"3fa7e38b_0f62263f","updated":"2019-10-28 11:01:51.000000000","message":"\"old servers are running with new ones\" - non-upgraded neutron server   (while DB is upgraded) will not set any value for the binding index which may cause constraint violation without autoincrement. Looks indeed this will not work if the column is not a PK.\nNot sure how test failures are related, I think we just need to add smth like https://github.com/openstack/neutron/blob/stable/train/neutron/tests/unit/objects/test_l3agent.py#L34 to DhcpAgentBinding object test.","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"eee79f86862f22a9f866f0c584ddcb256f8c8d15","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_bd1f5706","line":41,"range":{"start_line":41,"start_character":43,"end_line":41,"end_character":46},"in_reply_to":"5faad753_15fbe578","updated":"2019-09-16 10:02:24.000000000","message":"This setting is fine, it is still an integer value. You can check routerl3agentbindings table it also have 1 as server default and it is shown as \u00271\u0027 as well.\nIf you check explain command it will show 1 without quotes:\nmysql\u003e explain routerl3agentbindings;\n+---------------+-------------+------+-----+---------+-------+\n| Field         | Type        | Null | Key | Default | Extra |\n+---------------+-------------+------+-----+---------+-------+\n| router_id     | varchar(36) | NO   | PRI | NULL    |       |\n| l3_agent_id   | varchar(36) | NO   | PRI | NULL    |       |\n| binding_index | int(11)     | NO   |     | 1       |       |\n+---------------+-------------+------+-----+---------+-------+","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e17f5c4bbdde7cb6cd078c593a545bca210acb56","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_20d6dc29","line":41,"range":{"start_line":41,"start_character":43,"end_line":41,"end_character":46},"in_reply_to":"5faad753_bd1f5706","updated":"2019-09-16 11:06:32.000000000","message":"Ok, thx for confirmation Ann :)","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3c8edb7e98ac70966213f1bafb65d6daf8722a51","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_da8eda77","line":41,"range":{"start_line":41,"start_character":43,"end_line":41,"end_character":46},"in_reply_to":"5faad753_bf3750ff","updated":"2019-09-16 08:05:49.000000000","message":"This setting is correct, server_default is set in this way - it will be integer value https://docs.sqlalchemy.org/en/13/core/defaults.html#server-invoked-ddl-explicit-default-expressions","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"02125c9fada93570da3088ac05590cbf5c80b61f","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_15fbe578","line":41,"range":{"start_line":41,"start_character":43,"end_line":41,"end_character":46},"in_reply_to":"5faad753_da8eda77","updated":"2019-09-16 09:27:51.000000000","message":"@Ann, thx for explanation. Can You check my previous comment here? It looks for me that, at least in case of my test, it was set a bit differently as \u00271\u0027 instead of 1. But I\u0027m not DB expert at all, so maybe that is not a problem at all. Or maybe it is dependent on DB used (MySQL, MariaDB, etc.)?","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"6b158c96136cf3079d945ead0b9185531d85354d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"5faad753_193fbfb4","line":41,"range":{"start_line":41,"start_character":48,"end_line":41,"end_character":66},"in_reply_to":"7faddb67_35e4f3a7","updated":"2019-09-13 13:44:02.000000000","message":"I have the same result here as Slawek, the strange is that by documentation the autoincrement should be visible in the create table output.","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"018a5071c30a54ae455fc995b07d011787bdd155","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3fa7e38b_0f62263f","line":41,"range":{"start_line":41,"start_character":48,"end_line":41,"end_character":66},"in_reply_to":"7faddb67_35e4f3a7","updated":"2019-10-26 18:12:46.000000000","message":"Sorry for being very late for this review. The autoincrement parameter is only for integer primary key columns [1]. This is not a PK, and that\u0027s why why are experiencing some errors like [2][3].\n\nAlthough this is a bad praxis, because this patch was merged 1 day ago, we should remove this parameter.\n\nWhat do you mean by \"old server are running along with new ones\"?\n\n\n[1] https://docs.sqlalchemy.org/en/13/core/metadata.html#sqlalchemy.schema.Column.params.autoincrement\n[2] https://storage.gra1.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_2aa/690514/4/check/openstack-tox-py36/2aadb9c/testr_results.html.gz\n[3] https://cb9c7313dea10c2758bb-cbf7b900962d6b77d5144bf757270132.ssl.cf2.rackcdn.com/690514/4/gate/openstack-tox-py36/a9064c9/testr_results.html.gz","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"73e48460fcd07a980877dff6b378ccffa7b5cf1d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"def upgrade():"},{"line_number":39,"context_line":"    op.add_column(NETWORK_DHCP_AGENT_BINDING,"},{"line_number":40,"context_line":"                  sa.Column(\u0027binding_index\u0027, sa.Integer(), nullable\u003dFalse,"},{"line_number":41,"context_line":"                            server_default\u003d\u00271\u0027, autoincrement\u003dTrue))"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    bindings_table \u003d sa.Table("},{"line_number":44,"context_line":"        NETWORK_DHCP_AGENT_BINDING,"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_35e4f3a7","line":41,"range":{"start_line":41,"start_character":48,"end_line":41,"end_character":66},"in_reply_to":"7faddb67_5408987e","updated":"2019-09-03 07:04:09.000000000","message":"it\u0027s necessary for the case when old server are running along with new ones and with DB upgraded.\nI added this also to DB model, not sure why it\u0027s not shown in the above snippet..","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"}],"neutron/db/network_dhcp_agent_binding/models.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9dfd730b03b561bae4b1c9685ec5b0cd574c58ae","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from neutron.db.models import agent as agent_model"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"LOWEST_BINDING_INDEX \u003d 1"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class NetworkDhcpAgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_9477f008","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":20},"updated":"2019-09-02 15:28:18.000000000","message":"nit: maybe MIN_BINDING_INDEX would be better?","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"73e48460fcd07a980877dff6b378ccffa7b5cf1d","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from neutron.db.models import agent as agent_model"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"LOWEST_BINDING_INDEX \u003d 1"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class NetworkDhcpAgentBinding(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_55d08fc8","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":20},"in_reply_to":"7faddb67_9477f008","updated":"2019-09-03 07:04:09.000000000","message":"It\u0027s for consistency with neutron/db/models/l3agent.py","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9dfd730b03b561bae4b1c9685ec5b0cd574c58ae","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                              primary_key\u003dTrue)"},{"line_number":41,"context_line":"    binding_index \u003d sa.Column(sa.Integer, nullable\u003dFalse,"},{"line_number":42,"context_line":"                              server_default\u003dstr(LOWEST_BINDING_INDEX),"},{"line_number":43,"context_line":"                              autoincrement\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_1426a017","line":43,"range":{"start_line":43,"start_character":30,"end_line":43,"end_character":48},"updated":"2019-09-02 15:28:18.000000000","message":"same here","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"73e48460fcd07a980877dff6b378ccffa7b5cf1d","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                              primary_key\u003dTrue)"},{"line_number":41,"context_line":"    binding_index \u003d sa.Column(sa.Integer, nullable\u003dFalse,"},{"line_number":42,"context_line":"                              server_default\u003dstr(LOWEST_BINDING_INDEX),"},{"line_number":43,"context_line":"                              autoincrement\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_f5017b3a","line":43,"range":{"start_line":43,"start_character":30,"end_line":43,"end_character":48},"in_reply_to":"7faddb67_1426a017","updated":"2019-09-03 07:04:09.000000000","message":"it\u0027s necessary for the case when old server are running along with new ones and with DB upgraded.","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"}],"neutron/scheduler/dhcp_agent_scheduler.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a6682f17e8606dbddf8f95917f0052cb43927e57","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        # customize the bind logic"},{"line_number":171,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":172,"context_line":"        for agent in agents:"},{"line_number":173,"context_line":"            context.session.begin(subtransactions\u003dTrue)"},{"line_number":174,"context_line":"            # saving agent_id to use it after rollback to avoid"},{"line_number":175,"context_line":"            # DetachedInstanceError"},{"line_number":176,"context_line":"            agent_id \u003d agent.id"}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_d5e1059c","line":173,"range":{"start_line":173,"start_character":12,"end_line":173,"end_character":55},"updated":"2016-03-09 09:21:54.000000000","message":"Please, use context manager here, if exception occur, this transaction will hang.","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"fcd70a5e336cd1194b590cbec892c21574c0be33","unresolved":false,"context_lines":[{"line_number":170,"context_line":"        # customize the bind logic"},{"line_number":171,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":172,"context_line":"        for agent in agents:"},{"line_number":173,"context_line":"            context.session.begin(subtransactions\u003dTrue)"},{"line_number":174,"context_line":"            # saving agent_id to use it after rollback to avoid"},{"line_number":175,"context_line":"            # DetachedInstanceError"},{"line_number":176,"context_line":"            agent_id \u003d agent.id"}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_20ca246f","line":173,"range":{"start_line":173,"start_character":12,"end_line":173,"end_character":55},"in_reply_to":"5aef4532_d5e1059c","updated":"2016-03-11 06:37:52.000000000","message":"I understand that we need to use \"with\" here, because DBDuplicateEntry may not be the only exception. But if we use \"with\", I think we do not need to use rollback() manually, since it would be handled by \"with\" automatically when exception occurs.","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a6682f17e8606dbddf8f95917f0052cb43927e57","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                # try to actually write the changes and catch integrity"},{"line_number":183,"context_line":"                # DBDuplicateEntry"},{"line_number":184,"context_line":"                context.session.commit()"},{"line_number":185,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":186,"context_line":"                    agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"},{"line_number":187,"context_line":"                    query \u003d context.session.query("},{"line_number":188,"context_line":"                        agentschedulers_db.NetworkDhcpAgentBinding)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_756e3102","line":185,"range":{"start_line":185,"start_character":43,"end_line":185,"end_character":58},"updated":"2016-03-09 09:21:54.000000000","message":"use nested\u003dTrue here","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"fcd70a5e336cd1194b590cbec892c21574c0be33","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                # try to actually write the changes and catch integrity"},{"line_number":183,"context_line":"                # DBDuplicateEntry"},{"line_number":184,"context_line":"                context.session.commit()"},{"line_number":185,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":186,"context_line":"                    agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"},{"line_number":187,"context_line":"                    query \u003d context.session.query("},{"line_number":188,"context_line":"                        agentschedulers_db.NetworkDhcpAgentBinding)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_a0b914ce","line":185,"range":{"start_line":185,"start_character":43,"end_line":185,"end_character":58},"in_reply_to":"5aef4532_756e3102","updated":"2016-03-11 06:37:52.000000000","message":"My intention is to check the database again after ADD is committed not before that. If we use \"with\" in Line 173, I will have to move this checking part out of the session which starts from Line 173. That is to say, this is not a nested transaction.","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"a6682f17e8606dbddf8f95917f0052cb43927e57","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                                \u0027agent_id\u0027: agent_id})"},{"line_number":215,"context_line":"            except db_exc.DBDuplicateEntry:"},{"line_number":216,"context_line":"                # it\u0027s totally ok, someone just did our job!"},{"line_number":217,"context_line":"                context.session.rollback()"},{"line_number":218,"context_line":"                bound_agents.remove(agent)"},{"line_number":219,"context_line":"                LOG.info(_LI(\u0027Agent %s already present\u0027), agent_id)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_55ba156f","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":42},"updated":"2016-03-09 09:21:54.000000000","message":"Use rollback of transaction, not session.\n\n   with connection.begin() as trans:\n         trans.rollback()","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"fcd70a5e336cd1194b590cbec892c21574c0be33","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                                \u0027agent_id\u0027: agent_id})"},{"line_number":215,"context_line":"            except db_exc.DBDuplicateEntry:"},{"line_number":216,"context_line":"                # it\u0027s totally ok, someone just did our job!"},{"line_number":217,"context_line":"                context.session.rollback()"},{"line_number":218,"context_line":"                bound_agents.remove(agent)"},{"line_number":219,"context_line":"                LOG.info(_LI(\u0027Agent %s already present\u0027), agent_id)"},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5aef4532_20c6a449","line":217,"range":{"start_line":217,"start_character":16,"end_line":217,"end_character":42},"in_reply_to":"5aef4532_55ba156f","updated":"2016-03-11 06:37:52.000000000","message":"As mentioned above, if we use \"with\" in Line 173, I was wondering if we need to keep this rollback() manually here.","commit_id":"9db3514841479e6b42bd414be1380f78107be7d7"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d2268010f568afb288befc65f4c6e9f0daf54e1b","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":172,"context_line":"        for agent in agents:"},{"line_number":173,"context_line":"            try:"},{"line_number":174,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":175,"context_line":"                    # saving agent_id to use it after rollback to avoid"},{"line_number":176,"context_line":"                    # DetachedInstanceError"},{"line_number":177,"context_line":"                    agent_id \u003d agent.id"}],"source_content_type":"text/x-python","patch_set":3,"id":"3afc51ec_9fd71723","line":174,"range":{"start_line":174,"start_character":21,"end_line":174,"end_character":42},"updated":"2016-03-11 11:12:13.000000000","message":"I suggest to use autonested [1] here, as if there will external transaction,  DBDuplicateEntry will rollback both external and current transaction, probably it is good to create a special test on such situation.\n\n[1] - https://github.com/openstack/neutron/blob/a1d7bd40c02bbb9b2946eb9bc719e3cc482677a1/neutron/db/api.py#L82","commit_id":"df961fadb0e7aaa9bb24198fdbe1fb7239820473"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"a94ebfc0664549a20ef98b3c5487e93e3f8b9f89","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":172,"context_line":"        for agent in agents:"},{"line_number":173,"context_line":"            try:"},{"line_number":174,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":175,"context_line":"                    # saving agent_id to use it after rollback to avoid"},{"line_number":176,"context_line":"                    # DetachedInstanceError"},{"line_number":177,"context_line":"                    agent_id \u003d agent.id"}],"source_content_type":"text/x-python","patch_set":3,"id":"3afc51ec_14ec4c8e","line":174,"range":{"start_line":174,"start_character":21,"end_line":174,"end_character":42},"in_reply_to":"3afc51ec_9fd71723","updated":"2016-03-14 02:25:03.000000000","message":"Thanks Ann! This is very helpful info! I will change accordingly.","commit_id":"df961fadb0e7aaa9bb24198fdbe1fb7239820473"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"d2268010f568afb288befc65f4c6e9f0daf54e1b","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":187,"context_line":"                agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"},{"line_number":188,"context_line":"                query \u003d context.session.query("},{"line_number":189,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding)"},{"line_number":190,"context_line":"                query \u003d query.options(orm.contains_eager("},{"line_number":191,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.dhcp_agent))"},{"line_number":192,"context_line":"                query \u003d query.join("},{"line_number":193,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.dhcp_agent)"},{"line_number":194,"context_line":"                query \u003d query.filter("},{"line_number":195,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.network_id"},{"line_number":196,"context_line":"                    \u003d\u003d network_id)"},{"line_number":197,"context_line":"                existing_bindings \u003d []"},{"line_number":198,"context_line":"                for existing_alive_binding in query:"},{"line_number":199,"context_line":"                    query_agent \u003d context.session.query(agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef4532_09a90ce0","line":196,"range":{"start_line":188,"start_character":16,"end_line":196,"end_character":34},"updated":"2016-03-11 11:12:13.000000000","message":"May be I\u0027m missing something, why you can not do like this:\n\n    query \u003d context.session.query(\n            agentschedulers_db.NetworkDhcpAgentBinding).options(\n                    orm.joinedload(\u0027dhcp_agent\u0027)).filter(\n                    agentschedulers_db.NetworkDhcpAgentBinding.network_id \u003d\u003d network_id)\n\nAlso I think query can be moved out of the loop on top as query object is always the same.","commit_id":"df961fadb0e7aaa9bb24198fdbe1fb7239820473"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"a94ebfc0664549a20ef98b3c5487e93e3f8b9f89","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":187,"context_line":"                agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"},{"line_number":188,"context_line":"                query \u003d context.session.query("},{"line_number":189,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding)"},{"line_number":190,"context_line":"                query \u003d query.options(orm.contains_eager("},{"line_number":191,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.dhcp_agent))"},{"line_number":192,"context_line":"                query \u003d query.join("},{"line_number":193,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.dhcp_agent)"},{"line_number":194,"context_line":"                query \u003d query.filter("},{"line_number":195,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.network_id"},{"line_number":196,"context_line":"                    \u003d\u003d network_id)"},{"line_number":197,"context_line":"                existing_bindings \u003d []"},{"line_number":198,"context_line":"                for existing_alive_binding in query:"},{"line_number":199,"context_line":"                    query_agent \u003d context.session.query(agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3afc51ec_d4666428","line":196,"range":{"start_line":188,"start_character":16,"end_line":196,"end_character":34},"in_reply_to":"5aef4532_09a90ce0","updated":"2016-03-14 02:25:03.000000000","message":"Thank you for the catch, I would combine the multiple lines into one. \nAs for moving query out of the loop, I am afraid I might need to keep it here. I intended to get the latest query after every agent ADD, to make sure the ADD of this agent will not violate the auto-scheduling rule. If we move the query out of the loop, the query only happens once, which is not up-to-date.","commit_id":"df961fadb0e7aaa9bb24198fdbe1fb7239820473"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"c57764169d5fad53ad89630062b90606f6fe7e86","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                       \u0027agent_id\u0027: agent_id})"},{"line_number":167,"context_line":"        super(DhcpFilter, self).bind(context, bound_agents, network_id)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def bind_auto(self, context, agents, network_id):"},{"line_number":170,"context_line":"        \"\"\"Bind the network to the agents auto-schedule.\"\"\""},{"line_number":171,"context_line":"        # customize the bind logic"},{"line_number":172,"context_line":"        bound_agents \u003d agents[:]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_b8052912","line":169,"updated":"2016-03-17 21:29:56.000000000","message":"As far as I can see, DhcpFilter.bind() is no longer called outside of the tests. Is the intention to leave it as-is or remove it all together?","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"791d210ba5c70a437f4fdc3201eaad71ab288193","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                       \u0027agent_id\u0027: agent_id})"},{"line_number":167,"context_line":"        super(DhcpFilter, self).bind(context, bound_agents, network_id)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def bind_auto(self, context, agents, network_id):"},{"line_number":170,"context_line":"        \"\"\"Bind the network to the agents auto-schedule.\"\"\""},{"line_number":171,"context_line":"        # customize the bind logic"},{"line_number":172,"context_line":"        bound_agents \u003d agents[:]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_99717779","line":169,"in_reply_to":"1af94dfe_b8052912","updated":"2016-03-23 08:07:02.000000000","message":"This is my mistake. I had a wrong understanding of the structure of DhcpFilter.bind() before. I thought it would be in charge of manually add DHCP agents as well, rather than only auto-scheduling. I will apply my changes in DhcpFilter.bind_auto() to DhcpFilter.bind() and keep DhcpFilter.bind() only.","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"c57764169d5fad53ad89630062b90606f6fe7e86","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                    context.session.add(binding)"},{"line_number":183,"context_line":"            except db_exc.DBDuplicateEntry:"},{"line_number":184,"context_line":"                bound_agents.remove(agent)"},{"line_number":185,"context_line":"                LOG.info(_LI(\u0027Agent %s already present\u0027), agent_id)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":188,"context_line":"                agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_159d080a","line":185,"updated":"2016-03-17 21:29:56.000000000","message":"I\u0027m having a hard time understand this usage of LOG.info(), especially considering the network id isn\u0027t in the message. Did you perhaps want this to be debug?","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"791d210ba5c70a437f4fdc3201eaad71ab288193","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                    context.session.add(binding)"},{"line_number":183,"context_line":"            except db_exc.DBDuplicateEntry:"},{"line_number":184,"context_line":"                bound_agents.remove(agent)"},{"line_number":185,"context_line":"                LOG.info(_LI(\u0027Agent %s already present\u0027), agent_id)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":188,"context_line":"                agents_per_network \u003d cfg.CONF.dhcp_agents_per_network"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_b9971b47","line":185,"in_reply_to":"1af94dfe_159d080a","updated":"2016-03-23 08:07:02.000000000","message":"Actually, I inherited this part from DhcpFilter.bind(). The reason to use \u0027info\u0027 here is to test DBDuplicateEntry exception in test_dhcp_agent_scheduler.py in Line 108.","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"c57764169d5fad53ad89630062b90606f6fe7e86","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                    query_agent \u003d context.session.query(agents_db.Agent)"},{"line_number":197,"context_line":"                    existed_agent_id \u003d existing_alive_binding.dhcp_agent_id"},{"line_number":198,"context_line":"                    query_agent_alive \u003d query_agent.filter("},{"line_number":199,"context_line":"                        agents_db.Agent.id \u003d\u003d existed_agent_id).all()"},{"line_number":200,"context_line":"                    liveness \u003d query_agent_alive[0].heartbeat_timestamp"},{"line_number":201,"context_line":"                    if not agents_db.AgentDbMixin.is_agent_down(liveness):"},{"line_number":202,"context_line":"                        existing_bindings.append(existing_alive_binding)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_c3ef6a1c","line":199,"updated":"2016-03-17 21:29:56.000000000","message":"nit: couldn\u0027t you use one() here to save the index access in the next line?","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"791d210ba5c70a437f4fdc3201eaad71ab288193","unresolved":false,"context_lines":[{"line_number":196,"context_line":"                    query_agent \u003d context.session.query(agents_db.Agent)"},{"line_number":197,"context_line":"                    existed_agent_id \u003d existing_alive_binding.dhcp_agent_id"},{"line_number":198,"context_line":"                    query_agent_alive \u003d query_agent.filter("},{"line_number":199,"context_line":"                        agents_db.Agent.id \u003d\u003d existed_agent_id).all()"},{"line_number":200,"context_line":"                    liveness \u003d query_agent_alive[0].heartbeat_timestamp"},{"line_number":201,"context_line":"                    if not agents_db.AgentDbMixin.is_agent_down(liveness):"},{"line_number":202,"context_line":"                        existing_bindings.append(existing_alive_binding)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_398aeb1b","line":199,"in_reply_to":"1af94dfe_c3ef6a1c","updated":"2016-03-23 08:07:02.000000000","message":"Thanks for the catch. I will revise accordingly.","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"24edf6067d851c76a0d0d8ec09db414633cff2ea","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                    orm.joinedload(\u0027dhcp_agent\u0027)).filter("},{"line_number":167,"context_line":"                    agentschedulers_db.NetworkDhcpAgentBinding.network_id"},{"line_number":168,"context_line":"                    \u003d\u003d network_id)"},{"line_number":169,"context_line":"                existing_bindings \u003d []"},{"line_number":170,"context_line":"                for existing_alive_binding in query:"},{"line_number":171,"context_line":"                    query_agent \u003d context.session.query(agents_db.Agent)"},{"line_number":172,"context_line":"                    existed_agent_id \u003d existing_alive_binding.dhcp_agent_id"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_11de153f","line":169,"range":{"start_line":169,"start_character":36,"end_line":169,"end_character":38},"updated":"2016-06-07 03:28:48.000000000","message":"This logic isn\u0027t the responsibility of the \u0027bind\u0027 function. Bind is just supposed to bind the requested agents to the passed in network. This should probably be in auto_schedule_networks where agents_per_network logic already exists.","commit_id":"894ff4806209263d04105e24122ef202488826e1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e642263cec3c7e3172ffeb239e0b881fa854eef4","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":180,"context_line":"                    for existing_binding in \\"},{"line_number":181,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":182,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":183,"context_line":"                else:"},{"line_number":184,"context_line":"                    LOG.debug(\u0027Network %(network_id)s is scheduled to be \u0027"},{"line_number":185,"context_line":"                              \u0027hosted by DHCP agent %(agent_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3ac371cc_8da4b7b9","line":182,"updated":"2016-08-16 09:28:21.000000000","message":"I\u0027m not sure, what you suggested wasn\u0027t what I suggested at all. I\u0027m talking about deterministic cleanups, not determinstic additions.","commit_id":"894ff4806209263d04105e24122ef202488826e1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"24edf6067d851c76a0d0d8ec09db414633cff2ea","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":180,"context_line":"                    for existing_binding in \\"},{"line_number":181,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":182,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":183,"context_line":"                else:"},{"line_number":184,"context_line":"                    LOG.debug(\u0027Network %(network_id)s is scheduled to be \u0027"},{"line_number":185,"context_line":"                              \u0027hosted by DHCP agent %(agent_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_31e35107","line":182,"updated":"2016-06-07 03:28:48.000000000","message":"You will need to consider that two concurrent processes could be following this undo operation so this code needs to have a deterministic way that it is removing excessive agents (e.g. sorting these bindings by agent ID) so multiple don\u0027t remove different agents and leave the network with too few agents. This also means that you need to catch concurrent removal exceptions.","commit_id":"894ff4806209263d04105e24122ef202488826e1"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"f92a22776ee71e53989b1a1ba3b92d5c8724fad2","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":180,"context_line":"                    for existing_binding in \\"},{"line_number":181,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":182,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":183,"context_line":"                else:"},{"line_number":184,"context_line":"                    LOG.debug(\u0027Network %(network_id)s is scheduled to be \u0027"},{"line_number":185,"context_line":"                              \u0027hosted by DHCP agent %(agent_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3ac371cc_142052f6","line":182,"in_reply_to":"3ac371cc_8da4b7b9","updated":"2016-08-18 06:21:07.000000000","message":"I am sorry. I did not fully understand what you meant back then. If I understand it correctly now, what you suggested here was to sort the bindings which need to be deleted. That is to say, this sorting should happen after excessive dhcp agents have been associated with one network. If so, this is exactly what I am working on now. I will push a new PS soon.","commit_id":"894ff4806209263d04105e24122ef202488826e1"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"ead2dc3598183287c338ce7327accd3040365310","unresolved":false,"context_lines":[{"line_number":179,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":180,"context_line":"                    for existing_binding in \\"},{"line_number":181,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":182,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":183,"context_line":"                else:"},{"line_number":184,"context_line":"                    LOG.debug(\u0027Network %(network_id)s is scheduled to be \u0027"},{"line_number":185,"context_line":"                              \u0027hosted by DHCP agent %(agent_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_1259578e","line":182,"in_reply_to":"7aa08908_31e35107","updated":"2016-06-09 08:35:44.000000000","message":"\"sorting these bindings by agent ID\" This sentence just reminds me that there might be one easy solution. See Line 60, \"dhcp_agents\" are the query results which are available to host the network. If I do the sorting by agent ID here, then concurrent processes will be choosing the same dhcp agents to host this network. The latter session.add requests would be rejected by DBDuplicateEntry. \n\nBut I am concerning that this approach will only work if multiple dhcp agents are located on the same host (see Line 58). If multiple dhcp agents are distributed to multiple hosts, this approach cannot guarantee same agents are chosen. Some post clean-up is still required.","commit_id":"894ff4806209263d04105e24122ef202488826e1"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"710e472a4091d2dc8a5ac73bbfd190cc1f509cb4","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                 constants.AGENT_TYPE_DHCP,"},{"line_number":58,"context_line":"                                 agents_db.Agent.host \u003d\u003d host,"},{"line_number":59,"context_line":"                                 agents_db.Agent.admin_state_up \u003d\u003d sql.true()"},{"line_number":60,"context_line":"                                 ).order_by(agents_db.Agent.id.desc())"},{"line_number":61,"context_line":"            dhcp_agents \u003d query.all()"},{"line_number":62,"context_line":"            for dhcp_agent in dhcp_agents:"},{"line_number":63,"context_line":"                if agents_db.AgentDbMixin.is_agent_down("}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_2b77ae3c","line":60,"range":{"start_line":60,"start_character":44,"end_line":60,"end_character":69},"updated":"2016-06-15 10:58:35.000000000","message":"Is there any reason we need to sort by UUID?\nThis means that resources on an agent with the largest UUID in alphabetical order will be unassigned.\n\nAssume we have three dhcp agents and change num_agents_per_network from 3 to 2.\nAll networks will be hosted by the first two agents and the last DHCP agent will host nothing.","commit_id":"60ef49a6aa0c4d92a95dba93985a3291371f7562"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"24805e07d5b96d2f61619e32fb375f2269123ed2","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                                 constants.AGENT_TYPE_DHCP,"},{"line_number":58,"context_line":"                                 agents_db.Agent.host \u003d\u003d host,"},{"line_number":59,"context_line":"                                 agents_db.Agent.admin_state_up \u003d\u003d sql.true()"},{"line_number":60,"context_line":"                                 ).order_by(agents_db.Agent.id.desc())"},{"line_number":61,"context_line":"            dhcp_agents \u003d query.all()"},{"line_number":62,"context_line":"            for dhcp_agent in dhcp_agents:"},{"line_number":63,"context_line":"                if agents_db.AgentDbMixin.is_agent_down("}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_8650300c","line":60,"range":{"start_line":60,"start_character":44,"end_line":60,"end_character":69},"in_reply_to":"7aa08908_2b77ae3c","updated":"2016-06-16 00:24:41.000000000","message":"Sorry, I neglected the situation you mentioned. I will discard sorting by UUID here.","commit_id":"60ef49a6aa0c4d92a95dba93985a3291371f7562"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"710e472a4091d2dc8a5ac73bbfd190cc1f509cb4","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        for agent, net_id in bindings_to_add:"},{"line_number":85,"context_line":"            self.resource_filter.bind(context, [agent], net_id)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"            for agent in [agent]:"},{"line_number":88,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":89,"context_line":"                    query \u003d context.session.query("},{"line_number":90,"context_line":"                        agentschedulers_db.NetworkDhcpAgentBinding).options("}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_6bd6d6f2","line":87,"range":{"start_line":87,"start_character":11,"end_line":87,"end_character":33},"updated":"2016-06-15 10:58:35.000000000","message":"Looks redundant.","commit_id":"60ef49a6aa0c4d92a95dba93985a3291371f7562"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"710e472a4091d2dc8a5ac73bbfd190cc1f509cb4","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                        if ((not agents_db.AgentDbMixin.is_agent_down("},{"line_number":104,"context_line":"                           liveness)) and query_agent_alive.admin_state_up):"},{"line_number":105,"context_line":"                            existing_bindings.append(existing_alive_binding)"},{"line_number":106,"context_line":"                    if len(existing_bindings) \u003e agents_per_network:"},{"line_number":107,"context_line":"                        for existing_binding in \\"},{"line_number":108,"context_line":"                            existing_bindings[agents_per_network:]:"},{"line_number":109,"context_line":"                            context.session.delete(existing_binding)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_8ed99025","line":106,"updated":"2016-06-15 10:58:35.000000000","message":"This loop is done for bindings_to_add.\nWhen building bindings_to_add is calculated, we already checks agents_per_network. It looks you are doing the same thing twice. Can\u0027t you merge the loops?","commit_id":"60ef49a6aa0c4d92a95dba93985a3291371f7562"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"24805e07d5b96d2f61619e32fb375f2269123ed2","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                        if ((not agents_db.AgentDbMixin.is_agent_down("},{"line_number":104,"context_line":"                           liveness)) and query_agent_alive.admin_state_up):"},{"line_number":105,"context_line":"                            existing_bindings.append(existing_alive_binding)"},{"line_number":106,"context_line":"                    if len(existing_bindings) \u003e agents_per_network:"},{"line_number":107,"context_line":"                        for existing_binding in \\"},{"line_number":108,"context_line":"                            existing_bindings[agents_per_network:]:"},{"line_number":109,"context_line":"                            context.session.delete(existing_binding)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7aa08908_666b8486","line":106,"in_reply_to":"7aa08908_8ed99025","updated":"2016-06-16 00:24:41.000000000","message":"The thing that I want to fix here is concurrency. If I have two hosts, host-a and host-b, each with a DHCP agent. If two subnets are added to one network (which is not hosted by any dhcp agents yet) concurrently, then there is a chance that this network will be hosted by two dhcp agents, even if agents_per_network is set to 1. This is because the bindings_to_add validation check will valid for both requests, due to the fact that two requests happen at the same time and the network is not hosted by any agent at that time. Then both agents (from host-a and host-b) will be sent to bind(). The root cause is the \"check number of agents per network\" action and \"bind agents to network\" action do not happen in an atomic manner. \n\nThe fix I proposed here does a post clean up. After every agent is bound to a network, we check the number of agents hosting this network. If the number exceeds the configured number, we do a clean-up. As per Kevin\u0027s advice, the clean-up happen in concurrent requests as well, thus we need to find a way to avoid two requests deleting different dhcp agents and leaving the network with fewer agents than expected. So I add a sort by UUID to make sure the to-be-deleted agent in two requests is the same. \n\nThe logic here is a bit tricky. I hope I made myself clear enough.","commit_id":"60ef49a6aa0c4d92a95dba93985a3291371f7562"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7c4902c29bab6ac4b9bccb566544c6ef2a43bb1b","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                    net \u003d plugin.get_network(context, net_id)"},{"line_number":75,"context_line":"                    az_hints \u003d (net.get(az_ext.AZ_HINTS) or"},{"line_number":76,"context_line":"                                cfg.CONF.default_availability_zones)"},{"line_number":77,"context_line":"                    if (az_hints and dhcp_agent[\u0027availability_zone\u0027]"},{"line_number":78,"context_line":"                        not in az_hints):"},{"line_number":79,"context_line":"                        continue"},{"line_number":80,"context_line":"                    bindings_to_add.append((dhcp_agent, net_id))"},{"line_number":81,"context_line":"        # do it outside transaction so particular scheduling results don\u0027t"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_8a2eb989","line":78,"range":{"start_line":77,"start_character":0,"end_line":78,"end_character":41},"updated":"2016-06-27 06:13:59.000000000","message":"I think it is an unrelated change.","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"66ad96af08c751b4b71b0323c073283bedb6b7fd","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                    net \u003d plugin.get_network(context, net_id)"},{"line_number":75,"context_line":"                    az_hints \u003d (net.get(az_ext.AZ_HINTS) or"},{"line_number":76,"context_line":"                                cfg.CONF.default_availability_zones)"},{"line_number":77,"context_line":"                    if (az_hints and dhcp_agent[\u0027availability_zone\u0027]"},{"line_number":78,"context_line":"                        not in az_hints):"},{"line_number":79,"context_line":"                        continue"},{"line_number":80,"context_line":"                    bindings_to_add.append((dhcp_agent, net_id))"},{"line_number":81,"context_line":"        # do it outside transaction so particular scheduling results don\u0027t"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_1c1656a7","line":78,"range":{"start_line":77,"start_character":0,"end_line":78,"end_character":41},"in_reply_to":"3aaa91ec_8a2eb989","updated":"2016-06-28 05:59:40.000000000","message":"Done","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7c4902c29bab6ac4b9bccb566544c6ef2a43bb1b","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        # make other to fail"},{"line_number":83,"context_line":"        for agent, net_id in bindings_to_add:"},{"line_number":84,"context_line":"            self.resource_filter.bind(context, [agent], net_id)"},{"line_number":85,"context_line":"            for agent in [agent]:"},{"line_number":86,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":87,"context_line":"                    query \u003d context.session.query("},{"line_number":88,"context_line":"                        agentschedulers_db.NetworkDhcpAgentBinding).options("}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_0a592935","line":85,"range":{"start_line":85,"start_character":1,"end_line":85,"end_character":33},"updated":"2016-06-27 06:13:59.000000000","message":"why would you use this style, instead of using agent directly?","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"66ad96af08c751b4b71b0323c073283bedb6b7fd","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        # make other to fail"},{"line_number":83,"context_line":"        for agent, net_id in bindings_to_add:"},{"line_number":84,"context_line":"            self.resource_filter.bind(context, [agent], net_id)"},{"line_number":85,"context_line":"            for agent in [agent]:"},{"line_number":86,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"},{"line_number":87,"context_line":"                    query \u003d context.session.query("},{"line_number":88,"context_line":"                        agentschedulers_db.NetworkDhcpAgentBinding).options("}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_7c0b1a7e","line":85,"range":{"start_line":85,"start_character":1,"end_line":85,"end_character":33},"in_reply_to":"3aaa91ec_0a592935","updated":"2016-06-28 05:59:40.000000000","message":"Done","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7c4902c29bab6ac4b9bccb566544c6ef2a43bb1b","unresolved":false,"context_lines":[{"line_number":188,"context_line":"class DhcpFilter(base_resource_filter.BaseResourceFilter):"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def bind(self, context, agents, network_id):"},{"line_number":191,"context_line":"        \"\"\"Bind the network to the agents auto-schedule.\"\"\""},{"line_number":192,"context_line":"        # customize the bind logic"},{"line_number":193,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":194,"context_line":"        for agent in agents:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_6a6b8db7","line":191,"range":{"start_line":191,"start_character":8,"end_line":191,"end_character":59},"updated":"2016-06-27 06:13:59.000000000","message":"This is not only for auto-schedule, normal schedule will use this bind method too.","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"66ad96af08c751b4b71b0323c073283bedb6b7fd","unresolved":false,"context_lines":[{"line_number":188,"context_line":"class DhcpFilter(base_resource_filter.BaseResourceFilter):"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def bind(self, context, agents, network_id):"},{"line_number":191,"context_line":"        \"\"\"Bind the network to the agents auto-schedule.\"\"\""},{"line_number":192,"context_line":"        # customize the bind logic"},{"line_number":193,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":194,"context_line":"        for agent in agents:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_bc25820b","line":191,"range":{"start_line":191,"start_character":8,"end_line":191,"end_character":59},"in_reply_to":"3aaa91ec_6a6b8db7","updated":"2016-06-28 05:59:40.000000000","message":"Done","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"58656fce5d2e8cacce6231a05e368cf340a0b0df","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":104,"context_line":"                    for existing_binding in \\"},{"line_number":105,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":106,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":107,"context_line":"                        agent_id \u003d existing_binding.dhcp_agent_id"},{"line_number":108,"context_line":"                        LOG.debug(\u0027DHCP agent %(agent_id)s has been \u0027"},{"line_number":109,"context_line":"                                  \u0027removed from Network %(network_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3ac371cc_cdaeaf97","line":106,"updated":"2016-08-16 09:28:25.000000000","message":"you are going to need to catch exceptions if two processes try to deleting bindinds at the same time. Probably the whole transaction needs to be in try/except","commit_id":"62b079c25ca553be02e9c14a31380ece157711df"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"f81e9eab1b2727204791a2e7b5a7b7937ed368a5","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                if len(existing_bindings) \u003e agents_per_network:"},{"line_number":104,"context_line":"                    for existing_binding in \\"},{"line_number":105,"context_line":"                        existing_bindings[agents_per_network:]:"},{"line_number":106,"context_line":"                        context.session.delete(existing_binding)"},{"line_number":107,"context_line":"                        agent_id \u003d existing_binding.dhcp_agent_id"},{"line_number":108,"context_line":"                        LOG.debug(\u0027DHCP agent %(agent_id)s has been \u0027"},{"line_number":109,"context_line":"                                  \u0027removed from Network %(network_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3ac371cc_340bced7","line":106,"in_reply_to":"3ac371cc_cdaeaf97","updated":"2016-08-18 06:20:43.000000000","message":"I will add it in next PS","commit_id":"62b079c25ca553be02e9c14a31380ece157711df"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            self, context, network_id, is_manual_scheduling):"},{"line_number":180,"context_line":"        \"\"\"Return a vacant binding_index to use and whether or not it exists."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        Each NetworkDhcpAgentBinding has a binding_index which is unique per"},{"line_number":183,"context_line":"        network_id, and when creating a single binding we require to find a"},{"line_number":184,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example if we have"},{"line_number":185,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_67c41560","line":182,"updated":"2017-07-11 20:37:44.000000000","message":"why can\u0027t we just allow the server to autoincrement for us? I don\u0027t see a problem with having gaps in between.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            self, context, network_id, is_manual_scheduling):"},{"line_number":180,"context_line":"        \"\"\"Return a vacant binding_index to use and whether or not it exists."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        Each NetworkDhcpAgentBinding has a binding_index which is unique per"},{"line_number":183,"context_line":"        network_id, and when creating a single binding we require to find a"},{"line_number":184,"context_line":"        \u0027vacant\u0027 binding_index which isn\u0027t yet used - for example if we have"},{"line_number":185,"context_line":"        bindings with indices 1 and 3, then clearly binding_index \u003d\u003d 2 is free."}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_489f864f","line":182,"in_reply_to":"3f1d235d_67c41560","updated":"2019-08-19 12:34:50.000000000","message":"I might miss something, but if we just allow auto-increment the bug won\u0027t be fixed and we\u0027ll still have over-scheduled networks due to the race.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # creation of a binding_index even if it will exceed"},{"line_number":212,"context_line":"        # max_l3_agents_per_router."},{"line_number":213,"context_line":"        if is_manual_scheduling:"},{"line_number":214,"context_line":"            return max(all_indicies) + 1"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        return -1"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_c7a2c99a","line":214,"range":{"start_line":214,"start_character":27,"end_line":214,"end_character":35},"updated":"2017-07-11 20:37:44.000000000","message":"indices","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"14edfa416e6869682f24d4bbdca22e6b22687128","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # creation of a binding_index even if it will exceed"},{"line_number":212,"context_line":"        # max_l3_agents_per_router."},{"line_number":213,"context_line":"        if is_manual_scheduling:"},{"line_number":214,"context_line":"            return max(all_indicies) + 1"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        return -1"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7faddb67_08a50e9f","line":214,"range":{"start_line":214,"start_character":27,"end_line":214,"end_character":35},"in_reply_to":"3f1d235d_c7a2c99a","updated":"2019-08-19 12:34:50.000000000","message":"Done","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c36cd08bde6cdbbabc2b5c48405283563bbb1240","unresolved":false,"context_lines":[{"line_number":220,"context_line":"        # customize the bind logic"},{"line_number":221,"context_line":"        bound_agents \u003d agents[:]"},{"line_number":222,"context_line":"        for agent in agents:"},{"line_number":223,"context_line":"            binding_index \u003d self.get_vacant_network_dhcp_agent_binding_index("},{"line_number":224,"context_line":"                context, network_id, is_manual_scheduling)"},{"line_number":225,"context_line":"            if binding_index \u003c ndab_model.LOWEST_BINDING_INDEX:"},{"line_number":226,"context_line":"                LOG.debug(\u0027Unable to find a vacant binding_index for \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_0a8328ce","line":223,"updated":"2017-07-11 20:37:44.000000000","message":"my assumption is that we don\u0027t want to have contract migrations (that impose api downtime on upgrade). if that\u0027s the case, then we will need to accommodate for existence of older neutron-servers in the cluster and expect that they may create bindings that don\u0027t fulfill basic constraints set in here (like that index is always less than the number of agents set in config file).\n\nIt may mean that when looking for gaps in string of used indices, one may discover that the needed number of agents is already scheduled, just some with non-conforming indices. In that case, I would expect that we would fix that in-place for those non-conforming bindings and move on.","commit_id":"8f031ae3f29e21ee805b65a55db3fcd11cfcec9d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"72fd68a5d8028e49d8d9bd35d7e3a929b94447f0","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        :returns: binding_index."},{"line_number":188,"context_line":"        \"\"\""},{"line_number":189,"context_line":"        num_agents \u003d agent_obj.Agent.count("},{"line_number":190,"context_line":"            context, agent_type\u003dconstants.AGENT_TYPE_DHCP)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        bindings \u003d network.NetworkDhcpAgentBinding.get_objects("}],"source_content_type":"text/x-python","patch_set":23,"id":"7faddb67_892447b4","line":189,"range":{"start_line":189,"start_character":8,"end_line":189,"end_character":18},"updated":"2019-08-23 07:28:22.000000000","message":"seems this should not exceed dhcp_agents_per_network config","commit_id":"4048723c5d33a26a51943fd7cc32e4935f236563"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9dfd730b03b561bae4b1c9685ec5b0cd574c58ae","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if force_scheduling:"},{"line_number":211,"context_line":"            return max(all_indices) + 1"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        return -1"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def bind(self, context, agents, network_id, force_scheduling\u003dFalse):"},{"line_number":216,"context_line":"        \"\"\"Bind the network to the agents.\"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_ee18d3cd","line":213,"updated":"2019-09-02 15:28:18.000000000","message":"it looks for me like \"C style\" coding :) IMHO returning None would be more \"python style\"","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"73e48460fcd07a980877dff6b378ccffa7b5cf1d","unresolved":false,"context_lines":[{"line_number":210,"context_line":"        if force_scheduling:"},{"line_number":211,"context_line":"            return max(all_indices) + 1"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        return -1"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def bind(self, context, agents, network_id, force_scheduling\u003dFalse):"},{"line_number":216,"context_line":"        \"\"\"Bind the network to the agents.\"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"7faddb67_9598471b","line":213,"in_reply_to":"7faddb67_ee18d3cd","updated":"2019-09-03 07:04:09.000000000","message":"I guess this was also copied from neutron/db/l3_agentschedulers_db.py.\nIt\u0027s also used in \u0027if\u0027 statement below (#222), where \u0027None\u0027 won\u0027s suit.","commit_id":"69b3762dda47272513e02340e7942d5a39f825c5"}],"neutron/tests/unit/scheduler/test_dhcp_agent_scheduler.py":[{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"c57764169d5fad53ad89630062b90606f6fe7e86","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        results \u003d self.ctx.session.query("},{"line_number":82,"context_line":"            sched_db.NetworkDhcpAgentBinding).filter_by("},{"line_number":83,"context_line":"            network_id\u003dnetwork_id).all()"},{"line_number":84,"context_line":"        self.assertEqual(expected_hosted_agents, len(results))"},{"line_number":85,"context_line":"        existance \u003d False"},{"line_number":86,"context_line":"        for agent in agents:"},{"line_number":87,"context_line":"            for result in results:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1af94dfe_e3d42621","line":84,"updated":"2016-03-17 21:29:56.000000000","message":"I\u0027m probably missing something very obvious as this code is still a bit new to me, but where\u0027s the concurrency?","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"791d210ba5c70a437f4fdc3201eaad71ab288193","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        results \u003d self.ctx.session.query("},{"line_number":82,"context_line":"            sched_db.NetworkDhcpAgentBinding).filter_by("},{"line_number":83,"context_line":"            network_id\u003dnetwork_id).all()"},{"line_number":84,"context_line":"        self.assertEqual(expected_hosted_agents, len(results))"},{"line_number":85,"context_line":"        existance \u003d False"},{"line_number":86,"context_line":"        for agent in agents:"},{"line_number":87,"context_line":"            for result in results:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa0719c6_d9712f46","line":84,"in_reply_to":"1af94dfe_e3d42621","updated":"2016-03-23 08:07:02.000000000","message":"My mistake. I will revise the name.","commit_id":"4d99a5358867af69eecb3b68040523ad3a56ad62"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7c4902c29bab6ac4b9bccb566544c6ef2a43bb1b","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def test_schedule_bind_network_multi_agents(self):"},{"line_number":88,"context_line":"        agents \u003d self._create_and_set_agents_down([\u0027host-a\u0027, \u0027host-b\u0027])"},{"line_number":89,"context_line":"        cfg.CONF.set_override(\u0027dhcp_agents_per_network\u0027, 2)"},{"line_number":90,"context_line":"        self._test_schedule_bind_network(agents, self.network_id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_schedule_bind_network_multi_agent_fail_one(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_8ae1992c","line":89,"range":{"start_line":89,"start_character":0,"end_line":89,"end_character":59},"updated":"2016-06-27 06:13:59.000000000","message":"In real case, this configuration should be 2, but this test case if for scheduler.resource_filter.bind, which makes this configuration meaningless here. So, I think it is ok to keep it unchanged.","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"66ad96af08c751b4b71b0323c073283bedb6b7fd","unresolved":false,"context_lines":[{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def test_schedule_bind_network_multi_agents(self):"},{"line_number":88,"context_line":"        agents \u003d self._create_and_set_agents_down([\u0027host-a\u0027, \u0027host-b\u0027])"},{"line_number":89,"context_line":"        cfg.CONF.set_override(\u0027dhcp_agents_per_network\u0027, 2)"},{"line_number":90,"context_line":"        self._test_schedule_bind_network(agents, self.network_id)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_schedule_bind_network_multi_agent_fail_one(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"3aaa91ec_9c224600","line":89,"range":{"start_line":89,"start_character":0,"end_line":89,"end_character":59},"in_reply_to":"3aaa91ec_8ae1992c","updated":"2016-06-28 05:59:40.000000000","message":"Done","commit_id":"6fcf15b7761b5a77d7d57213dcdc26dc1e29afee"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"0d1d0002e1ad5dff089a3a7e7a29d1fe935dfaf3","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        expected_hosted_agents \u003d 1"},{"line_number":417,"context_line":"        binding_index \u003d 1"},{"line_number":418,"context_line":"        scheduler.auto_schedule_networks(plugin, self.ctx, \u0027host-a\u0027)"},{"line_number":419,"context_line":"        with mock.patch(dhcpfilter + \\"},{"line_number":420,"context_line":"            \u0027.get_vacant_network_dhcp_agent_binding_index\u0027,"},{"line_number":421,"context_line":"            context\u003dself.ctx, network_id\u003dself.network_id) as ndab:"},{"line_number":422,"context_line":"            ndab.return_value \u003d binding_index"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a1ced50_d66225ad","line":419,"range":{"start_line":419,"start_character":36,"end_line":419,"end_character":38},"updated":"2017-03-16 22:52:23.000000000","message":"You don\u0027t need this. The parenthesis allows you to continue the line","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"},{"author":{"_account_id":17491,"name":"Lujin Luo","email":"luo.lujin@jp.fujitsu.com","username":"Lujin"},"change_message_id":"ff82c69bd340251e02a646092cf4265b4f7c4c7b","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        expected_hosted_agents \u003d 1"},{"line_number":417,"context_line":"        binding_index \u003d 1"},{"line_number":418,"context_line":"        scheduler.auto_schedule_networks(plugin, self.ctx, \u0027host-a\u0027)"},{"line_number":419,"context_line":"        with mock.patch(dhcpfilter + \\"},{"line_number":420,"context_line":"            \u0027.get_vacant_network_dhcp_agent_binding_index\u0027,"},{"line_number":421,"context_line":"            context\u003dself.ctx, network_id\u003dself.network_id) as ndab:"},{"line_number":422,"context_line":"            ndab.return_value \u003d binding_index"}],"source_content_type":"text/x-python","patch_set":15,"id":"1a1ced50_d273034c","line":419,"range":{"start_line":419,"start_character":36,"end_line":419,"end_character":38},"in_reply_to":"1a1ced50_d66225ad","updated":"2017-03-17 09:39:17.000000000","message":"Done","commit_id":"734b3f15e5306f16c57bc36b08c95e1ee0a3187e"}]}