)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"bc4171036df04ddc579f646682ef35e62e23f102","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"872b51ef_fe11d608","updated":"2022-08-03 22:00:22.000000000","message":"I think `devices_skip_list` is too nonobvious of a name for what could be a feature exploited for bad. We need to ensure it\u0027s abundantly clear what\u0027s being skipped by any admin looking at a node object. ","commit_id":"2a720362dc807eb9217b42c211cd9ffcc7f66e8d"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"e021172c69832731d1c392f61f9e6893c94cdf2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3fc7c088_63bb54f4","updated":"2022-08-10 08:53:52.000000000","message":"\u003e Patch Set 11:\n\u003e \n\u003e We discussed a more generic option that will also prevent using a disk as a root device. Do you want to proceed?\n\nI am currently in the process of testing this on a real node. The code is quite simple and I can push it thought.","commit_id":"23869c9f24d5ebcfc95214967326797d2860fc57"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"74768dbcaec3fe1153f7fc29c099ff2c1979d7de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"331a8bc8_da24a4c5","updated":"2022-08-04 15:03:11.000000000","message":"LGTM now; I wouldn\u0027t be surprised if there\u0027s a little more back and forth on the name. We\u0027ll have to see what Dmitry thinks.","commit_id":"23869c9f24d5ebcfc95214967326797d2860fc57"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"1c094c744c4a46dc299db86cbe84e94031ee3cbf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"df70f3d4_a063229e","updated":"2022-08-10 21:54:12.000000000","message":"Please add the requested logging, I\u0027d prefer it be here but a follow up is OK too if you need it urgently.\n\nKeep in mind that this is a potentially dangerous feature, so we should be verbose anytime it causes us to do less cleaning.","commit_id":"b73767fe37cebfc7a38bc00b8ab6f39e0cef8b20"}],"ironic_python_agent/hardware.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1390,"context_line":"    def list_block_devices_for_cleaning(self, node, include_partitions\u003dFalse):"},{"line_number":1391,"context_line":"        block_devices \u003d self.list_block_devices("},{"line_number":1392,"context_line":"            include_partitions\u003dinclude_partitions)"},{"line_number":1393,"context_line":"        dinfo \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":1394,"context_line":"        skip_list_hints \u003d dinfo.get(\"devices_skip_list\", [])"},{"line_number":1395,"context_line":"        if skip_list_hints is not None:"},{"line_number":1396,"context_line":"            skip_list \u003d []"}],"source_content_type":"text/x-python","patch_set":4,"id":"b7966813_146b57d2","line":1393,"updated":"2022-07-26 09:46:34.000000000","message":"I\u0027m leaning towards using properties instead of driver_info for consistency with the root_device property.","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1391,"context_line":"        block_devices \u003d self.list_block_devices("},{"line_number":1392,"context_line":"            include_partitions\u003dinclude_partitions)"},{"line_number":1393,"context_line":"        dinfo \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":1394,"context_line":"        skip_list_hints \u003d dinfo.get(\"devices_skip_list\", [])"},{"line_number":1395,"context_line":"        if skip_list_hints is not None:"},{"line_number":1396,"context_line":"            skip_list \u003d []"},{"line_number":1397,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"}],"source_content_type":"text/x-python","patch_set":4,"id":"2504990e_dcec3999","line":1394,"updated":"2022-07-26 09:46:34.000000000","message":"Some bike shedding: it\u0027s not obvious from the name what is skipped. Maybe \"clean_skip_list\"?","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"bc4171036df04ddc579f646682ef35e62e23f102","unresolved":true,"context_lines":[{"line_number":1391,"context_line":"        block_devices \u003d self.list_block_devices("},{"line_number":1392,"context_line":"            include_partitions\u003dinclude_partitions)"},{"line_number":1393,"context_line":"        dinfo \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":1394,"context_line":"        skip_list_hints \u003d dinfo.get(\"devices_skip_list\", [])"},{"line_number":1395,"context_line":"        if skip_list_hints is not None:"},{"line_number":1396,"context_line":"            skip_list \u003d []"},{"line_number":1397,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"}],"source_content_type":"text/x-python","patch_set":4,"id":"711b9337_890b78fe","line":1394,"in_reply_to":"2504990e_dcec3999","updated":"2022-08-03 22:00:22.000000000","message":"I think this is still a valid comment worth addressing. The name `devices_skip_list` is not really obvious as to what it\u0027s doing.\n\nIn fact, I kinda think `clean_skip_list` is too ambiguous. It needs to be 100% obvious to an admin what\u0027s been configured to happen.\n\n`skip_clean_erase_on_devices` ?","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":35099,"name":"Jakub Jelinek","email":"jakub.jelinek@cern.ch","username":"kubajj"},"change_message_id":"75dde5fc22e6b42e1c929babad10f311f5361775","unresolved":true,"context_lines":[{"line_number":1391,"context_line":"        block_devices \u003d self.list_block_devices("},{"line_number":1392,"context_line":"            include_partitions\u003dinclude_partitions)"},{"line_number":1393,"context_line":"        dinfo \u003d node.get(\u0027driver_info\u0027, {})"},{"line_number":1394,"context_line":"        skip_list_hints \u003d dinfo.get(\"devices_skip_list\", [])"},{"line_number":1395,"context_line":"        if skip_list_hints is not None:"},{"line_number":1396,"context_line":"            skip_list \u003d []"},{"line_number":1397,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"}],"source_content_type":"text/x-python","patch_set":4,"id":"cb2896bf_6ef5b544","line":1394,"in_reply_to":"711b9337_890b78fe","updated":"2022-08-04 07:42:11.000000000","message":"Ok, I will change this","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1396,"context_line":"            skip_list \u003d []"},{"line_number":1397,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"},{"line_number":1398,"context_line":"            for hint in skip_list_hints:"},{"line_number":1399,"context_line":"                if type(hint) !\u003d dict:"},{"line_number":1400,"context_line":"                    LOG.error(\"hint is not a dictionary\")"},{"line_number":1401,"context_line":"                    continue"},{"line_number":1402,"context_line":"                found_devs \u003d il_utils.find_devices_by_hints(serialized_devs,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5eeb6708_a5bfde7c","line":1399,"updated":"2022-07-26 09:46:34.000000000","message":"The hints are already validated in ironic-lib, no need for this.","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1401,"context_line":"                    continue"},{"line_number":1402,"context_line":"                found_devs \u003d il_utils.find_devices_by_hints(serialized_devs,"},{"line_number":1403,"context_line":"                                                            hint)"},{"line_number":1404,"context_line":"                list_found_devs \u003d list(found_devs)"},{"line_number":1405,"context_line":"                skip_list \u003d skip_list + list_found_devs"},{"line_number":1406,"context_line":"        if skip_list is not None:"},{"line_number":1407,"context_line":"            skip_list_wwn \u003d [d[\"wwn\"] for d in skip_list]"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba452173_d002816b","line":1404,"updated":"2022-07-26 09:46:34.000000000","message":"nit: you can only keep names and use a set instead of a list\n\n excluded_devs \u003d {dev.name for dev in found_devs}","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1403,"context_line":"                                                            hint)"},{"line_number":1404,"context_line":"                list_found_devs \u003d list(found_devs)"},{"line_number":1405,"context_line":"                skip_list \u003d skip_list + list_found_devs"},{"line_number":1406,"context_line":"        if skip_list is not None:"},{"line_number":1407,"context_line":"            skip_list_wwn \u003d [d[\"wwn\"] for d in skip_list]"},{"line_number":1408,"context_line":"            block_devices \u003d [d for d in block_devices"},{"line_number":1409,"context_line":"                             if d.wwn not in skip_list_wwn]"}],"source_content_type":"text/x-python","patch_set":4,"id":"18450cb2_847e2e81","line":1406,"updated":"2022-07-26 09:46:34.000000000","message":"You meant skip_list_hints, skip_list cannot be None (but can be undefined)","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"5dc1c7310c9e3c34aa50920a5b46fc27937898c3","unresolved":true,"context_lines":[{"line_number":1404,"context_line":"                list_found_devs \u003d list(found_devs)"},{"line_number":1405,"context_line":"                skip_list \u003d skip_list + list_found_devs"},{"line_number":1406,"context_line":"        if skip_list is not None:"},{"line_number":1407,"context_line":"            skip_list_wwn \u003d [d[\"wwn\"] for d in skip_list]"},{"line_number":1408,"context_line":"            block_devices \u003d [d for d in block_devices"},{"line_number":1409,"context_line":"                             if d.wwn not in skip_list_wwn]"},{"line_number":1410,"context_line":"        return block_devices"}],"source_content_type":"text/x-python","patch_set":4,"id":"e06d0c02_3bba4168","line":1407,"updated":"2022-07-26 09:46:34.000000000","message":"A device will not necessarily have a WWN. I think using names is actually appropriate here.","commit_id":"038b72149cf1f90e6d395efd3462f9af0561e453"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"1c094c744c4a46dc299db86cbe84e94031ee3cbf","unresolved":true,"context_lines":[{"line_number":1390,"context_line":"            )"},{"line_number":1391,"context_line":"        return block_devices"},{"line_number":1392,"context_line":""},{"line_number":1393,"context_line":"    def list_block_devices_check_skip_list(self, node,"},{"line_number":1394,"context_line":"                                           include_partitions\u003dFalse):"},{"line_number":1395,"context_line":"        block_devices \u003d self.list_block_devices("},{"line_number":1396,"context_line":"            include_partitions\u003dinclude_partitions)"}],"source_content_type":"text/x-python","patch_set":12,"id":"efdc2181_8d945f05","line":1393,"updated":"2022-08-10 21:54:12.000000000","message":"This method needs to log when it\u0027s skipping a device, likely including the details about the device it\u0027s skipping and the hint that made it skip.","commit_id":"b73767fe37cebfc7a38bc00b8ab6f39e0cef8b20"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a458f7b1b697287268caf12e43a33ced964d64b1","unresolved":false,"context_lines":[{"line_number":1396,"context_line":"            include_partitions\u003dinclude_partitions)"},{"line_number":1397,"context_line":"        properties \u003d node.get(\u0027properties\u0027, {})"},{"line_number":1398,"context_line":"        skip_list_hints \u003d properties.get(\"skip_block_devices\", [])"},{"line_number":1399,"context_line":"        if skip_list_hints is not None:"},{"line_number":1400,"context_line":"            skip_list \u003d None"},{"line_number":1401,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"},{"line_number":1402,"context_line":"            for hint in skip_list_hints:"}],"source_content_type":"text/x-python","patch_set":13,"id":"0fb92759_469c9b81","line":1399,"updated":"2022-08-16 10:14:01.000000000","message":"Please short-circuit to reduce indentation:\n\n if not skip_list_hints:\n    return block_devices","commit_id":"0212337bd564763eecb8f207ca31820497e27c26"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a458f7b1b697287268caf12e43a33ced964d64b1","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        properties \u003d node.get(\u0027properties\u0027, {})"},{"line_number":1398,"context_line":"        skip_list_hints \u003d properties.get(\"skip_block_devices\", [])"},{"line_number":1399,"context_line":"        if skip_list_hints is not None:"},{"line_number":1400,"context_line":"            skip_list \u003d None"},{"line_number":1401,"context_line":"            serialized_devs \u003d [dev.serialize() for dev in block_devices]"},{"line_number":1402,"context_line":"            for hint in skip_list_hints:"},{"line_number":1403,"context_line":"                found_devs \u003d il_utils.find_devices_by_hints(serialized_devs,"}],"source_content_type":"text/x-python","patch_set":13,"id":"81f26279_aa1b00ce","line":1400,"updated":"2022-08-16 10:14:01.000000000","message":"if you initialize skip_list to set(), you don\u0027t need to special-case None on line 1407","commit_id":"0212337bd564763eecb8f207ca31820497e27c26"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a458f7b1b697287268caf12e43a33ced964d64b1","unresolved":false,"context_lines":[{"line_number":1410,"context_line":"                else:"},{"line_number":1411,"context_line":"                    skipped_devices \u003d excluded_devs.difference(skip_list)"},{"line_number":1412,"context_line":"                    skip_list \u003d skip_list.union(excluded_devs)"},{"line_number":1413,"context_line":"                if skipped_devices is not None and len(skipped_devices) \u003e 0:"},{"line_number":1414,"context_line":"                    for d in skipped_devices:"},{"line_number":1415,"context_line":"                        LOG.warning(\"Skipping device %(device)s \""},{"line_number":1416,"context_line":"                                    \"using hint %(hint)s\","}],"source_content_type":"text/x-python","patch_set":13,"id":"98571ecb_6ffd7a07","line":1413,"updated":"2022-08-16 10:14:01.000000000","message":"just\n\n if skipped_devices:","commit_id":"0212337bd564763eecb8f207ca31820497e27c26"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a458f7b1b697287268caf12e43a33ced964d64b1","unresolved":false,"context_lines":[{"line_number":1414,"context_line":"                    for d in skipped_devices:"},{"line_number":1415,"context_line":"                        LOG.warning(\"Skipping device %(device)s \""},{"line_number":1416,"context_line":"                                    \"using hint %(hint)s\","},{"line_number":1417,"context_line":"                                    {\u0027device\u0027: d, \u0027hint\u0027: hint})"},{"line_number":1418,"context_line":"            if skip_list is not None:"},{"line_number":1419,"context_line":"                block_devices \u003d [d for d in block_devices"},{"line_number":1420,"context_line":"                                 if d.name not in skip_list]"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f7725d2_de21f28a","line":1417,"updated":"2022-08-16 10:14:01.000000000","message":"I\u0027d prefer a single logging line using \u0027, \u0027.join(skipped_devices)","commit_id":"0212337bd564763eecb8f207ca31820497e27c26"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a458f7b1b697287268caf12e43a33ced964d64b1","unresolved":false,"context_lines":[{"line_number":1415,"context_line":"                        LOG.warning(\"Skipping device %(device)s \""},{"line_number":1416,"context_line":"                                    \"using hint %(hint)s\","},{"line_number":1417,"context_line":"                                    {\u0027device\u0027: d, \u0027hint\u0027: hint})"},{"line_number":1418,"context_line":"            if skip_list is not None:"},{"line_number":1419,"context_line":"                block_devices \u003d [d for d in block_devices"},{"line_number":1420,"context_line":"                                 if d.name not in skip_list]"},{"line_number":1421,"context_line":"        return block_devices"}],"source_content_type":"text/x-python","patch_set":13,"id":"09fb5ed8_6d68ca17","line":1418,"updated":"2022-08-16 10:14:01.000000000","message":"This also own\u0027t be needed if you initialize skip_list to set()","commit_id":"0212337bd564763eecb8f207ca31820497e27c26"}]}