)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"1d19b4044379456955f2acac21265a2950339a7b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"257a81a4_d564a8f5","updated":"2023-10-11 10:49:25.000000000","message":"Needs a rebase now","commit_id":"50de105c4f44064686a609855f4e3fb9ac190f5a"}],"ironic/drivers/modules/inspector/hooks/lldp_parsers.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bab9296cac95e9d5b03e22db61a7f1f0eccb5a14","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"fbc807d7_fcfc02f3","updated":"2023-10-13 14:27:07.000000000","message":"nit: this module is not a hook, so should be moved one or two levels up","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"075fe806048fc4c5e931fee93e72dade4ea3f16f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"691c5b4e_1a5d85fa","in_reply_to":"fbc807d7_fcfc02f3","updated":"2023-10-17 11:37:52.000000000","message":"Done","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"}],"ironic/drivers/modules/inspector/hooks/lldp_tlvs.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bab9296cac95e9d5b03e22db61a7f1f0eccb5a14","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f769454d_ec3f4091","updated":"2023-10-13 14:27:07.000000000","message":"nit: this module is not a hook, so should be moved one or two levels up","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"075fe806048fc4c5e931fee93e72dade4ea3f16f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3fe46982_8d163c7e","in_reply_to":"f769454d_ec3f4091","updated":"2023-10-17 11:37:52.000000000","message":"Done","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bab9296cac95e9d5b03e22db61a7f1f0eccb5a14","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"import construct"},{"line_number":21,"context_line":"from construct import core"},{"line_number":22,"context_line":"import netaddr"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e8ced8cc_b676adcc","line":22,"updated":"2023-10-13 14:27:07.000000000","message":"netaddr is not in requirements, needs to be added there.\n\nIdeally, we should migrate away from it eventually since Python offers an ipaddress module nowadays.","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"075fe806048fc4c5e931fee93e72dade4ea3f16f","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"import construct"},{"line_number":21,"context_line":"from construct import core"},{"line_number":22,"context_line":"import netaddr"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":6,"id":"527b7fa7_e0af34ee","line":22,"in_reply_to":"e8ced8cc_b676adcc","updated":"2023-10-17 11:37:52.000000000","message":"Added netaddr to the requirements","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"36dc8c8f90175c567cf776d447c309a5fa3fd50d","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"8301bf98_5460df11","line":87,"updated":"2023-10-17 12:50:05.000000000","message":"let\u0027s try to use ipaddress instead?\nsomething like ipaddress.ip_address should work here\n\nadding a new requirement is always a burden, especially considering that we have a valid alternative\nthis is slightly out of scope though so I guess we can consider doing it in a follow up, but I would like to hear other opinions on this","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a838f79c9775d900d0bbd1a4bfc1b90a485972fd","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"63e9d5c9_619a73ff","line":87,"in_reply_to":"39753ee7_bb0babc4","updated":"2023-10-19 10:50:46.000000000","message":"Let\u0027s look at that in a follow-up then. There may be dependencies that are lighter that netaddr (which is a huge project written in C if I remember it right).","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"5a8f0a1af7b77097075edb221cf1fecef29687a0","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"aeebf559_c6304cbc","line":87,"in_reply_to":"63e9d5c9_619a73ff","updated":"2023-10-19 11:37:58.000000000","message":"Alright. If by follow up, you mean another separate patch, then this one is ready for review :) Else I can switch to macaddress as a dependency (Tt seems more lightweight than netaddr but less actively developed (last release about a year ago))","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"c0f850409620ab6f06e3435c44d652d9d14eac5c","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"39753ee7_bb0babc4","line":87,"in_reply_to":"64f2babd_f58ecf72","updated":"2023-10-18 15:22:18.000000000","message":"The MACAddress struct (line 97) uses the netaddr.EUI class to encode and decode the MAC address, but there is nothing in the standard library that handles MAC addresses (ipaddress does not deal with MAC addresses either). There is a \u0027macaddress\u0027 module, but that would mean introducing a new requirement as well.\nWhat do we think?","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"28a3bd591e1959729edc43b58d57b7282f78d154","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"64f2babd_f58ecf72","line":87,"in_reply_to":"8301bf98_5460df11","updated":"2023-10-17 14:45:25.000000000","message":"Ack, I will use ipaddress instead. It might be more difficult to get back to it later.","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"d6143aea0b5908687b0c64b27cd9a359f018a824","unresolved":true,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"IPv4Address \u003d core.ExprAdapter("},{"line_number":86,"context_line":"    core.Byte[4],"},{"line_number":87,"context_line":"    encoder\u003dlambda obj, ctx: netaddr.IPAddress(obj).words,"},{"line_number":88,"context_line":"    decoder\u003dlambda obj, ctx: str(netaddr.IPAddress(bytes_to_int(obj)))"},{"line_number":89,"context_line":")"},{"line_number":90,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"956b05e3_5ac3012f","line":87,"in_reply_to":"aeebf559_c6304cbc","updated":"2023-10-19 12:15:13.000000000","message":"a separate patch as follow up does work for me, thanks","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"}],"ironic/drivers/modules/inspector/hooks/local_link_connection.py":[{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"9be2576c45ef1c739fd9fe5cf3e69445678f4fe2","unresolved":true,"context_lines":[{"line_number":39,"context_line":"        for tlv_type, tlv_value in lldp_data:"},{"line_number":40,"context_line":"            try:"},{"line_number":41,"context_line":"                data \u003d bytearray(binascii.unhexlify(tlv_value))"},{"line_number":42,"context_line":"                print(\u0027data: \u0027, data)"},{"line_number":43,"context_line":"            except binascii.Error:"},{"line_number":44,"context_line":"                print(\u0027TLV value for TLV type %d is not in correct format. \u0027"},{"line_number":45,"context_line":"                      \u0027Ensure that the TLV value is in hexidecimal format \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"4eafb02a_54219200","line":42,"updated":"2023-10-08 21:25:43.000000000","message":"It looks like all these print calls were for dev purposes and should be removed now","commit_id":"769ebc209c198fbff38993d7cfbedd52935c4285"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"d56337b2843acc0786a002072c307527068457c9","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        for tlv_type, tlv_value in lldp_data:"},{"line_number":40,"context_line":"            try:"},{"line_number":41,"context_line":"                data \u003d bytearray(binascii.unhexlify(tlv_value))"},{"line_number":42,"context_line":"                print(\u0027data: \u0027, data)"},{"line_number":43,"context_line":"            except binascii.Error:"},{"line_number":44,"context_line":"                print(\u0027TLV value for TLV type %d is not in correct format. \u0027"},{"line_number":45,"context_line":"                      \u0027Ensure that the TLV value is in hexidecimal format \u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"bbe481e0_a05396f4","line":42,"in_reply_to":"4eafb02a_54219200","updated":"2023-10-10 09:18:00.000000000","message":"They were indeed uploaded by mistake. Removed now.","commit_id":"769ebc209c198fbff38993d7cfbedd52935c4285"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bab9296cac95e9d5b03e22db61a7f1f0eccb5a14","unresolved":true,"context_lines":[{"line_number":113,"context_line":"                          mac_address, task.node.uuid)"},{"line_number":114,"context_line":"                continue"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"            lldp_data \u003d lldp_raw.get(\u0027lldp_raw\u0027) or iface.get(\u0027lldp\u0027)"},{"line_number":117,"context_line":"            if lldp_data is None:"},{"line_number":118,"context_line":"                LOG.warning(\u0027No LLDP data found for interface %s of node %s\u0027,"},{"line_number":119,"context_line":"                            mac_address, task.node.uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3b9e59ad_7bc347dc","line":116,"updated":"2023-10-13 14:27:07.000000000","message":"You probably meant lldp_raw.get(iface[\u0027name\u0027])","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"075fe806048fc4c5e931fee93e72dade4ea3f16f","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                          mac_address, task.node.uuid)"},{"line_number":114,"context_line":"                continue"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"            lldp_data \u003d lldp_raw.get(\u0027lldp_raw\u0027) or iface.get(\u0027lldp\u0027)"},{"line_number":117,"context_line":"            if lldp_data is None:"},{"line_number":118,"context_line":"                LOG.warning(\u0027No LLDP data found for interface %s of node %s\u0027,"},{"line_number":119,"context_line":"                            mac_address, task.node.uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a2e988a_3450e64f","line":116,"in_reply_to":"3b9e59ad_7bc347dc","updated":"2023-10-17 11:37:52.000000000","message":"Done","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"}],"ironic/drivers/modules/inspector/hooks/parse_lldp.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bab9296cac95e9d5b03e22db61a7f1f0eccb5a14","unresolved":true,"context_lines":[{"line_number":32,"context_line":"    sets. Store parsed data in the plugin_data as a new parsed_lldp dictionary"},{"line_number":33,"context_line":"    with interface names as keys."},{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def _parse_lldp_tlvs(self, tlvs, node_uuid):"},{"line_number":37,"context_line":"        \"\"\"Parse LLDP TLVs into a dictionary of name/value pairs"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"b4f38869_3917c2aa","line":35,"updated":"2023-10-13 14:27:07.000000000","message":"Requires validate-interfaces, I assume?","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"},{"author":{"_account_id":35929,"name":"Mahnoor Asghar","display_name":"Mahnoor Asghar","email":"masghar@redhat.com","username":"mahnoorasghar"},"change_message_id":"075fe806048fc4c5e931fee93e72dade4ea3f16f","unresolved":true,"context_lines":[{"line_number":32,"context_line":"    sets. Store parsed data in the plugin_data as a new parsed_lldp dictionary"},{"line_number":33,"context_line":"    with interface names as keys."},{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def _parse_lldp_tlvs(self, tlvs, node_uuid):"},{"line_number":37,"context_line":"        \"\"\"Parse LLDP TLVs into a dictionary of name/value pairs"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"b143810f_baaf7f6d","line":35,"in_reply_to":"b4f38869_3917c2aa","updated":"2023-10-17 11:37:52.000000000","message":"It doesnt check for the interface name being present in plugin_data[\u0027all_interfaces\u0027] or plugin_data[\u0027valid_interfaces\u0027], so as of now validate-interfaces is not a requirement. But it can be made as a requirement (like for local-link-connection)","commit_id":"5a0b251bb2e27255484b00f0e6fd43295afe1f09"}],"ironic/tests/unit/drivers/modules/inspector/hooks/test_parse_lldp.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"fce6dd06e6c7c3f1cf020402b3a288c796840bcd","unresolved":true,"context_lines":[{"line_number":15,"context_line":"from ironic.conductor import task_manager"},{"line_number":16,"context_line":"from ironic.conf import CONF"},{"line_number":17,"context_line":"from ironic.drivers.modules.inspector.hooks import parse_lldp as hook"},{"line_number":18,"context_line":"from ironic.drivers.modules.inspector import lldp_parsers as nv"},{"line_number":19,"context_line":"from ironic.tests.unit.db import base as db_base"},{"line_number":20,"context_line":"from ironic.tests.unit.objects import utils as obj_utils"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"c196a1a2_398f9fe2","line":18,"updated":"2023-10-20 16:13:38.000000000","message":"nit: this is a weird rename. Maybe you could follow-up with a change that reduces the number of renamed modules in this PR?","commit_id":"c3ee90ddac70db25e03a172903ec3e3b7d031fff"}]}