)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sebastian Lohff \u003csebastian.lohff@sap.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-18 10:47:58 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Limit max ports per rpc for dhcp_ready_ports()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The Neutron dhcp agents reports all ready ports to the Neutron"},{"line_number":10,"context_line":"server via the dhcp_ready_ports() rpc call. When the dhcp agent"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_4892724d","line":7,"range":{"start_line":7,"start_character":28,"end_line":7,"end_character":46},"updated":"2019-06-21 23:55:36.000000000","message":"There is no dhcp_ready_ports() rpc call: http://codesearch.openstack.org/?q\u003ddhcp_ready_ports\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d. The correct rpc call is dhcp_ready_on_ports: https://github.com/openstack/neutron/blob/3b92b131b4b4bce27e9ae1bd2fd94b347a3a3bf5/neutron/agent/dhcp/agent.py#L771. Please refer to the correct call","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Limit max ports per rpc for dhcp_ready_ports()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The Neutron dhcp agents reports all ready ports to the Neutron"},{"line_number":10,"context_line":"server via the dhcp_ready_ports() rpc call. When the dhcp agent"},{"line_number":11,"context_line":"gets ports ready faster than the server can process them the amount"},{"line_number":12,"context_line":"of ports per rpc call can grow so high (e.g. 10000 Ports) that the"},{"line_number":13,"context_line":"neutron server never has a chance of processing the request before"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_089c7a5b","line":10,"range":{"start_line":10,"start_character":15,"end_line":10,"end_character":33},"updated":"2019-06-21 23:55:36.000000000","message":"Ditto","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":13,"context_line":"neutron server never has a chance of processing the request before"},{"line_number":14,"context_line":"the rpc timeout kills the request, leading to the dhcp agent"},{"line_number":15,"context_line":"sending the request again, resulting in an endless loop of"},{"line_number":16,"context_line":"dhcp_ready_ports() calls. This happens especially on agent startup."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To mitigate this problems we now limit the number of ports send"},{"line_number":19,"context_line":"per dhcp_ready_ports() call."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_a8a20e99","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":18},"updated":"2019-06-21 23:55:36.000000000","message":"Ditto","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"sending the request again, resulting in an endless loop of"},{"line_number":16,"context_line":"dhcp_ready_ports() calls. This happens especially on agent startup."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To mitigate this problems we now limit the number of ports send"},{"line_number":19,"context_line":"per dhcp_ready_ports() call."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I407e126e760ebf6aca4c31b9c3ff58dcfa55107f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_28b85ec6","line":18,"range":{"start_line":18,"start_character":59,"end_line":18,"end_character":63},"updated":"2019-06-21 23:55:36.000000000","message":"nit: sent","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_ready_ports() calls. This happens especially on agent startup."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To mitigate this problems we now limit the number of ports send"},{"line_number":19,"context_line":"per dhcp_ready_ports() call."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I407e126e760ebf6aca4c31b9c3ff58dcfa55107f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_c89d0258","line":19,"range":{"start_line":19,"start_character":4,"end_line":19,"end_character":22},"updated":"2019-06-21 23:55:36.000000000","message":"Ditto","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c20c16ed5cbf98bc8c1055208fadaface80d2a15","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_ready_ports() calls. This happens especially on agent startup."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To mitigate this problems we now limit the number of ports send"},{"line_number":19,"context_line":"per dhcp_ready_ports() call."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I407e126e760ebf6aca4c31b9c3ff58dcfa55107f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_eb0d6d90","line":19,"updated":"2019-06-21 09:55:11.000000000","message":"Is there any bug related to this change?","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":16,"context_line":"dhcp_ready_ports() calls. This happens especially on agent startup."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"To mitigate this problems we now limit the number of ports send"},{"line_number":19,"context_line":"per dhcp_ready_ports() call."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I407e126e760ebf6aca4c31b9c3ff58dcfa55107f"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fb8cfa7_08b55acd","line":19,"in_reply_to":"9fb8cfa7_eb0d6d90","updated":"2019-06-21 23:55:36.000000000","message":"Yes, please file a bug","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"}],"neutron/agent/dhcp/agent.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"13ac89c1db63a85fd1f339b086d7532d113286ad","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"},{"line_number":252,"context_line":"                    self.dhcp_ready_ports \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_56c2ba5a","line":249,"updated":"2019-05-15 13:10:41.000000000","message":"My only concern here is that, although this loop can be executed relatively fast, other process can change self.dhcp_ready_ports in the meanwhile.\n\nThose event functions (network/subnet update/delete) modifying this variable have a _wait_if_syncing decorator to prevent interferences, but we can\u0027t add it to this _dhcp_ready_ports_loop.","commit_id":"206140ad2dc0608ee7bc7d6256fee5f6ef9e4004"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"831543ccc51dd5e3b0c775271e42c0332c318b13","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"},{"line_number":252,"context_line":"                    self.dhcp_ready_ports \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_608a8f6a","line":249,"in_reply_to":"dfbec78f_56c2ba5a","updated":"2019-05-15 15:24:45.000000000","message":"+1.\n\nWhy can\u0027t we just grab them all and iterate through it in chunks if there was a timeout?  Similar to what we do in the l3-agent fetch_and_sync_all_routers() code (and i think the ml2 agent as well).","commit_id":"206140ad2dc0608ee7bc7d6256fee5f6ef9e4004"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"31b7decbaadd1730d48bae1e61b56ba926e1ae23","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"},{"line_number":252,"context_line":"                    self.dhcp_ready_ports \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_aa014608","line":249,"in_reply_to":"dfbec78f_56c2ba5a","updated":"2019-05-15 14:26:53.000000000","message":"As far as I saw in the agent.py this line is the only line removing items from this variable. All other calls are only adding contents to this set, therefore there are always enough elements to pop by and Python sets should be sufficiently threadsafe that one thread can add contents while the other can remove it. Or did i misunderstand anything either in Python or in your problem description?","commit_id":"206140ad2dc0608ee7bc7d6256fee5f6ef9e4004"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"5b1302973a7afb7a216d559998b8696b3fc08f87","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"},{"line_number":252,"context_line":"                    self.dhcp_ready_ports \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_700a3820","line":249,"in_reply_to":"dfbec78f_608a8f6a","updated":"2019-06-14 16:15:16.000000000","message":"Well, we could also build it that way and let it sync as many ports as it can inside a certain range, varying between two values, adapting to the current load of neutron-server. But what would these values be?\n\nMy original intent for this code was to fix a problem I observed in production where the dhcp agent sent 14000 ready-ports in $rpc_timeout_intervals to a neutron-server, leaving all our neutron-servers pretty busy. From that perspective I didn\u0027t need an elastic chunksize, I just needed it to be lower than $insane_value and chose a generally sane value.\n\nSo I would argue that this elasticity is not really needed. If you think it is I can certainly take a shot at resembling fetch_and_sync_all_routers().","commit_id":"206140ad2dc0608ee7bc7d6256fee5f6ef9e4004"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1b73744e614921302cce4cdb319ff1fe774fcbc4","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"},{"line_number":252,"context_line":"                    self.dhcp_ready_ports \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_13df4669","line":249,"in_reply_to":"dfbec78f_aa014608","updated":"2019-06-14 17:12:03.000000000","message":"My concern was about how thread safe is .pop() and the other set manipulations done in this class. But according to the documentation, those operations are thread safe.","commit_id":"206140ad2dc0608ee7bc7d6256fee5f6ef9e4004"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"274be39a01b953734ea6a23c9f84cb01be769beb","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_61fd7a07","line":248,"updated":"2019-06-17 08:44:32.000000000","message":"with something like:\n\n    for port_count in range(min(DHCP_READY_PORTS_SYNC_MAX, len(self.dhcp_ready_ports))):\n\nYou would probably don\u0027t need this \"if...else\" here","commit_id":"67159af838b66bdd970b02ab090100e60a282403"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"b8d6c4b2971810f3b922fa392832e6ded314d3c3","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":246,"context_line":"                if len(self.dhcp_ready_ports) \u003e DHCP_READY_PORTS_SYNC_MAX:"},{"line_number":247,"context_line":"                    ports_to_send \u003d set()"},{"line_number":248,"context_line":"                    for port_count in range(DHCP_READY_PORTS_SYNC_MAX):"},{"line_number":249,"context_line":"                        ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":"                else:"},{"line_number":251,"context_line":"                    ports_to_send \u003d self.dhcp_ready_ports"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_1c3981f1","line":248,"in_reply_to":"9fb8cfa7_61fd7a07","updated":"2019-06-17 10:56:40.000000000","message":"I like the idea. Makes the change much shorter and concise and the pop()/add() is a fast operation anyway.","commit_id":"67159af838b66bdd970b02ab090100e60a282403"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"90717e61fc2dde70bea5ff6e3d0ee2df883c4308","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_6f2d33a7","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"updated":"2019-06-19 13:08:18.000000000","message":"Yes, we\u0027ve done some similar work for ovs-agent to update the port sets to neutron-server. But, for my concern, this may have influence on port DHCP  provisioning_block, something like instance failed to boot due to this long wait. Could you confirm this?","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7e7c7390779b9781f5464d06bd990e426287ae31","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_67016e6a","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_271ff608","updated":"2019-06-26 12:04:44.000000000","message":"Actually IMO this may even speed it up as currently all ports are send in one message so are processed by one rpc worker.\nAnd after this change it can be distributed between many rpc workers.","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"59aeeb3777753a0de56aa41cf3ba2afb8c380449","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_08951f40","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_4d248960","updated":"2019-06-26 14:05:14.000000000","message":"It won\u0027t speed up I think, as the batches are still processed synchronously in this thread. Regarding the timeout: Yes, if the rpc has a timeout the next batch will have to wait. But in my experience when this rpc call times out it is not dependent much on the specific ports, but more on the amount of ports. The chosen value of 64 should be reasonably low that a timeout due to the amount of ports is very unlikely, so I\u0027d expect on a timeout that there\u0027s something else wrong with neutron-server that would be causing this.","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a56453d6b6e85e3eb726624309790d7c1eb0537f","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_4d248960","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_67016e6a","updated":"2019-06-26 13:20:02.000000000","message":"If line 253 meets timeout exception, that means the following port sets have wait for that time long. The later sequence of remain ports , the higher chance of provisioning timeout for them.","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"5b58242d59a130aa36f6e7ea515fc6f55323859e","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_a8cbee50","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_6f2d33a7","updated":"2019-06-21 23:55:36.000000000","message":"Would you please respond to this concern?","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"2d55a2142cd9b947ad9838d771b924359c29c4fa","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_dcfb4dbb","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_a8cbee50","updated":"2019-06-26 10:37:09.000000000","message":"I don\u0027t think this will make provisioning_block slower.\n\nThe only way it makes it slower is by us having extra wait due to not all ports being sent at once, but being split up into multiple apicalls. This would also mean that we have more than DHCP_READY_PORTS_SYNC_MAX ports ready and at this point processing them in batches that are still processable by neutron-server is better than the alternative of neutron-server not being able to respond and being sent the same, growing, bunch of ports over and over again.\n\nIn \"normal\" situations there shouldn\u0027t be fewer than DHCP_READY_PORTS_SYNC_MAX ports and it should behave pretty much the same.\n\nIf your worry is on the other hand that the forloop takes too long, at least on my laptop popping 64 ports of a set with 10k entries takes less than 0.5 miliseconds, 1k ports is at ~3ms.","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d2e2deaeeff8a1d626f3947e7a360cf47c252478","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            eventlet.sleep(0.1)"},{"line_number":246,"context_line":"            if self.dhcp_ready_ports:"},{"line_number":247,"context_line":"                ports_to_send \u003d set()"},{"line_number":248,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":249,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":250,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fb8cfa7_271ff608","line":249,"range":{"start_line":248,"start_character":16,"end_line":249,"end_character":72},"in_reply_to":"9fb8cfa7_dcfb4dbb","updated":"2019-06-26 11:46:22.000000000","message":"I agree Sebastian. The max loop size will be DHCP_READY_PORTS_SYNC_MAX\u003d64. This is going to be quite fast.","commit_id":"14d8096527e6afd9b1aeaacc465f87565e4c54c9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3c4ba54ecb605947a4aa9126ffaf688eb86c065c","unresolved":false,"context_lines":[{"line_number":246,"context_line":"                ports_to_send \u003d set()"},{"line_number":247,"context_line":"                for port_count in range(min(len(self.dhcp_ready_ports),"},{"line_number":248,"context_line":"                                            DHCP_READY_PORTS_SYNC_MAX)):"},{"line_number":249,"context_line":"                    ports_to_send.add(self.dhcp_ready_ports.pop())"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"                try:"},{"line_number":252,"context_line":"                    self.plugin_rpc.dhcp_ready_on_ports(ports_to_send)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_cd4b99ae","line":249,"updated":"2019-06-26 13:36:10.000000000","message":"So this is about a 2K message, len(uuid) * 64 \u003d 2304 - just want to make sure that\u0027s in a \"good\" range.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"}],"neutron/tests/unit/agent/dhcp/test_agent.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"72ed2980b3ab1da296f33306fbea10e072c15278","unresolved":false,"context_lines":[{"line_number":482,"context_line":"        # should have been called with all ports again after the failure"},{"line_number":483,"context_line":"        ready.assert_has_calls([mock.call(set(range(4)))] * 2)"},{"line_number":484,"context_line":""},{"line_number":485,"context_line":"    def test_dhcp_ready_ports_loop_with_limit_ports_per_call(self):"},{"line_number":486,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"},{"line_number":487,"context_line":"        port_count \u003d dhcp_agent.DHCP_READY_PORTS_SYNC_MAX + 1"},{"line_number":488,"context_line":"        dhcp.dhcp_ready_ports \u003d set(range(port_count))"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_ca8b7865","line":485,"updated":"2019-05-28 13:02:28.000000000","message":"this new test if failing on lower-constraints job. Please check it.","commit_id":"21d779df831224aa1f8000e33e87a8c649d294de"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"72ed2980b3ab1da296f33306fbea10e072c15278","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        # second one with one port"},{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0].args[0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1].args[0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0].args[0] |"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_aa9ba470","line":504,"range":{"start_line":504,"start_character":29,"end_line":504,"end_character":60},"updated":"2019-05-28 13:02:28.000000000","message":"looks like this works differently in mock 2.0.0 version and that cause problem","commit_id":"21d779df831224aa1f8000e33e87a8c649d294de"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"5b1302973a7afb7a216d559998b8696b3fc08f87","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        # second one with one port"},{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0].args[0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1].args[0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0].args[0] |"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_d01a046d","line":504,"range":{"start_line":504,"start_character":29,"end_line":504,"end_character":60},"in_reply_to":"bfb3d3c7_aa9ba470","updated":"2019-06-14 16:15:16.000000000","message":"Thanks for pointing that out. Test adapted.","commit_id":"21d779df831224aa1f8000e33e87a8c649d294de"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3c4ba54ecb605947a4aa9126ffaf688eb86c065c","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        ready.assert_has_calls([mock.call(set(range(4)))] * 2)"},{"line_number":484,"context_line":""},{"line_number":485,"context_line":"    def test_dhcp_ready_ports_loop_with_limit_ports_per_call(self):"},{"line_number":486,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"},{"line_number":487,"context_line":"        port_count \u003d dhcp_agent.DHCP_READY_PORTS_SYNC_MAX + 1"},{"line_number":488,"context_line":"        dhcp.dhcp_ready_ports \u003d set(range(port_count))"},{"line_number":489,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_8ddda115","line":486,"updated":"2019-06-26 13:36:10.000000000","message":"if you set this here...\n\nsync_max \u003d dhcp_agent.DHCP_READY_PORTS_SYNC_MAX\n\n(so things don\u0027t wrap as much)","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ecac45bbe5c04f6919bc6007cff4eb1e8f1270f5","unresolved":false,"context_lines":[{"line_number":483,"context_line":"        ready.assert_has_calls([mock.call(set(range(4)))] * 2)"},{"line_number":484,"context_line":""},{"line_number":485,"context_line":"    def test_dhcp_ready_ports_loop_with_limit_ports_per_call(self):"},{"line_number":486,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"},{"line_number":487,"context_line":"        port_count \u003d dhcp_agent.DHCP_READY_PORTS_SYNC_MAX + 1"},{"line_number":488,"context_line":"        dhcp.dhcp_ready_ports \u003d set(range(port_count))"},{"line_number":489,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_d8efbeae","line":486,"in_reply_to":"9fb8cfa7_8ddda115","updated":"2019-06-27 13:35:50.000000000","message":"+1","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2d403b8a549ff45f9941ac9ca236c882e054ec98","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        dhcp.dhcp_ready_ports \u003d set(range(port_count))"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        with mock.patch.object(dhcp.plugin_rpc, \u0027dhcp_ready_on_ports\u0027,"},{"line_number":491,"context_line":"                               side_effect\u003d[0, 0]) as ready:"},{"line_number":492,"context_line":"            # exit after 2 iterations"},{"line_number":493,"context_line":"            with mock.patch.object(dhcp_agent.eventlet, \u0027sleep\u0027,"},{"line_number":494,"context_line":"                                   side_effect\u003d[0, 0, RuntimeError]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_3fcdb6b6","line":491,"range":{"start_line":491,"start_character":31,"end_line":491,"end_character":49},"updated":"2019-06-26 15:20:07.000000000","message":"We have no side-effect from this call, like an exception, so this is not necessary.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"01a60f5f9449d9de962128b6d6401ba3407f3978","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        dhcp.dhcp_ready_ports \u003d set(range(port_count))"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        with mock.patch.object(dhcp.plugin_rpc, \u0027dhcp_ready_on_ports\u0027,"},{"line_number":491,"context_line":"                               side_effect\u003d[0, 0]) as ready:"},{"line_number":492,"context_line":"            # exit after 2 iterations"},{"line_number":493,"context_line":"            with mock.patch.object(dhcp_agent.eventlet, \u0027sleep\u0027,"},{"line_number":494,"context_line":"                                   side_effect\u003d[0, 0, RuntimeError]):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_ff7abe21","line":491,"range":{"start_line":491,"start_character":31,"end_line":491,"end_character":49},"in_reply_to":"9fb8cfa7_3fcdb6b6","updated":"2019-06-26 15:33:36.000000000","message":"Right. I will remove this.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3c4ba54ecb605947a4aa9126ffaf688eb86c065c","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_ade2e5d3","line":505,"updated":"2019-06-26 13:36:10.000000000","message":"...then the above three lines can be replaced by:\n\n        ready.assert_has_calls(\n            [mock.call(set(range(sync_max)))],\n            [mock.call(set(range(sync_max, port_count)))]\n        )","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2d403b8a549ff45f9941ac9ca236c882e054ec98","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_df733a7f","line":505,"in_reply_to":"9fb8cfa7_3ceb8478","updated":"2019-06-26 15:20:07.000000000","message":"But L483 is doing the exact thing I\u0027m suggesting, and that test has been Ok for the three years it\u0027s been here.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"f682a77b09a474dd7587b36a47e28bca2c848386","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_3ceb8478","line":505,"in_reply_to":"9fb8cfa7_5ca198a8","updated":"2019-06-26 15:02:39.000000000","message":"any_order\u003dTrue says that the calls being made do not need to be in the correct order[0], but this is not the problem here. The set(range(port_count)) creates a set with an arbitrary ordering. The python documentation for set().pop reads \"remove and return an arbitrary element from set\". This means that when we give the code we want to test our set we cannot be sure that the first set will be set(range(port_count - 1)) and the second set could be set([port_count - 1]). The order of the set is arbitrary (though not random, apparently). So therefore if I now change this test to checking the exact set we would rely on the order pop() retrieves the element from the set, which here is implementation specific to cPython as far as I understood.\n\nThe behaviour seems to be the same in Python2 and Python3, also it seems like Python is sorting that set.\n\nIn [28]: list(set(range(1000 - 1, -1, -1))) \u003d\u003d list(range(1000))\nOut[28]: True\n\nBut do we really want to rely on implementation specific behaviour for this test?\n\n\n[0] https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.assert_has_calls","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"59aeeb3777753a0de56aa41cf3ba2afb8c380449","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_c8462751","line":505,"in_reply_to":"9fb8cfa7_ade2e5d3","updated":"2019-06-26 14:05:14.000000000","message":"A set is by Python\u0027s docs an undordered data structure[0], therefore I decided to only check for \"how many ports have been processed per call\" and not of the contents, as with an unordered datastructure I cannot predict the ports that are actually sent over (even if in this specific Python implementation we could).\n\nI could still do the sync_max \u003d ... thing and the linebreak into 504 would vanish if you like. Do you want me to change that?\n\n[0] https://docs.python.org/2/library/sets.html","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2dbc7101303fd79767f54e5beee68ed9b8839506","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_5ca198a8","line":505,"in_reply_to":"9fb8cfa7_c8462751","updated":"2019-06-26 14:46:52.000000000","message":"assert_has_calls() can handle out-of-ordering by using any_order\u003dTrue, although it doesn\u0027t seem necessary based on L483.  It\u0027s much cleaner than using call_args_list IMO.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"01a60f5f9449d9de962128b6d6401ba3407f3978","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_fff63e8f","line":505,"in_reply_to":"9fb8cfa7_df733a7f","updated":"2019-06-26 15:33:36.000000000","message":"Yes, it works, and it will also work in L483, but it\u0027s still implementation specific (and undocumented) behavior we\u0027re relying on, which might change at any point in time. I would\u0027ve considered it bad style to rely on these things and opt for the version not doing so. Nevertheless, if you think it makes the test much more concise and maintainable, alas making relying on implementation specific/undocumented behavior the lesser of two evils I will change the test to your version and will also remove L506-509, as they won\u0027t be needed anymore then.\n\nShall I do so?","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"76039bc69134cdc670cae33b3628ba54427ea627","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_628f0075","line":505,"in_reply_to":"9fb8cfa7_f8d3a2ff","updated":"2019-06-27 15:22:01.000000000","message":"All right, let\u0027s leave this alone then, better to not rely on undefined behavior.  Thanks for spending the time to convince me.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ecac45bbe5c04f6919bc6007cff4eb1e8f1270f5","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_f8d3a2ff","line":505,"in_reply_to":"9fb8cfa7_fac5df5c","updated":"2019-06-27 13:35:50.000000000","message":"I agree with Sebastian here too. In L483 it is expected that all ports which are in dhcp.dhcp_ready_ports are passed to \"ready\" at once so You can for sure define what arguments it will have there. In this case You can\u0027t be sure that as it may be that it will be called e.g. like:\n\n    ready(set([2, 3, 4]))\n    ready(set([1]))","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":26250,"name":"Johannes Kulik","email":"johannes.kulik@sap.com","username":"jkulik"},"change_message_id":"87ae34d33612ab0b1c5b8a493d5ec2c627fbe814","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        self.assertEqual(2, ready.call_count)"},{"line_number":503,"context_line":"        self.assertEqual(dhcp_agent.DHCP_READY_PORTS_SYNC_MAX,"},{"line_number":504,"context_line":"                         len(ready.call_args_list[0][0][0]))"},{"line_number":505,"context_line":"        self.assertEqual(1, len(ready.call_args_list[1][0][0]))"},{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_fac5df5c","line":505,"in_reply_to":"9fb8cfa7_fff63e8f","updated":"2019-06-27 08:31:29.000000000","message":"I agree with Sebastian\u0027s view here. L483 is fine, because before, all ports were sent unconditionally. With sending just a subset of the ports and objects retrieved from the set in arbitrary order, we cannot use the same solution.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3c4ba54ecb605947a4aa9126ffaf688eb86c065c","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"},{"line_number":509,"context_line":"        self.assertEqual(set(range(port_count)), ports_ready)"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def test_dhcp_ready_ports_updates_after_enable_dhcp(self):"},{"line_number":512,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_4de329d9","line":509,"updated":"2019-06-26 13:36:10.000000000","message":"Isn\u0027t this just testing the same as above?  The [0][0][0] doesn\u0027t make it easy to know.  Plus we\u0027ve already checked the set is empty on L499.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"59aeeb3777753a0de56aa41cf3ba2afb8c380449","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"},{"line_number":509,"context_line":"        self.assertEqual(set(range(port_count)), ports_ready)"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def test_dhcp_ready_ports_updates_after_enable_dhcp(self):"},{"line_number":512,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_889b4fc7","line":509,"in_reply_to":"9fb8cfa7_4de329d9","updated":"2019-06-26 14:05:14.000000000","message":"The emptyset check checks that all ports have been processed, e.g. have been taken out of the set, the checks afterwards check that the amount of ports processed is right. As the order of a set is more or less random I cannot say that all ports have been processed, but only how many. So to be 100% sure I added the last assert that makes sure that each port has been processed.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2dbc7101303fd79767f54e5beee68ed9b8839506","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"},{"line_number":509,"context_line":"        self.assertEqual(set(range(port_count)), ports_ready)"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def test_dhcp_ready_ports_updates_after_enable_dhcp(self):"},{"line_number":512,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_dc94a846","line":509,"in_reply_to":"9fb8cfa7_889b4fc7","updated":"2019-06-26 14:46:52.000000000","message":"Right, but changing to use assert_has_calls() covers this case as well.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"6b9d7af7ebf192a5b0f00af00029c8118993eb00","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        # all ports need to be ready"},{"line_number":507,"context_line":"        ports_ready \u003d (ready.call_args_list[0][0][0] |"},{"line_number":508,"context_line":"                       ready.call_args_list[1][0][0])"},{"line_number":509,"context_line":"        self.assertEqual(set(range(port_count)), ports_ready)"},{"line_number":510,"context_line":""},{"line_number":511,"context_line":"    def test_dhcp_ready_ports_updates_after_enable_dhcp(self):"},{"line_number":512,"context_line":"        dhcp \u003d dhcp_agent.DhcpAgent(HOSTNAME)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9fb8cfa7_141990d5","line":509,"in_reply_to":"9fb8cfa7_dc94a846","updated":"2019-06-28 15:23:34.000000000","message":"As we\u0027re keeping the change the way it is in L505 I\u0027m also leaving this in, as discussed above.","commit_id":"acca7f260fdeb2a0a27db2988136b0360319aac3"}]}