)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5f7111cac172dec85f8cb64f8f5de6404befd644","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"095e3f1c_45a6d4a0","updated":"2026-04-17 00:58:57.000000000","message":"recheck ironic devstack plugin fixed","commit_id":"e359e4652cc45f8e7d3c8ee7ab640d991679e252"}],"doc/source/admin.rst":[{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":572,"context_line":"* ``username`` - NDFC authentication username"},{"line_number":573,"context_line":"* ``password`` - NDFC authentication password"},{"line_number":574,"context_line":"* ``ip`` - Switch IP address (used to identify switch in NDFC inventory)"},{"line_number":575,"context_line":"* ``ndfc_fabric_name`` - NDFC fabric name"},{"line_number":576,"context_line":""},{"line_number":577,"context_line":"Optional:"},{"line_number":578,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"e7be65dc_efa8ce25","line":575,"range":{"start_line":575,"start_character":4,"end_line":575,"end_character":20},"updated":"2026-06-12 13:09:30.000000000","message":"nit: could add (default: `default`)","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"}],"networking_generic_switch/devices/ndfc_devices/cisco.py":[{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"EXPERIMENTAL: This driver is experimental and based on publicly available"},{"line_number":16,"context_line":"NDFC API documentation. It manages Cisco NX-OS switches through the NDFC"},{"line_number":17,"context_line":"REST API instead of direct SSH access. NDFC provides centralized fabric"},{"line_number":18,"context_line":"management with audit trails, templating, and policy-based configuration."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"API Documentation:"}],"source_content_type":"text/x-python","patch_set":9,"id":"45e2eab9_9535a3ee","line":17,"range":{"start_line":17,"start_character":9,"end_line":17,"end_character":37},"updated":"2026-06-12 13:09:30.000000000","message":"nit: the \"instead of ...\" part can be dropped.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":102,"context_line":"    \"\"\"Cisco NDFC REST API-based switch driver."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    Manages Cisco NX-OS switches through Nexus Dashboard Fabric"},{"line_number":105,"context_line":"    Controller (NDFC) REST API instead of SSH. Supports VLAN"},{"line_number":106,"context_line":"    provisioning, port configuration, trunk management, and"},{"line_number":107,"context_line":"    VXLAN L2VNI configuration via NDFC fabric controller."},{"line_number":108,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"90e3abe9_ff031a0e","line":105,"range":{"start_line":105,"start_character":31,"end_line":105,"end_character":46},"updated":"2026-06-12 13:09:30.000000000","message":"nit: the instead of SSH part can be dropped.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":106,"context_line":"    provisioning, port configuration, trunk management, and"},{"line_number":107,"context_line":"    VXLAN L2VNI configuration via NDFC fabric controller."},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    Port can be disabled: True"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    Configuration Parameters"},{"line_number":112,"context_line":"    ~~~~~~~~~~~~~~~~~~~~~~~~"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fbbe67b_2483f675","line":109,"range":{"start_line":109,"start_character":4,"end_line":109,"end_character":30},"updated":"2026-06-12 13:09:30.000000000","message":"In doc/source/admin.rst the opposite is in limitations?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":170,"context_line":"    interface configuration, BGP EVPN control plane, and VXLAN data"},{"line_number":171,"context_line":"    plane automatically based on the fabric template."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    Security Group Implementation"},{"line_number":174,"context_line":"    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    Security groups are not currently implemented. Support for"},{"line_number":177,"context_line":"    NDFC policies and contracts is planned for a future release."},{"line_number":178,"context_line":"    \"\"\""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    # Signal L2VNI support (methods implemented dynamically)"}],"source_content_type":"text/x-python","patch_set":9,"id":"318837bd_a713242e","line":177,"range":{"start_line":173,"start_character":0,"end_line":177,"end_character":64},"updated":"2026-06-12 13:09:30.000000000","message":"May want to add this in admin.rst under limitation?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":190,"context_line":"        return False"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    def __init__(self, device_cfg, device_name\u003d\"\"):"},{"line_number":193,"context_line":"        # Extract ngs_* config before parent __init__ removes"},{"line_number":194,"context_line":"        # all ngs_* options from device_cfg."},{"line_number":195,"context_line":"        self.ndfc_max_connections \u003d device_cfg.get("},{"line_number":196,"context_line":"            \u0027ngs_max_connections\u0027, 5)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        # Extract NDFC-specific config before super strips it"},{"line_number":199,"context_line":"        ndfc_host \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"ba19a36c_b2b7b65e","line":196,"range":{"start_line":193,"start_character":1,"end_line":196,"end_character":37},"updated":"2026-06-12 13:09:30.000000000","message":"Instead of this, other drivers use self.ngs_config. All options should be in NGS_INTERNAL_OPTS[1] and then after super().__init__() do:\n `self.ndfc_max_connections \u003d self.ngs_config[\u0027ngs_max_connections\u0027]`\n\n\n[1] https://opendev.org/openstack/networking-generic-switch/src/branch/master/networking_generic_switch/devices/__init__.py#L30","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        # Extract NDFC-specific config before super strips it"},{"line_number":199,"context_line":"        ndfc_host \u003d ("},{"line_number":200,"context_line":"            device_cfg.get(\u0027host\u0027) or device_cfg.get(\u0027ip\u0027)"},{"line_number":201,"context_line":"        )"},{"line_number":202,"context_line":"        ndfc_port \u003d device_cfg.get(\u0027port\u0027, 443)"},{"line_number":203,"context_line":"        ndfc_username \u003d device_cfg.get(\u0027username\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"d1e99a58_543d417f","line":200,"range":{"start_line":200,"start_character":0,"end_line":200,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"I think we should use `host` only here? The `ip` is used for individual switch identification and would never be the same as the NDFC\u0027s `ip`?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        self.ndfc_max_connections \u003d device_cfg.get("},{"line_number":196,"context_line":"            \u0027ngs_max_connections\u0027, 5)"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        # Extract NDFC-specific config before super strips it"},{"line_number":199,"context_line":"        ndfc_host \u003d ("},{"line_number":200,"context_line":"            device_cfg.get(\u0027host\u0027) or device_cfg.get(\u0027ip\u0027)"},{"line_number":201,"context_line":"        )"},{"line_number":202,"context_line":"        ndfc_port \u003d device_cfg.get(\u0027port\u0027, 443)"},{"line_number":203,"context_line":"        ndfc_username \u003d device_cfg.get(\u0027username\u0027)"},{"line_number":204,"context_line":"        ndfc_password \u003d device_cfg.get(\u0027password\u0027)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"        super(CiscoNDFC, self).__init__(device_cfg, device_name)"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"531d4ebd_3937e588","line":204,"range":{"start_line":198,"start_character":0,"end_line":204,"end_character":50},"updated":"2026-06-12 13:09:30.000000000","message":"These are not needed? Only `ngs_` prefixed options are stripped.\nRef: https://opendev.org/openstack/networking-generic-switch/src/branch/master/networking_generic_switch/devices/__init__.py#L116-L128","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        self.ndfc_username \u003d ndfc_username"},{"line_number":212,"context_line":"        self.ndfc_password \u003d ndfc_password"},{"line_number":213,"context_line":"        self.ndfc_fabric \u003d device_cfg.get("},{"line_number":214,"context_line":"            \u0027ndfc_fabric_name\u0027, \u0027default\u0027)"},{"line_number":215,"context_line":"        self.ndfc_verify_ssl \u003d strutils.bool_from_string("},{"line_number":216,"context_line":"            device_cfg.get(\u0027ndfc_verify_ssl\u0027, True))"},{"line_number":217,"context_line":"        self.ndfc_timeout \u003d device_cfg.get(\u0027ndfc_timeout\u0027, 30)"}],"source_content_type":"text/x-python","patch_set":9,"id":"07629661_f3b7aa37","line":214,"range":{"start_line":214,"start_character":13,"end_line":214,"end_character":29},"updated":"2026-06-12 13:09:30.000000000","message":"Should all these be in NGS_INTERNAL_OPTS, or a new NDFC_OPTS?.\n\nAs it is now, there will be warning logged for all of these, ref: https://opendev.org/openstack/networking-generic-switch/src/branch/master/networking_generic_switch/devices/__init__.py#L125-L127","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":213,"context_line":"        self.ndfc_fabric \u003d device_cfg.get("},{"line_number":214,"context_line":"            \u0027ndfc_fabric_name\u0027, \u0027default\u0027)"},{"line_number":215,"context_line":"        self.ndfc_verify_ssl \u003d strutils.bool_from_string("},{"line_number":216,"context_line":"            device_cfg.get(\u0027ndfc_verify_ssl\u0027, True))"},{"line_number":217,"context_line":"        self.ndfc_timeout \u003d device_cfg.get(\u0027ndfc_timeout\u0027, 30)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # Switch identification (IP used to find serial in NDFC)"}],"source_content_type":"text/x-python","patch_set":9,"id":"d7fefce8_bcd0f3b4","line":216,"range":{"start_line":216,"start_character":28,"end_line":216,"end_character":43},"updated":"2026-06-12 13:09:30.000000000","message":"In the proposed restconf changes I used `ngs_restconf_verify_ssl` - Should we add a generic `ngs_verify_ssl` option instead and align?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":216,"context_line":"            device_cfg.get(\u0027ndfc_verify_ssl\u0027, True))"},{"line_number":217,"context_line":"        self.ndfc_timeout \u003d device_cfg.get(\u0027ndfc_timeout\u0027, 30)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # Switch identification (IP used to find serial in NDFC)"},{"line_number":220,"context_line":"        self.switch_ip \u003d self.config.get(\u0027ip\u0027)"},{"line_number":221,"context_line":"        if not self.switch_ip:"},{"line_number":222,"context_line":"            LOG.error(\u0027Switch IP address is required for NDFC driver\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"be91034d_9770890e","line":219,"range":{"start_line":219,"start_character":8,"end_line":219,"end_character":64},"updated":"2026-06-12 13:09:30.000000000","message":"There is a potential scaling issue here?\nWe would define separate ini sections for each switch, all section has a common host (the NDFC) but the `ip` is for each individual switch managed by the NDFC.\n\nngs_max_connections is supposed to control the number of concurrent connections to the NDFC. Since that is per switch, not per NDFC we cannot keep that below `switches * 1` so 100 switches \u003d 100 connections.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":217,"context_line":"        self.ndfc_timeout \u003d device_cfg.get(\u0027ndfc_timeout\u0027, 30)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"        # Switch identification (IP used to find serial in NDFC)"},{"line_number":220,"context_line":"        self.switch_ip \u003d self.config.get(\u0027ip\u0027)"},{"line_number":221,"context_line":"        if not self.switch_ip:"},{"line_number":222,"context_line":"            LOG.error(\u0027Switch IP address is required for NDFC driver\u0027)"},{"line_number":223,"context_line":"            raise exc.GenericSwitchNDFCConfigError()"}],"source_content_type":"text/x-python","patch_set":9,"id":"14dcfd34_a3b0e0a7","line":220,"range":{"start_line":220,"start_character":0,"end_line":220,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"Here we access `ip` after the above extract to `ndfc_host` - proving super does not strip these.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        # plug_port_to_network, and delete_port). Using {network_id} would"},{"line_number":228,"context_line":"        # produce inconsistent names across these call sites."},{"line_number":229,"context_line":"        name_format \u003d self.ngs_config[\u0027ngs_network_name_format\u0027]"},{"line_number":230,"context_line":"        if \u0027{network_id}\u0027 in name_format:"},{"line_number":231,"context_line":"            LOG.error("},{"line_number":232,"context_line":"                \"ngs_network_name_format \u0027%s\u0027 contains {network_id} which \""},{"line_number":233,"context_line":"                \"is not supported by the NDFC driver. Use {segmentation_id} \""},{"line_number":234,"context_line":"                \"instead.\", name_format)"},{"line_number":235,"context_line":"            raise exc.GenericSwitchNDFCConfigError()"},{"line_number":236,"context_line":"        if \u0027{segmentation_id}\u0027 not in name_format:"},{"line_number":237,"context_line":"            LOG.error("},{"line_number":238,"context_line":"                \"ngs_network_name_format \u0027%s\u0027 must contain {segmentation_id} \""},{"line_number":239,"context_line":"                \"for the NDFC driver.\", name_format)"},{"line_number":240,"context_line":"            raise exc.GenericSwitchNDFCConfigError()"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        # NDFC template and policy configuration"},{"line_number":243,"context_line":"        self.ndfc_network_template \u003d device_cfg.get("}],"source_content_type":"text/x-python","patch_set":9,"id":"b36d4c09_bdea6e61","line":240,"range":{"start_line":230,"start_character":0,"end_line":240,"end_character":52},"updated":"2026-06-12 13:09:30.000000000","message":"We only need the second condition? - Or we ignore the option, since segmentation_id is the only choice?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        # NDFC template and policy configuration"},{"line_number":243,"context_line":"        self.ndfc_network_template \u003d device_cfg.get("},{"line_number":244,"context_line":"            \u0027ndfc_network_template\u0027,"},{"line_number":245,"context_line":"            \u0027Default_Network_Universal\u0027)"},{"line_number":246,"context_line":"        self.ndfc_network_extension_template \u003d device_cfg.get("},{"line_number":247,"context_line":"            \u0027ndfc_network_extension_template\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"9ff4a167_f4396b34","line":244,"range":{"start_line":244,"start_character":13,"end_line":244,"end_character":34},"updated":"2026-06-12 13:09:30.000000000","message":"same, use NGS_INTERNAL_OPTS pattern, or add separate NDFC_OPTS pattern to avoid the warning logs.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":248,"context_line":"            \u0027Default_Network_Extension_Universal\u0027)"},{"line_number":249,"context_line":"        self.ndfc_vrf \u003d device_cfg.get(\u0027ndfc_vrf\u0027, \u0027default\u0027)"},{"line_number":250,"context_line":"        self.ndfc_interface_policy \u003d device_cfg.get("},{"line_number":251,"context_line":"            \u0027ndfc_interface_policy\u0027, \u0027int_trunk_host\u0027)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        # Generate session key from credentials (hashed)"},{"line_number":254,"context_line":"        # This ensures password changes result in new sessions."}],"source_content_type":"text/x-python","patch_set":9,"id":"46065f8e_708318c5","line":251,"range":{"start_line":251,"start_character":38,"end_line":251,"end_character":52},"updated":"2026-06-12 13:09:30.000000000","message":"This does not match the default in doc/source/admin.rst. Change it in docs to align.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":419,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":420,"context_line":"            LOG.error(\"NDFC connection failed to %s:%s: %s\","},{"line_number":421,"context_line":"                      self.ndfc_host, self.ndfc_port, e)"},{"line_number":422,"context_line":"            raise exc.GenericSwitchNDFCConnectError()"},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"    def _parse_response(self, response, method, endpoint, url):"},{"line_number":425,"context_line":"        \"\"\"Parse an NDFC HTTP response, checking for embedded failures."}],"source_content_type":"text/x-python","patch_set":9,"id":"34be630f_0f450bd4","line":422,"range":{"start_line":422,"start_character":0,"end_line":422,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"Use `from e` here ?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":456,"context_line":"    def _is_auth_error(e):"},{"line_number":457,"context_line":"        \"\"\"Return True if HTTPError indicates an authentication failure.\"\"\""},{"line_number":458,"context_line":"        status_code \u003d ("},{"line_number":459,"context_line":"            e.response.status_code if e.response is not None else None"},{"line_number":460,"context_line":"        )"},{"line_number":461,"context_line":"        body \u003d {}"},{"line_number":462,"context_line":"        if e.response is not None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"6fdd4631_07326a74","line":459,"range":{"start_line":459,"start_character":35,"end_line":459,"end_character":70},"updated":"2026-06-12 13:09:30.000000000","message":"This if else looks redundant?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":471,"context_line":"            or \u0027not authorized\u0027 in body_str"},{"line_number":472,"context_line":"        )"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def _ndfc_request(self, method, endpoint, **kwargs):"},{"line_number":475,"context_line":"        \"\"\"Execute NDFC REST API request with error handling."},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        Automatically handles token expiration by re-authenticating and"}],"source_content_type":"text/x-python","patch_set":9,"id":"dc37a8a5_feba6d46","line":474,"range":{"start_line":474,"start_character":8,"end_line":474,"end_character":21},"updated":"2026-06-12 13:09:30.000000000","message":"May want to add some tests for this method?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":507,"context_line":"            if not self._is_auth_error(e):"},{"line_number":508,"context_line":"                detail \u003d self._http_error_detail(e)"},{"line_number":509,"context_line":"                LOG.error(\"NDFC API error: %s\", detail if detail else e)"},{"line_number":510,"context_line":"                raise exc.GenericSwitchNDFCConfigError()"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"            status \u003d ("},{"line_number":513,"context_line":"                e.response.status_code if e.response is not None else None"}],"source_content_type":"text/x-python","patch_set":9,"id":"4e0a5ba5_cf685ac3","line":510,"range":{"start_line":510,"start_character":0,"end_line":510,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"Use `from e` here ?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":510,"context_line":"                raise exc.GenericSwitchNDFCConfigError()"},{"line_number":511,"context_line":""},{"line_number":512,"context_line":"            status \u003d ("},{"line_number":513,"context_line":"                e.response.status_code if e.response is not None else None"},{"line_number":514,"context_line":"            )"},{"line_number":515,"context_line":"            LOG.info(\"NDFC token expired (HTTP %s), \""},{"line_number":516,"context_line":"                     \"re-authenticating and retrying request\", status)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7b2ddcd7_0ada48d1","line":513,"range":{"start_line":513,"start_character":39,"end_line":513,"end_character":74},"updated":"2026-06-12 13:09:30.000000000","message":"same, redundant if else pattern?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":531,"context_line":"                detail \u003d self._http_error_detail(retry_error)"},{"line_number":532,"context_line":"                LOG.error(\"NDFC API error after re-auth: %s\","},{"line_number":533,"context_line":"                          detail if detail else retry_error)"},{"line_number":534,"context_line":"                raise exc.GenericSwitchNDFCConfigError()"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        except requests.exceptions.RequestException as e:"},{"line_number":537,"context_line":"            # Attempt to extract a response body from the cause chain"}],"source_content_type":"text/x-python","patch_set":9,"id":"00dda269_9755009e","line":534,"range":{"start_line":534,"start_character":0,"end_line":534,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"Use `from e` here ?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":550,"context_line":"                else f\"NDFC request failed: {e}\""},{"line_number":551,"context_line":"            )"},{"line_number":552,"context_line":"            LOG.error(error_msg)"},{"line_number":553,"context_line":"            raise exc.GenericSwitchNDFCConfigError()"},{"line_number":554,"context_line":""},{"line_number":555,"context_line":"    def _get_network_attachments(self, network_name):"},{"line_number":556,"context_line":"        \"\"\"Query current network attachments from NDFC."}],"source_content_type":"text/x-python","patch_set":9,"id":"8eac5fa2_51c3fe39","line":553,"range":{"start_line":553,"start_character":0,"end_line":553,"end_character":2},"updated":"2026-06-12 13:09:30.000000000","message":"Use `from e` here ?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":799,"context_line":"        # Modern API wraps switches in a \"switches\" key"},{"line_number":800,"context_line":"        switches \u003d response.get(\u0027switches\u0027, [])"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"        # Match by IP address (v2 uses \u0027fabricManagementIp\u0027)"},{"line_number":803,"context_line":"        for switch in switches:"},{"line_number":804,"context_line":"            if switch.get(\u0027fabricManagementIp\u0027) \u003d\u003d self.switch_ip:"},{"line_number":805,"context_line":"                self._switch_serial \u003d switch.get(\u0027serialNumber\u0027)"},{"line_number":806,"context_line":"                LOG.debug(\"Found switch %s with serial %s\","},{"line_number":807,"context_line":"                          self.switch_ip, self._switch_serial)"},{"line_number":808,"context_line":"                return self._switch_serial"},{"line_number":809,"context_line":""},{"line_number":810,"context_line":"        LOG.error(f\"Switch with IP {self.switch_ip} not found in \""},{"line_number":811,"context_line":"                  f\"fabric \u0027{self.ndfc_fabric}\u0027\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"cba5323d_309d764b","line":808,"range":{"start_line":802,"start_character":0,"end_line":808,"end_character":42},"updated":"2026-06-12 13:09:30.000000000","message":"I think we could move this filter server side? `f\u0027{self._api_base}switches?fabricManagementIp\u003d{self.switch_ip}\u0027`","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":831,"context_line":""},{"line_number":832,"context_line":"        :param switch_serial: Serial number of the switch to deploy to"},{"line_number":833,"context_line":"        \"\"\""},{"line_number":834,"context_line":"        if not self._get_save_configuration():"},{"line_number":835,"context_line":"            LOG.debug(\"Configuration deployment disabled\")"},{"line_number":836,"context_line":"            return"},{"line_number":837,"context_line":""},{"line_number":838,"context_line":"        _control_base \u003d ("},{"line_number":839,"context_line":"            \"appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control\""}],"source_content_type":"text/x-python","patch_set":9,"id":"05388f88_bcf87a3b","line":836,"range":{"start_line":834,"start_character":0,"end_line":836,"end_character":18},"updated":"2026-06-12 13:09:30.000000000","message":"This seems like the wrong usage of `ngs_save_configuration`.\n\n_get_save_configuration is for \"write memory\" i.e switch runtime config is applied, just not saved to survive reboot. In this case it would be \"do not configure switch att all\" just stage to NDFC\u0027s intent database?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":1216,"context_line":"        \"\"\""},{"line_number":1217,"context_line":"        switch_serial \u003d self._get_switch_serial()"},{"line_number":1218,"context_line":""},{"line_number":1219,"context_line":"        existing \u003d self._get_interface_allowed_vlans(switch_serial, port)"},{"line_number":1220,"context_line":"        new_vlans \u003d existing | {int(sp.get(\u0027segmentation_id\u0027))"},{"line_number":1221,"context_line":"                                for sp in subports}"},{"line_number":1222,"context_line":""},{"line_number":1223,"context_line":"        LOG.info(\"Adding subport VLANs %s to trunk port %s on %s\","},{"line_number":1224,"context_line":"                 [sp.get(\u0027segmentation_id\u0027) for sp in subports],"}],"source_content_type":"text/x-python","patch_set":9,"id":"2cbb12fd_d3956a7b","line":1221,"range":{"start_line":1219,"start_character":0,"end_line":1221,"end_character":51},"updated":"2026-06-12 13:09:30.000000000","message":"In a follow up, should we consider using the fact that the port object holds all the subport VLAN\u0027s and do a single PUT operation to \"converge\". Would remove the need to got GET -\u003e diff -\u003e PUT. Same goes for some other places where we do the GET -\u003e diff -\u003e PUT dance.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":1282,"context_line":""},{"line_number":1283,"context_line":"        self._deploy_config(switch_serial)"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"    def plug_switch_to_network(self, vni,"},{"line_number":1286,"context_line":"                               segmentation_id,"},{"line_number":1287,"context_line":"                               physnet\u003dNone):"},{"line_number":1288,"context_line":"        \"\"\"Configure L2VNI mapping in NDFC fabric."}],"source_content_type":"text/x-python","patch_set":9,"id":"77e67fb4_cafffee8","line":1285,"range":{"start_line":1285,"start_character":8,"end_line":1285,"end_character":30},"updated":"2026-06-12 13:09:30.000000000","message":"This is 99% the same as `add_network()` above, just the additional `_set_vlan_vni_mapping` and `_do_vlan_management`? I think a shared helper function to do the the bulk of this would be good. Just pass different `networkId`?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":1506,"context_line":"        LOG.debug(\"VLAN %s has no ports\", segmentation_id)"},{"line_number":1507,"context_line":"        return False"},{"line_number":1508,"context_line":""},{"line_number":1509,"context_line":"    def _parse_vlan_list(self, vlan_str):"},{"line_number":1510,"context_line":"        \"\"\"Parse a VLAN list string into a set of integers."},{"line_number":1511,"context_line":""},{"line_number":1512,"context_line":"        Handles formats like: \"1,10,20-25,30\" and \"none\"."},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"        :param vlan_str: VLAN list string"},{"line_number":1515,"context_line":"        :returns: Set of VLAN IDs as integers"},{"line_number":1516,"context_line":"        \"\"\""},{"line_number":1517,"context_line":"        vlans \u003d set()"},{"line_number":1518,"context_line":"        for part in vlan_str.split(\u0027,\u0027):"},{"line_number":1519,"context_line":"            part \u003d part.strip()"},{"line_number":1520,"context_line":"            if \u0027-\u0027 in part:"},{"line_number":1521,"context_line":"                start, end \u003d part.split(\u0027-\u0027)"},{"line_number":1522,"context_line":"                vlans.update(range(int(start), int(end) + 1))"},{"line_number":1523,"context_line":"            elif part.isdigit():"},{"line_number":1524,"context_line":"                vlans.add(int(part))"},{"line_number":1525,"context_line":"        return vlans"},{"line_number":1526,"context_line":""},{"line_number":1527,"context_line":"    def _vlan_list_to_str(self, vlans):"},{"line_number":1528,"context_line":"        \"\"\"Serialise a set of VLAN IDs to a compact comma-separated string."},{"line_number":1529,"context_line":""},{"line_number":1530,"context_line":"        Sequential VLANs are collapsed into range notation, e.g."},{"line_number":1531,"context_line":"        {100, 200, 201, 202, 300} → \u0027100,200-202,300\u0027."},{"line_number":1532,"context_line":""},{"line_number":1533,"context_line":"        :param vlans: Set of VLAN IDs as integers"},{"line_number":1534,"context_line":"        :returns: Compact sorted string, or \u0027none\u0027 if empty"},{"line_number":1535,"context_line":"        \"\"\""},{"line_number":1536,"context_line":"        if not vlans:"},{"line_number":1537,"context_line":"            return \u0027none\u0027"},{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        sorted_vlans \u003d sorted(vlans)"},{"line_number":1540,"context_line":"        parts \u003d []"},{"line_number":1541,"context_line":"        start \u003d prev \u003d sorted_vlans[0]"},{"line_number":1542,"context_line":""},{"line_number":1543,"context_line":"        for v in sorted_vlans[1:]:"},{"line_number":1544,"context_line":"            if v \u003d\u003d prev + 1:"},{"line_number":1545,"context_line":"                prev \u003d v"},{"line_number":1546,"context_line":"            else:"},{"line_number":1547,"context_line":"                parts.append(str(start) if start \u003d\u003d prev"},{"line_number":1548,"context_line":"                             else f\u0027{start}-{prev}\u0027)"},{"line_number":1549,"context_line":"                start \u003d prev \u003d v"},{"line_number":1550,"context_line":"        parts.append(str(start) if start \u003d\u003d prev else f\u0027{start}-{prev}\u0027)"},{"line_number":1551,"context_line":""},{"line_number":1552,"context_line":"        return \u0027,\u0027.join(parts)"},{"line_number":1553,"context_line":""},{"line_number":1554,"context_line":"    # Security group methods - not implemented"},{"line_number":1555,"context_line":"    def add_security_group(self, sg_id, rules):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e6aa7916_81ba33a5","line":1552,"range":{"start_line":1509,"start_character":0,"end_line":1552,"end_character":30},"updated":"2026-06-12 13:09:30.000000000","message":"If we do an iteration, may want to move these out to some utils/lib package to re-use with future netmiko implementation of \"trunk_vlans_converge\" introduces in Change-Id: I1432ace9142feb1a405f9573d29222b03d290b4e.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"}],"networking_generic_switch/tests/unit/ndfc/test_cisco_ndfc.py":[{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":9,"id":"5a1325e2_95a77678","updated":"2026-06-12 13:09:30.000000000","message":"We could remove quite a bit of repetition by setting up the _get_switch_serial and _ndfc_request patches in setUp, and a common BASE_DEVICE_CFG with the common config keys pre-set.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":237,"context_line":"    @mock.patch.object(cisco.CiscoNDFC, \u0027_ndfc_request\u0027, autospec\u003dTrue)"},{"line_number":238,"context_line":"    @mock.patch.object(cisco.CiscoNDFC, \u0027_get_switch_serial\u0027, autospec\u003dTrue)"},{"line_number":239,"context_line":"    def test_add_network_with_trunk_ports("},{"line_number":240,"context_line":"            self, mock_serial, mock_request, mock_deploy):"},{"line_number":241,"context_line":"        \"\"\"Test add_network configures trunk ports via interface policy.\"\"\""},{"line_number":242,"context_line":"        mock_serial.return_value \u003d \u0027ABC1234567\u0027"},{"line_number":243,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d6de756d_a8a70e8f","line":240,"range":{"start_line":240,"start_character":31,"end_line":240,"end_character":43},"updated":"2026-06-12 13:09:30.000000000","message":"May want to make this mock return existing vlans on the port, to cover appending to existing?","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"e7531e73f41b2095dab07509561772748b747e5d","unresolved":true,"context_lines":[{"line_number":945,"context_line":""},{"line_number":946,"context_line":"    @mock.patch.object(cisco.CiscoNDFC, \u0027_ndfc_request\u0027, autospec\u003dTrue)"},{"line_number":947,"context_line":"    @mock.patch.object(cisco.CiscoNDFC, \u0027_get_switch_serial\u0027, autospec\u003dTrue)"},{"line_number":948,"context_line":"    def test_plug_switch_to_network_idempotent("},{"line_number":949,"context_line":"            self, mock_serial, mock_request):"},{"line_number":950,"context_line":"        \"\"\"Test L2VNI creation is idempotent when network exists.\"\"\""},{"line_number":951,"context_line":"        mock_serial.return_value \u003d \u0027ABC1234567\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"c269dd02_90d51926","line":948,"range":{"start_line":948,"start_character":8,"end_line":948,"end_character":46},"updated":"2026-06-12 13:09:30.000000000","message":"unsure about test name, is it really testing idempotency? It just verfies the same calls happen at each invocation.","commit_id":"b43a60ec7e8b23aa672ae21b1f37db0090d41bf8"}]}