)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7bd9e2148f5f0194cf6a2e3ae30080edc4ce592d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c6f4151f_7af1dd01","updated":"2021-12-13 08:28:49.000000000","message":"recheck","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ba884f2ee463b9ffc3a8c44a3085afd422eef696","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"df8da9a6_fa556882","updated":"2021-12-11 08:10:15.000000000","message":"recheck","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a72a3836eea80b1d890df80af1c7636b7055e4ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1ae3a125_74479840","updated":"2021-12-27 07:06:33.000000000","message":"docstring to be updated","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"684cd0f39e096a347db36439cdfa5f461ac732e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fbae4190_19041f6b","in_reply_to":"1ae3a125_74479840","updated":"2022-01-04 11:34:00.000000000","message":"Done","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"}],"neutron/api/rpc/handlers/dhcp_rpc.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"22e60d4e52e06853d2dc4ec1dc0123f55a35d4ef","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"79b1ec8a_326a78ce","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"updated":"2021-12-23 08:59:46.000000000","message":"So if \"enable_dhcp_filter\" is False this will now filter only subnets with dhcp disabled, while logically it should just include all subnets.\nMoreover it looks agent now only expects dhcp enabled subnets [1], so is there a reason to keep \u0027enable_dhcp_filter\u0027 parameter at all?\n\n[1] https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L403","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a72a3836eea80b1d890df80af1c7636b7055e4ac","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"09fe8a1c_d8d82a31","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"in_reply_to":"6ce13db8_cad88846","updated":"2021-12-27 07:06:33.000000000","message":"Ack, thanks!","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"45a8a9abfeba763b38bb719b865d90893321d1f6","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"c8d9ab7b_ddb42d7f","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"in_reply_to":"79b1ec8a_326a78ce","updated":"2021-12-24 14:52:44.000000000","message":"Very good catch. I\u0027ll create a UT to check this and avoid a regression. Thanks!\n\nIn [1] we are still using enable_dhcp_filter\u003dFalse. Than means we need to keep this parameter.\n\n[1]https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L273-L274","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1afb0587001708810c1aa1f8b5bd86db9f0edaf1","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"6ce13db8_cad88846","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"in_reply_to":"9b3098d9_f8fad5c5","updated":"2021-12-24 16:22:23.000000000","message":"Ok, now I see that: [1] is safe because this is calling the driver\u0027s \"enable\" method. For the \"dnsmasq\" implementation in Neutron (the only in-tree driver), the method is [2], that calls [3]. In [3], if the network does not have any subnet with DHCP enabled, it returns False and does nothing.\n\nThe DHCP agent will store in the cache this network, regardless of not having any subnet with DHCP. When the network is deleted or updated (admin_state_up\u003dFalse), the method \"disable_dhcp_helper\" is called too, but the drivers skips this call too.\n\n[1]https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L403\n[2]https://github.com/openstack/neutron/blob/ca90d8c041151efce19dea10758fa46a7d32cc39/neutron/agent/linux/dhcp.py#L271\n[3]https://github.com/openstack/neutron/blob/ca90d8c041151efce19dea10758fa46a7d32cc39/neutron/agent/linux/dhcp.py#L264-L269","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4a7f2b43a8fdf83bfbaf2d1e2a8b8dcf67c94a48","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"9b3098d9_f8fad5c5","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"in_reply_to":"ba35b636_acd9fbb5","updated":"2021-12-24 16:07:06.000000000","message":"We do, yes, but as I commented we are still using this parameter in [1] and we can\u0027t remove this functionality.\n\n\"configure_dhcp_for_network\" is called from two places:\n1) \"enable_dhcp_helper\": the network passed comes from \"safe_get_network_info\". By default, we only pass networks with subnets with DHCP enabled. This one is safe.\n2) \"safe_configure_dhcp_for_network\": this one is called from \"sync_state\". The network information passed to this method comes from the call to get_active_networks_info(enable_dhcp_filter\u003dFalse). I\u0027m investigating this now but this patch is not changing this functionality. We could maybe remove the \"enable_dhcp_filter\" but not in this patch.\n\nIn any case, that\u0027s strange: what is happening if we pass a network without any subnet with DHCP enable? As I said, I\u0027m testing this right now. I hope I can update this review today.\n\n[1]https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L273-L274","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ab22cdda8f0628c3c4a870a778e757cc41ef1dd","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":202,"context_line":"        # Only subnets with DHCP enabled."},{"line_number":203,"context_line":"        subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":204,"context_line":"                   network.db_obj.subnets if subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":205,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":206,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":207,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba35b636_acd9fbb5","line":204,"range":{"start_line":204,"start_character":45,"end_line":204,"end_character":78},"in_reply_to":"c8d9ab7b_ddb42d7f","updated":"2021-12-24 15:18:32.000000000","message":"\u003e Very good catch. I\u0027ll create a UT to check this and avoid a regression. Thanks!\n\u003e \n\u003e In [1] we are still using enable_dhcp_filter\u003dFalse. Than means we need to keep this parameter.\n\u003e \n\u003e [1]https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L273-L274\n\nYeah, but further down the stack in [2] it assumes all network subnets as dhcp enabled, so it will try to enable dhcp even for nets with only dhcp disabled subnets.. Am I missing something?\n\n[2] https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/agent/dhcp/agent.py#L403","commit_id":"00f54295a771e79eaae43dcb7b429f8ea6ceb2d8"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a72a3836eea80b1d890df80af1c7636b7055e4ac","unresolved":true,"context_lines":[{"line_number":176,"context_line":"        \"\"\"Retrieve and return information about a network."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network"},{"line_number":179,"context_line":"        with only DHCP enabled subnets."},{"line_number":180,"context_line":"        \"\"\""},{"line_number":181,"context_line":"        # \"enable_dhcp\" could be True, False or None."},{"line_number":182,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9d5d27d4_fa892d1b","line":179,"range":{"start_line":179,"start_character":8,"end_line":179,"end_character":38},"updated":"2021-12-27 07:06:33.000000000","message":"Should this be updated?","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"684cd0f39e096a347db36439cdfa5f461ac732e5","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        \"\"\"Retrieve and return information about a network."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network"},{"line_number":179,"context_line":"        with only DHCP enabled subnets."},{"line_number":180,"context_line":"        \"\"\""},{"line_number":181,"context_line":"        # \"enable_dhcp\" could be True, False or None."},{"line_number":182,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"}],"source_content_type":"text/x-python","patch_set":5,"id":"0c319077_db2e51a7","line":179,"range":{"start_line":179,"start_character":8,"end_line":179,"end_character":38},"in_reply_to":"9d5d27d4_fa892d1b","updated":"2022-01-04 11:34:00.000000000","message":"Yes, I\u0027ve removed it. We can filter or not using \"enable_dhcp\".","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a72a3836eea80b1d890df80af1c7636b7055e4ac","unresolved":true,"context_lines":[{"line_number":178,"context_line":"        Retrieve and return extended information about a network"},{"line_number":179,"context_line":"        with only DHCP enabled subnets."},{"line_number":180,"context_line":"        \"\"\""},{"line_number":181,"context_line":"        # \"enable_dhcp\" could be True, False or None."},{"line_number":182,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":183,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"},{"line_number":184,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":5,"id":"d8e1dd8e_7075cc8c","line":181,"range":{"start_line":181,"start_character":8,"end_line":181,"end_character":52},"updated":"2021-12-27 07:06:33.000000000","message":"Not sure \"False\" is/will be used..","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"684cd0f39e096a347db36439cdfa5f461ac732e5","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        Retrieve and return extended information about a network"},{"line_number":179,"context_line":"        with only DHCP enabled subnets."},{"line_number":180,"context_line":"        \"\"\""},{"line_number":181,"context_line":"        # \"enable_dhcp\" could be True, False or None."},{"line_number":182,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":183,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"},{"line_number":184,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":5,"id":"91131528_b0cb6102","line":181,"range":{"start_line":181,"start_character":8,"end_line":181,"end_character":52},"in_reply_to":"d8e1dd8e_7075cc8c","updated":"2022-01-04 11:34:00.000000000","message":"But this is a valid input and is covered in the code.","commit_id":"932380d86c0703f860a9c6fb35b5607048d583c6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59462949158a747a98049acc9552a8559b93e0cd","unresolved":true,"context_lines":[{"line_number":163,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":164,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"},{"line_number":165,"context_line":"        # default is to filter subnets based on \u0027enable_dhcp\u0027 flag"},{"line_number":166,"context_line":"        enable_dhcp \u003d True if kwargs.get(\u0027enable_dhcp_filter\u0027, True) else None"},{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("}],"source_content_type":"text/x-python","patch_set":6,"id":"1ba0009f_dedac5e6","line":166,"updated":"2022-01-05 15:24:30.000000000","message":"it could be probably shorter:\n\n    enable_dhcp \u003d kwargs.get(\u0027enable_dhcp_filter\u0027, True) or None\n\nbut that\u0027s just a nitty nit for which You don\u0027t need to respin it :)","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"115bd98d0e7f826c87e5193571306f9174d99312","unresolved":true,"context_lines":[{"line_number":163,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":164,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"},{"line_number":165,"context_line":"        # default is to filter subnets based on \u0027enable_dhcp\u0027 flag"},{"line_number":166,"context_line":"        enable_dhcp \u003d True if kwargs.get(\u0027enable_dhcp_filter\u0027, True) else None"},{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("}],"source_content_type":"text/x-python","patch_set":6,"id":"46ff3278_fce9bf44","line":166,"in_reply_to":"1ba0009f_dedac5e6","updated":"2022-01-05 17:25:57.000000000","message":"Right!","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f3e246c185652dac96ffed5312a33e584d449009","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":164,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"},{"line_number":165,"context_line":"        # default is to filter subnets based on \u0027enable_dhcp\u0027 flag"},{"line_number":166,"context_line":"        enable_dhcp \u003d True if kwargs.get(\u0027enable_dhcp_filter\u0027, True) else None"},{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("}],"source_content_type":"text/x-python","patch_set":6,"id":"b059d1c8_4f83c382","line":166,"in_reply_to":"46ff3278_fce9bf44","updated":"2022-01-19 08:53:11.000000000","message":"Done","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6a53c1d808743ee41996a6a085bd86bcbb7afeab","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":164,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"},{"line_number":165,"context_line":"        # default is to filter subnets based on \u0027enable_dhcp\u0027 flag"},{"line_number":166,"context_line":"        enable_dhcp \u003d True if kwargs.get(\u0027enable_dhcp_filter\u0027, True) else None"},{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("}],"source_content_type":"text/x-python","patch_set":6,"id":"2ed47af8_5a562cf7","line":166,"in_reply_to":"b059d1c8_4f83c382","updated":"2022-01-19 09:00:35.000000000","message":"No, this is not the same as the previous behaviour.\n\n\"enable_dhcp\" was True only when \"enable_dhcp_filter\" was True. In other case, the value was None (not False).","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bcbee33727bee0180ddedd228c5340c22e1c95ba","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("},{"line_number":170,"context_line":"                context, network\u003dnetwork, enable_dhcp\u003denable_dhcp,"},{"line_number":171,"context_line":"                ports\u003dgrouped_ports.get(network.id, [])))"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        return ret"}],"source_content_type":"text/x-python","patch_set":6,"id":"b4dbc24b_c57110b9","line":170,"range":{"start_line":170,"start_character":42,"end_line":170,"end_character":65},"updated":"2022-01-14 12:37:30.000000000","message":"enable_dhcp_filter\u003dkwargs.get(\u0027enable_dhcp_filter\u0027, True)","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6a53c1d808743ee41996a6a085bd86bcbb7afeab","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("},{"line_number":170,"context_line":"                context, network\u003dnetwork, enable_dhcp\u003denable_dhcp,"},{"line_number":171,"context_line":"                ports\u003dgrouped_ports.get(network.id, [])))"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        return ret"}],"source_content_type":"text/x-python","patch_set":6,"id":"57908b43_9dc37692","line":170,"range":{"start_line":170,"start_character":42,"end_line":170,"end_character":65},"in_reply_to":"71cd199c_bd64d62a","updated":"2022-01-19 09:00:35.000000000","message":"No, this is not the same as the previous behaviour.","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f3e246c185652dac96ffed5312a33e584d449009","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        ret \u003d []"},{"line_number":168,"context_line":"        for network in networks:"},{"line_number":169,"context_line":"            ret.append(self.get_network_info("},{"line_number":170,"context_line":"                context, network\u003dnetwork, enable_dhcp\u003denable_dhcp,"},{"line_number":171,"context_line":"                ports\u003dgrouped_ports.get(network.id, [])))"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        return ret"}],"source_content_type":"text/x-python","patch_set":6,"id":"71cd199c_bd64d62a","line":170,"range":{"start_line":170,"start_character":42,"end_line":170,"end_character":65},"in_reply_to":"b4dbc24b_c57110b9","updated":"2022-01-19 08:53:11.000000000","message":"Done","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"a6489dc23cbaaad577b631f870166a385f5d07fa","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network."},{"line_number":179,"context_line":"        The optional argument \"enable_dhcp\" can be used to filter the subnets"},{"line_number":180,"context_line":"        using the same flag (True/False). If None is passed, no filtering is"},{"line_number":181,"context_line":"        done."},{"line_number":182,"context_line":"        \"\"\""},{"line_number":183,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":184,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"601bbb42_29437241","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":13},"updated":"2022-01-10 11:46:02.000000000","message":"still not sure who might need to call it with enable_dhcp\u003d\u003dFalse, what\u0027s the use case? Seems currently the only usages are to get network(s) with all or only dhcp enabled subnets. My question here is - do we really need to introduce this enable_dhcp parameter, or could we stay with existing enable_dhcp_filter and just pass it from get_active_networks_info down here?","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b1a981b7bae1d69456031c571bc53cc69bfc7cfa","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network."},{"line_number":179,"context_line":"        The optional argument \"enable_dhcp\" can be used to filter the subnets"},{"line_number":180,"context_line":"        using the same flag (True/False). If None is passed, no filtering is"},{"line_number":181,"context_line":"        done."},{"line_number":182,"context_line":"        \"\"\""},{"line_number":183,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":184,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"781351df_dfc71d52","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":13},"in_reply_to":"27b6ecd3_75e8b828","updated":"2022-01-19 08:55:38.000000000","message":"https://github.com/openstack/neutron/blob/2268b92cb9ce901dc50c164d30cb3def3a5949e4/neutron/agent/dhcp/agent.py#L273-L274","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bcbee33727bee0180ddedd228c5340c22e1c95ba","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network."},{"line_number":179,"context_line":"        The optional argument \"enable_dhcp\" can be used to filter the subnets"},{"line_number":180,"context_line":"        using the same flag (True/False). If None is passed, no filtering is"},{"line_number":181,"context_line":"        done."},{"line_number":182,"context_line":"        \"\"\""},{"line_number":183,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":184,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"aacf6c04_7d886d7e","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":13},"in_reply_to":"5dab40ae_19a422fb","updated":"2022-01-14 12:37:30.000000000","message":"My point is: if no-one needs to filter by enable_dhcp\u003d\u003dFalse - then no need to add this possibility with this patch. It will be cleaner (and less code) to stay with existing enable_dhcp_filter, just add the same parameter to get_network_info. Please see my other 2 comments (above and beyond). Sorry for being boring :)","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"798c852a480eddd3b05438368c9f2b6ec60d6991","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network."},{"line_number":179,"context_line":"        The optional argument \"enable_dhcp\" can be used to filter the subnets"},{"line_number":180,"context_line":"        using the same flag (True/False). If None is passed, no filtering is"},{"line_number":181,"context_line":"        done."},{"line_number":182,"context_line":"        \"\"\""},{"line_number":183,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":184,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5dab40ae_19a422fb","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":13},"in_reply_to":"601bbb42_29437241","updated":"2022-01-14 11:59:36.000000000","message":"This is not a matter of what methods are currently passing enable_dhcp\u003d\u003dFalse but handling this possibility. This method \"get_network_info\" accepts \"enable_dhcp\" as \"True\", \"False\" and \"None\"; that means we are considering all possible parameter values. If callers are only passing \"enable_dhcp\u003dTrue\" and \"enable_dhcp\u003dNone\", that\u0027s ok. Method code covers this parameter inputs.","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f3e246c185652dac96ffed5312a33e584d449009","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        Retrieve and return extended information about a network."},{"line_number":179,"context_line":"        The optional argument \"enable_dhcp\" can be used to filter the subnets"},{"line_number":180,"context_line":"        using the same flag (True/False). If None is passed, no filtering is"},{"line_number":181,"context_line":"        done."},{"line_number":182,"context_line":"        \"\"\""},{"line_number":183,"context_line":"        enable_dhcp \u003d kwargs.get(\u0027enable_dhcp\u0027, True)"},{"line_number":184,"context_line":"        network \u003d kwargs.get(\u0027network\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"27b6ecd3_75e8b828","line":181,"range":{"start_line":180,"start_character":8,"end_line":181,"end_character":13},"in_reply_to":"aacf6c04_7d886d7e","updated":"2022-01-19 08:53:11.000000000","message":"This is just the input parameter possible values. Filtering out the \"False\" value will add more complexity to the code of the method.","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"bcbee33727bee0180ddedd228c5340c22e1c95ba","unresolved":true,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        seg_plug \u003d directory.get_plugin("},{"line_number":203,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":204,"context_line":"        if enable_dhcp is None:"},{"line_number":205,"context_line":"            subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":206,"context_line":"                       network.db_obj.subnets]"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":209,"context_line":"                       network.db_obj.subnets if"},{"line_number":210,"context_line":"                       subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":211,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":212,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":213,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":6,"id":"30bc1ecd_229c7fcf","line":210,"range":{"start_line":204,"start_character":8,"end_line":210,"end_character":57},"updated":"2022-01-14 12:37:30.000000000","message":"With enable_dhcp_filter: \n subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in\n            network.db_obj.subnets if \n            (not enable_dhcp_filter or subnet.enable_dhcp)]","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f3e246c185652dac96ffed5312a33e584d449009","unresolved":false,"context_lines":[{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        seg_plug \u003d directory.get_plugin("},{"line_number":203,"context_line":"            segment_ext.SegmentPluginBase.get_plugin_type())"},{"line_number":204,"context_line":"        if enable_dhcp is None:"},{"line_number":205,"context_line":"            subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":206,"context_line":"                       network.db_obj.subnets]"},{"line_number":207,"context_line":"        else:"},{"line_number":208,"context_line":"            subnets \u003d [plugin._make_subnet_dict(subnet) for subnet in"},{"line_number":209,"context_line":"                       network.db_obj.subnets if"},{"line_number":210,"context_line":"                       subnet.enable_dhcp \u003d\u003d enable_dhcp]"},{"line_number":211,"context_line":"        seg_subnets \u003d [subnet for subnet in subnets if"},{"line_number":212,"context_line":"                       subnet.get(\u0027segment_id\u0027)]"},{"line_number":213,"context_line":"        nonlocal_subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":6,"id":"ac73cba2_a809011e","line":210,"range":{"start_line":204,"start_character":8,"end_line":210,"end_character":57},"in_reply_to":"30bc1ecd_229c7fcf","updated":"2022-01-19 08:53:11.000000000","message":"Done","commit_id":"e0f387f808dec2f31ad4dec256b132df0beeaaad"}],"neutron/db/agentschedulers_db.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"de7fb97e7c57720dd41318ee99f61ebf92445472","unresolved":true,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        net_ids \u003d [item.network_id for item in query]"},{"line_number":465,"context_line":"        if net_ids:"},{"line_number":466,"context_line":"            return self.get_networks("},{"line_number":467,"context_line":"                context,"},{"line_number":468,"context_line":"                filters\u003d{\u0027id\u0027: net_ids, \u0027admin_state_up\u0027: [True]}"},{"line_number":469,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"070c115b_d4cd44bf","side":"PARENT","line":466,"range":{"start_line":466,"start_character":24,"end_line":466,"end_character":36},"updated":"2021-12-23 09:01:38.000000000","message":"are we sure callers of this method do not expect network extended attributes that are add with _make_network_dict()?","commit_id":"01debb20bf12267d16f932a678c77450cf761419"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"45a8a9abfeba763b38bb719b865d90893321d1f6","unresolved":true,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        net_ids \u003d [item.network_id for item in query]"},{"line_number":465,"context_line":"        if net_ids:"},{"line_number":466,"context_line":"            return self.get_networks("},{"line_number":467,"context_line":"                context,"},{"line_number":468,"context_line":"                filters\u003d{\u0027id\u0027: net_ids, \u0027admin_state_up\u0027: [True]}"},{"line_number":469,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"96685e33_fac63f7d","side":"PARENT","line":466,"range":{"start_line":466,"start_character":24,"end_line":466,"end_character":36},"in_reply_to":"070c115b_d4cd44bf","updated":"2021-12-24 14:52:44.000000000","message":"This method is only called from [1], and \"_get_active_networks\" is only called from \"get_active_networks_info\". We are safe on this one.\n\n[1]https://github.com/openstack/neutron/blob/19a20eaca8a51d6c095a68970478de847fb256f5/neutron/api/rpc/handlers/dhcp_rpc.py#L91-L92","commit_id":"01debb20bf12267d16f932a678c77450cf761419"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6ab22cdda8f0628c3c4a870a778e757cc41ef1dd","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        net_ids \u003d [item.network_id for item in query]"},{"line_number":465,"context_line":"        if net_ids:"},{"line_number":466,"context_line":"            return self.get_networks("},{"line_number":467,"context_line":"                context,"},{"line_number":468,"context_line":"                filters\u003d{\u0027id\u0027: net_ids, \u0027admin_state_up\u0027: [True]}"},{"line_number":469,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":4,"id":"88502483_c031e38f","side":"PARENT","line":466,"range":{"start_line":466,"start_character":24,"end_line":466,"end_character":36},"in_reply_to":"96685e33_fac63f7d","updated":"2021-12-24 15:18:32.000000000","message":"Ack","commit_id":"01debb20bf12267d16f932a678c77450cf761419"}]}