)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"416efbbf53eb03c738f379e2a9032ad719e3f895","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f305f55a_041da94b","updated":"2026-07-27 18:57:15.000000000","message":"I need to do a deeper review of the parent patch, but this one lgtm. The duplication was worse than I thought, it seems that multiple drivers had identical copies of the get_pci_devices","commit_id":"1648d6f7e20608eba243979ada88aaf7b7c531e5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f716c68a_954b894d","updated":"2026-07-28 16:54:21.000000000","message":"this looks good\n\nsome more comment in line but for the most part this is correct directionally","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d46f56ce_09fa6c1a","in_reply_to":"f716c68a_954b894d","updated":"2026-07-29 14:40:37.000000000","message":"Thanks for the detailed review. Applied all the inline comments in this revision.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"844565a4_7cd8a00e","updated":"2026-07-29 14:40:37.000000000","message":"On the PCI enumeration rework mentioned in the review: while migrating I found one more duplicate. `pci/utils.py` `get_pci_devices()` is another copy of the same lspci call with its own `VENDOR_MAPS`. I kept it out of this series since it belongs to that area. Happy to fold it into the rework, or send a follow-up.","commit_id":"a57289ead2cac55060551d07da12558e51a85052"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"db96d59a445e3d444878cafca167a7f6c80af4e4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"861c8cbb_c3e4e1ef","updated":"2026-08-04 16:36:06.000000000","message":"i guess -1 is more correct.\n\ndirectionally i like how this looks but we shoudl fix the two inline issue before proceedign with this","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"961a4b438d88a62b2544740b1c0ebdcac974fd9c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"71c30eb3_263b9759","updated":"2026-08-01 11:20:39.000000000","message":"recheck","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"bbd9899fef9033c2d8c6ca75c053257c6b84b5fd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b0368e8f_4221896a","in_reply_to":"861c8cbb_c3e4e1ef","updated":"2026-08-05 13:47:59.000000000","message":"Addressed both inline issues in this revision. Added a unit test for `pci_details` and switched ascend to device_name for the model field with a matching test assertion. Ready for another look.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"069112f1a7a8e71c7619d7e1c8abab26a220f35e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"41932ee2_72d332f0","updated":"2026-08-06 14:08:46.000000000","message":"Thanks both. This revision keeps the aichip model fix but documents it in the commit message and closes bug 2162962 via Closes-Bug, and switches the ascend test data to a raw multiline string. Left the test tech debt (privsep fixture, deeper mocks, xilinx test data) for separate followups as discussed.","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4ed2a00a_0f853b0d","updated":"2026-08-05 19:05:54.000000000","message":"t","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"552dd08e0f938fd54e6713e85ad400c93e6181bc","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6af97c8e_8a19150d","in_reply_to":"4ed2a00a_0f853b0d","updated":"2026-08-05 19:08:52.000000000","message":"this looks good to me over all\n\nits proably best to split the other bug fix out as melaine suggest but the majority of the tech debt in the tests shoudl be adress seperatly such as porting the privsep fixture form nova so you can ignore those comemnts in the next revision\ni was mostly just taking note for myslef of refactoring/cleanpus we shoudl be doing in the future.","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e324acd4903fc00f1fb5a3d68194a07f48acdab4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"40d7ba75_2cc07cb1","updated":"2026-08-06 15:03:14.000000000","message":"This is great, thank you","commit_id":"9ee937cb7731a0711ae68d4e832aba064c8571bc"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"afc5539fa0349c5ff00fe2131e565730e7ea45d3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6ee5fdc7_ad2c13dc","updated":"2026-08-06 23:22:16.000000000","message":"recheck","commit_id":"9ee937cb7731a0711ae68d4e832aba064c8571bc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f7ceb8903877113b815821a50954e504ee9f653","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"12d0b3d8_3ae4398c","updated":"2026-08-06 15:39:09.000000000","message":"recheck tempest failed to install with a wereid pip error\n\ni think the scope of this is alreayd large enough that we\nshoudl porceed with thsi as is and continute ot impvoethignin new patches\n\nille leave this open for another day or so but assuming ci is green ill merge this tomorrow if there is no other feedback","commit_id":"9ee937cb7731a0711ae68d4e832aba064c8571bc"}],"cyborg/accelerator/common/utils.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148386d4af9528da0b8cc94e17a9b69962fb26f5","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-nn\u0027, \u0027-D\u0027)[0]"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@cyborg.privsep.sys_admin_pctxt.entrypoint"},{"line_number":165,"context_line":"def pci_details(device):"},{"line_number":166,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device)[0]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"b7bb8f3a_a7d06254","line":164,"updated":"2026-08-01 11:31:20.000000000","message":"The pci_details(device) privsep helper introduced in common/utils.py has no unit test. The closely related lspci_privileged, parse_lspci_line, and get_pci_devices helpers all received tests in test_utils.py, but pci_details was added without any test coverage.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: The pci_details helper has zero test coverage and its only consumer (_get_pf_type) is also untested in any invocation path, meaning regressions in the lspci -k -s parsing or privsep decorator wiring would not be caught by CI.\n\n**Recommendation**:\nAdd a unit test in test_utils.py that mocks processutils.execute (or the privsep entrypoint) and verifies pci_details returns the stdout string for a given device address, mirroring how lspci_privileged is tested indirectly via test_get_pci_devices.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"bbd9899fef9033c2d8c6ca75c053257c6b84b5fd","unresolved":true,"context_lines":[{"line_number":161,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-nn\u0027, \u0027-D\u0027)[0]"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@cyborg.privsep.sys_admin_pctxt.entrypoint"},{"line_number":165,"context_line":"def pci_details(device):"},{"line_number":166,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device)[0]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff53997e_b770ff04","line":164,"in_reply_to":"76b674ff_cc48bae1","updated":"2026-08-05 13:47:59.000000000","message":"Added `test_pci_details` in `test_utils.py`. It runs the privsep context in process, mocks `processutils.execute`, and asserts it is called with `lspci -k -s \u003cdevice\u003e` and returns stdout. That covers the small surface you mentioned.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98cb07abfb6d0c0f7538422a54595b505c472f60","unresolved":true,"context_lines":[{"line_number":161,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-nn\u0027, \u0027-D\u0027)[0]"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@cyborg.privsep.sys_admin_pctxt.entrypoint"},{"line_number":165,"context_line":"def pci_details(device):"},{"line_number":166,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device)[0]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"76b674ff_cc48bae1","line":164,"in_reply_to":"b7bb8f3a_a7d06254","updated":"2026-08-04 16:35:25.000000000","message":"thats fair.\n\nthe only reall testing we can do however is assert that when pass a device sting we inovke exec with that string so we proably shoudl add that but the testable surface here is very minimal","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":false,"context_lines":[{"line_number":161,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-nn\u0027, \u0027-D\u0027)[0]"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"@cyborg.privsep.sys_admin_pctxt.entrypoint"},{"line_number":165,"context_line":"def pci_details(device):"},{"line_number":166,"context_line":"    return processutils.execute(\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device)[0]"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"f3640aa8_9115e429","line":164,"in_reply_to":"ff53997e_b770ff04","updated":"2026-08-05 19:05:54.000000000","message":"that works for this one of test\n\nthe better long term approch si to port\n\nhttps://github.com/openstack/nova/blob/926ac87a92af987be9bc306a919b137db2cb3b04/nova/tests/fixtures/nova.py#L1563-L1590\n\nadd `self.useFixture(nova_fixtures.PrivsepNoHelperFixture())` here\nhttps://github.com/openstack/cyborg/blob/master/cyborg/tests/base.py#L51\n \nand in the new test enable the privsep fixture\n\nself.useFixture(fixtures.PrivsepFixture())\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/privsep/test_fs.py#L29\n\n\ni think we can do that in a followup as there my be exisitng issue we need to adress to make that work\n\nso for now im ok with your current proposal","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/accelerator/drivers/aichip/huawei/ascend.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":66,"context_line":"        driver_dep.driver_name \u003d self.VENDOR"},{"line_number":67,"context_line":"        return [driver_dep]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(yikun): can be extracted into PCIDeviceDriver"},{"line_number":70,"context_line":"    def _get_pci_lines(self, keywords\u003d()):"},{"line_number":71,"context_line":"        pci_lines \u003d []"},{"line_number":72,"context_line":"        if keywords:"},{"line_number":73,"context_line":"            lspci_out \u003d utils.lspci_privileged().split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"            for i in range(len(lspci_out)):"},{"line_number":75,"context_line":"                # filter out pci devices info that contains all keywords"},{"line_number":76,"context_line":"                if all([k in (lspci_out[i]) for k in keywords]):"},{"line_number":77,"context_line":"                    pci_lines.append(lspci_out[i])"},{"line_number":78,"context_line":"        return pci_lines"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def discover(self):"},{"line_number":81,"context_line":"        \"\"\"The PCI line would be matched as:"}],"source_content_type":"text/x-python","patch_set":3,"id":"a84c35c3_3e3abf98","line":78,"range":{"start_line":69,"start_character":1,"end_line":78,"end_character":24},"updated":"2026-07-28 16:54:21.000000000","message":"so this appares to eb only used in one palce","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        driver_dep.driver_name \u003d self.VENDOR"},{"line_number":67,"context_line":"        return [driver_dep]"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(yikun): can be extracted into PCIDeviceDriver"},{"line_number":70,"context_line":"    def _get_pci_lines(self, keywords\u003d()):"},{"line_number":71,"context_line":"        pci_lines \u003d []"},{"line_number":72,"context_line":"        if keywords:"},{"line_number":73,"context_line":"            lspci_out \u003d utils.lspci_privileged().split(\u0027\\n\u0027)"},{"line_number":74,"context_line":"            for i in range(len(lspci_out)):"},{"line_number":75,"context_line":"                # filter out pci devices info that contains all keywords"},{"line_number":76,"context_line":"                if all([k in (lspci_out[i]) for k in keywords]):"},{"line_number":77,"context_line":"                    pci_lines.append(lspci_out[i])"},{"line_number":78,"context_line":"        return pci_lines"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def discover(self):"},{"line_number":81,"context_line":"        \"\"\"The PCI line would be matched as:"}],"source_content_type":"text/x-python","patch_set":3,"id":"406eea44_07afccb0","line":78,"range":{"start_line":69,"start_character":1,"end_line":78,"end_character":24},"in_reply_to":"a84c35c3_3e3abf98","updated":"2026-07-29 14:40:37.000000000","message":"Removed. Inlined it into `discover()`.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":91,"context_line":"          \u0027revision\u0027: \u002720\u0027                     # Revision number"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":"        \"\"\""},{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ae3e188a_0e4dc8d8","line":94,"updated":"2026-07-28 16:54:21.000000000","message":"and you can replace this with \n\n```\n        ascends \u003d list(utils.get_pci_devices(lambda line: \u0027d100\u0027 in line))\n```\nor slightly shorter\n\n```\n     ascends \u003d list(utils.get_pci_devices(utils.has_flags([\u0027d100\u0027])))\n```\n\nalthough since we are just looping over the device we dont actully need to conver this to a list\n\nwe can also inlien the regex as a predicate\n\n\n```suggestion\n        ascends \u003d utils.get_pci_devices(\n            lambda line: \u0027d100\u0027 in line, # 1. Fast string check\n            PCI_INFO_PATTERN.search, # 2. Slower regex check)\n```","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":91,"context_line":"          \u0027revision\u0027: \u002720\u0027                     # Revision number"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":"        \"\"\""},{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"}],"source_content_type":"text/x-python","patch_set":3,"id":"d52a50e9_927661a9","line":94,"in_reply_to":"ae3e188a_0e4dc8d8","updated":"2026-07-29 14:40:37.000000000","message":"`discover()` now calls `get_pci_devices()` with the \u0027d100\u0027 keyword and the regex as predicates.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"},{"line_number":98,"context_line":"            if m:"},{"line_number":99,"context_line":"                pci_dict \u003d m.groupdict()"},{"line_number":100,"context_line":"                pci_dict[\"slot_json\"] \u003d utils.pci_str_to_json(pci_dict[\"slot\"])"},{"line_number":101,"context_line":"                device \u003d driver_device.DriverDevice()"},{"line_number":102,"context_line":"                device.stub \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"b369cc17_8f39f9ae","line":99,"range":{"start_line":97,"start_character":10,"end_line":99,"end_character":40},"updated":"2026-07-28 16:54:21.000000000","message":"if we do the regex match above we still need to do it here to get the groups but we can remove the if since we know it will match and dedet the rest of the loop body\n\n```\n        npu_list \u003d []\n        for ascend in ascends:\n           m \u003d PCI_INFO_PATTERN.search(ascend)\n           pci_dict \u003d m.groupdict()\n           ...\n```","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        ascends \u003d self._get_pci_lines((\u0027d100\u0027,))"},{"line_number":95,"context_line":"        npu_list \u003d []"},{"line_number":96,"context_line":"        for ascend in ascends:"},{"line_number":97,"context_line":"            m \u003d PCI_INFO_PATTERN.match(ascend)"},{"line_number":98,"context_line":"            if m:"},{"line_number":99,"context_line":"                pci_dict \u003d m.groupdict()"},{"line_number":100,"context_line":"                pci_dict[\"slot_json\"] \u003d utils.pci_str_to_json(pci_dict[\"slot\"])"},{"line_number":101,"context_line":"                device \u003d driver_device.DriverDevice()"},{"line_number":102,"context_line":"                device.stub \u003d False"}],"source_content_type":"text/x-python","patch_set":3,"id":"58595270_c691db39","line":99,"range":{"start_line":97,"start_character":10,"end_line":99,"end_character":40},"in_reply_to":"b369cc17_8f39f9ae","updated":"2026-07-29 14:40:37.000000000","message":"Dropped the match check since the regex predicate guarantees a match, and dedented the loop body.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"0091abbcedf1b96f255c19c24f2ee3d3490f54c3","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"48588ecd_6ceaecf7","line":82,"updated":"2026-08-01 08:02:15.000000000","message":"The ascend discover() method sets device.model from ascend.get(\u0027model\u0027, \u0027\u0027), but the unified parsed lspci dict from parse_lspci_line() contains \u0027device_name\u0027, not \u0027model\u0027. This key never exists in the dict, so model is always an empty string. All four other drivers (GPU, SSD, Inspur FPGA, Xilinx...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Ascend AI chip devices will always report model as empty string, while all other accelerator types (GPU, SSD, FPGA) correctly populate the model with the device name. This is a behavioral inconsistency across driver types in the same codebase after unification.\n\n**Recommendation**:\nChange `ascend.get(\u0027model\u0027, \u0027\u0027)` to `ascend.get(\u0027device_name\u0027, \u0027\u0027)` on line 82 of ascend.py, matching the pattern used by all other drivers. This will correctly populate the model field with the parsed device name.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f7ceb8903877113b815821a50954e504ee9f653","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"24be81ab_d0712622","line":82,"in_reply_to":"06d382d5_9466afd6","updated":"2026-08-06 15:39:09.000000000","message":"Acknowledged","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1f81f1d7db45e8a176b53fc7847f65c523c5e168","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"a558fd2a_3475e361","line":82,"in_reply_to":"48588ecd_6ceaecf7","updated":"2026-08-04 16:56:12.000000000","message":"This looks like a valid bug but should be fixed separately with its own bug report.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"98cb07abfb6d0c0f7538422a54595b505c472f60","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"90cb274f_7e0c5793","line":82,"in_reply_to":"48588ecd_6ceaecf7","updated":"2026-08-04 16:35:25.000000000","message":"hum so model was never a part of the orginal regex or the docstring example.\n\n```\nPCI_INFO_PATTERN \u003d re.compile(\n    r\"(?P\u003cslot\u003e[0-9a-f]{4}:[0-9a-f]{2}:\"\n    r\"[0-9a-f]{2}\\.[0-9a-f]) \"\n    r\"(?P\u003cclass\u003e.*) [\\[].*]: (?P\u003cdevice\u003e.*) .*\"\n    r\"[\\[](?P\u003cvendor_id\u003e[0-9a-fA-F]\"\n    r\"{4}):(?P\u003cdevice_id\u003e[0-9a-fA-F]{4})].*\"\n    r\"[(rev ](?P\u003crevision\u003e[0-9a-f]{2})\"\n)\n```\n\nso i think device.model was alwasy the empty stting before.\n\nmy review bot got truncated but what its trying to say is if we look at the other regexes for other drivers\n\n```\nINSPUR_FPGA_INFO_PATTERN \u003d re.compile(\n    r\"(?P\u003cdevices\u003e[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:\"\n    r\"[0-9a-fA-F]{2}\\.[0-9a-fA-F]) \"\n    r\"(?P\u003ccontroller\u003e.*) [\\[].*]: (?P\u003cmodel\u003e.*) .*\"\n    r\"[\\[](?P\u003cvendor_id\u003e[0-9a-fA-F]\"\n    r\"{4}):(?P\u003cproduct_id\u003e[0-9a-fA-F]{4})].*\"\n)\n```\n\n```\n[\\[].*]: (?P\u003cmodel\u003e.*) .*\n```\n\nwoudl appre tto match with \n\n```\n[\\[].*]: (?P\u003cdevice\u003e.*) .*\n```\nwhich in the new commen regex woudl be \n\n  r\"(?P\u003cdevice_name\u003e.*?) \"\n  \n  https://review.opendev.org/c/openstack/cyborg/+/998817/5/cyborg/accelerator/drivers/fpga/inspur/sysinfo.py#77\n \nso yes thsi shoudl be\n  \nascend.get(\u0027device_name\u0027, \u0027\u0027)\n  \nalthough in the past i think it was alwasy `\u0027\u0027` as jmodel was never set.\n  \nthere was never any test coverge for tat model should be\n  \nhttps://github.com/openstack/cyborg/blob/master/cyborg/tests/unit/accelerator/drivers/aichip/huawei/test_ascend.py","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"069112f1a7a8e71c7619d7e1c8abab26a220f35e","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"06d382d5_9466afd6","line":82,"in_reply_to":"6a5f0605_39acdce1","updated":"2026-08-06 14:08:46.000000000","message":"Kept the fix here and filed a bug for it: https://bugs.launchpad.net/openstack-cyborg/+bug/2162962. I called it out in the commit message and added Closes-Bug, so it is documented and closed by this change. Point taken on not folding fixes into refactors as a habit.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"bbd9899fef9033c2d8c6ca75c053257c6b84b5fd","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"e297b9b8_6ebc3b91","line":82,"in_reply_to":"7558c730_ed20e9df","updated":"2026-08-05 13:47:59.000000000","message":"Fixed to `ascend.get(\u0027device_name\u0027, \u0027\u0027)`, the field the other drivers populate. Since it is a one line correctness fix with a test and we are not backporting, I think it is better to fix it here than to open a separate bug for it. \n\nI also added an assertion in test_ascend so model is covered now, since it never was before.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b909b846aa27b96e2d61a62c94e73110efc01558","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"7558c730_ed20e9df","line":82,"in_reply_to":"a558fd2a_3475e361","updated":"2026-08-04 17:09:45.000000000","message":"how do you think we shoudl proceed\n\ni was debating betwen hardcodign it to \u0027\u0027 which this will do indirectly or fixing it now. if we wanted to fix and backport it would be simpler to do that on master first\n\nbut if we jsut want to fix it going forward the order is less improant","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"cac8a3d1_1173a9c1","line":82,"in_reply_to":"ba27705e_871adcf6","updated":"2026-08-05 19:05:54.000000000","message":"yep totally fair.\n\nin this case im ok with floding it in or spliting but your right that that is the correct pattern to follow in general","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"352948f86a5b441046396e1e85f7694c5018342e","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"6a5f0605_39acdce1","line":82,"in_reply_to":"cac8a3d1_1173a9c1","updated":"2026-08-05 21:05:02.000000000","message":"Yeah ... definitely don\u0027t want to keep doing this. That said, the fix isn\u0027t mentioned in the commit message either and I thought it should at least be mentioned there.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4ae9a02a4ff09c8dac934b4bb1a51dccffa02609","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            device \u003d driver_device.DriverDevice()"},{"line_number":80,"context_line":"            device.stub \u003d False"},{"line_number":81,"context_line":"            device.vendor \u003d ascend[\"vendor_id\"]"},{"line_number":82,"context_line":"            device.model \u003d ascend.get(\u0027model\u0027, \u0027\u0027)"},{"line_number":83,"context_line":"            std_board_info \u003d {"},{"line_number":84,"context_line":"                \u0027device_id\u0027: ascend.get(\u0027device_id\u0027, None),"},{"line_number":85,"context_line":"                \u0027class\u0027: ascend.get(\u0027class_name\u0027, None),"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba27705e_871adcf6","line":82,"in_reply_to":"e297b9b8_6ebc3b91","updated":"2026-08-05 17:08:59.000000000","message":"This is OK but personally I don\u0027t want to get into the habit of folding bug fixes into changes that are for refactoring and not about the bug, even if they are small.\n\nMy reasoning is that there are real users out in the world using Cyborg and if there is a functionality bug we should have a bug report that explains the problem and when we fix it, we have a patch targeting the bug and closing it. So that it is clear to an outside observer that there was a bug and it was fixed by a specific commit. And not get muddy into the waters of unrelated refactoring.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/accelerator/drivers/fpga/inspur/sysinfo.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    fpgas \u003d utils.get_pci_devices("},{"line_number":69,"context_line":"        utils.has_flags(INSPUR_FPGA_FLAGS), utils.has_vendor(VENDOR_ID)"},{"line_number":70,"context_line":"    )"},{"line_number":71,"context_line":"    for fpga in fpgas:"},{"line_number":72,"context_line":"        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)"},{"line_number":73,"context_line":"        if m:"},{"line_number":74,"context_line":"            fpga_dict \u003d m.groupdict()"},{"line_number":75,"context_line":"            # generate traits info"},{"line_number":76,"context_line":"            traits \u003d get_traits("},{"line_number":77,"context_line":"                fpga_dict[\"vendor_id\"], fpga_dict[\"product_id\"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"58325111_3c6aac43","line":74,"range":{"start_line":71,"start_character":4,"end_line":74,"end_character":37},"updated":"2026-07-28 16:54:21.000000000","message":"since we need to do that anyway im actuly not sure if includignt  the regex in the predicates acly is beter of not\n```suggestion\n    for fpga in fpgas:\n        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)\n        if m is None:\n          continue\n       \n        fpga_dict \u003d m.groupdict()\n```\nis a better way to dedent the loop body.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":68,"context_line":"    fpgas \u003d utils.get_pci_devices("},{"line_number":69,"context_line":"        utils.has_flags(INSPUR_FPGA_FLAGS), utils.has_vendor(VENDOR_ID)"},{"line_number":70,"context_line":"    )"},{"line_number":71,"context_line":"    for fpga in fpgas:"},{"line_number":72,"context_line":"        m \u003d INSPUR_FPGA_INFO_PATTERN.match(fpga)"},{"line_number":73,"context_line":"        if m:"},{"line_number":74,"context_line":"            fpga_dict \u003d m.groupdict()"},{"line_number":75,"context_line":"            # generate traits info"},{"line_number":76,"context_line":"            traits \u003d get_traits("},{"line_number":77,"context_line":"                fpga_dict[\"vendor_id\"], fpga_dict[\"product_id\"]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1243ed97_3e9ad5f4","line":74,"range":{"start_line":71,"start_character":4,"end_line":74,"end_character":37},"in_reply_to":"58325111_3c6aac43","updated":"2026-07-29 14:40:37.000000000","message":"dedented the loop with an early continue.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"}],"cyborg/accelerator/drivers/fpga/xilinx/sysinfo.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a33c8a4a825760f1f6cc338d250bdd12a36b0b1","unresolved":true,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def _get_pf_type(device):"},{"line_number":65,"context_line":"    cmd \u003d [\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device]"},{"line_number":66,"context_line":"    result \u003d utils.lspci_privileged(cmd)"},{"line_number":67,"context_line":"    for k, v in XILINX_PF_MAPS.items():"},{"line_number":68,"context_line":"        if v in result:"},{"line_number":69,"context_line":"            return k"}],"source_content_type":"text/x-python","patch_set":3,"id":"abe9b946_eaa1f101","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":2},"updated":"2026-07-28 16:54:21.000000000","message":"-1 we do not allow passing  arrays of in generic command args to functions decoreted with privsep that is an explicit any pattern. \n\n\n you can pass fixed arguments like the device address but we want to prevent arbaity command execution by having very narrow contract for the privileged functions.\n\ni would create a dedicated pci_details function in utils that does \n\n` cmd \u003d [\u0027lspci\u0027, \u0027-kvv\u0027, \u0027-s\u0027, device]`\nyou do not need vv currenly so we do not need to add that but we may want that later.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"f66c09de19e744985447562fc77f4b85b9eda48c","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def _get_pf_type(device):"},{"line_number":65,"context_line":"    cmd \u003d [\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device]"},{"line_number":66,"context_line":"    result \u003d utils.lspci_privileged(cmd)"},{"line_number":67,"context_line":"    for k, v in XILINX_PF_MAPS.items():"},{"line_number":68,"context_line":"        if v in result:"},{"line_number":69,"context_line":"            return k"}],"source_content_type":"text/x-python","patch_set":3,"id":"1b4d16dd_2719dbee","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":2},"in_reply_to":"abe9b946_eaa1f101","updated":"2026-07-29 14:40:37.000000000","message":"Added a narrow `pci_details(device)` in common utils that runs a fixed \u0027lspci -k -s \u003cdevice\u003e\u0027. Left out -vv since it is not needed yet. xilinx `_get_pf_type()` now calls it.","commit_id":"833beeb3531bf0ac0f1826057f8a573ac7f699a4"}],"cyborg/tests/unit/accelerator/drivers/aichip/huawei/test_ascend.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1f81f1d7db45e8a176b53fc7847f65c523c5e168","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"775749c0_7a317b2d","line":25,"updated":"2026-08-04 16:56:12.000000000","message":"Unnecessary change.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"352948f86a5b441046396e1e85f7694c5018342e","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"8bb6b28f_8f075cf1","line":25,"in_reply_to":"00290867_6ce8afff","updated":"2026-08-05 21:05:02.000000000","message":"Ah ... right. My bad.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"bbd9899fef9033c2d8c6ca75c053257c6b84b5fd","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"8c1c2685_2d120b8a","line":25,"in_reply_to":"775749c0_7a317b2d","updated":"2026-08-05 13:47:59.000000000","message":"This one is load bearing. The trailing comma made `d100_pci_res` a one element tuple. The new `utils.lspci_privileged` returns a plain string and `get_pci_devices` calls \u0027splitlines\u0027 on it, so the mock has to return a string. \n\nRestoring the comma makes `test_discover` fail with `AttributeError`: \u0027tuple\u0027 object has no attribute \u0027splitlines\u0027. \n\nHappy to write it a clearer way if you prefer.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"069112f1a7a8e71c7619d7e1c8abab26a220f35e","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e84b34d9_e5db320e","line":25,"in_reply_to":"8bb6b28f_8f075cf1","updated":"2026-08-06 14:08:46.000000000","message":"Switched d100_pci_res to a raw multiline string, much clearer.","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"00290867_6ce8afff","line":25,"in_reply_to":"8c1c2685_2d120b8a","updated":"2026-08-05 19:05:54.000000000","message":"oh yes that is very true\n\nthe comma did make it a tuple rather then just a string\n\na slightly clearer fix would be to use a raw multiline stirng here\n\n```\nd100_pci_res \u003d \"\"\"\\\n0000:00:0c.0 Processing accelerators [1200]: Device [19e5:d100] (rev 20)\n0000:00:0d.0 Processing accelerators [1200]: Device [19e5:d100] (rev 20)\n\"\"\"\n```\nwoudl be more clear\n\ni think this is the only thing that we shoudl really focus on impoving in this change.\n\nthe comment are mainly me realsiging how much technilal debt there is in the way the test are written","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f7ceb8903877113b815821a50954e504ee9f653","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    \u00270000:00:0c.0 Processing accelerators [1200]:\u0027"},{"line_number":23,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":24,"context_line":"    \u00270000:00:0d.0 Processing accelerators [1200]:\u0027"},{"line_number":25,"context_line":"    \u0027 Device [19e5:d100] (rev 20)\\n\u0027"},{"line_number":26,"context_line":")"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"291d8884_2222c2d3","line":25,"in_reply_to":"e84b34d9_e5db320e","updated":"2026-08-06 15:39:09.000000000","message":"Done","commit_id":"c9a71f53325a941dd5062bdac27fc3d0c93444ca"}],"cyborg/tests/unit/accelerator/drivers/fpga/xilinx/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":true,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from cyborg.accelerator.drivers.fpga.xilinx.driver import XilinxFPGADriver"},{"line_number":20,"context_line":"from cyborg.tests import base"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"XILINX_FPGA_INFO \u003d ["},{"line_number":24,"context_line":"    \"0000:3b:00.0 Processing accelerators [1200]: \""},{"line_number":25,"context_line":"    \"Xilinx Corporation Device [10ee:5000]\\n\""},{"line_number":26,"context_line":"    \"0000:3b:00.1 Processing accelerators [1200]: \""},{"line_number":27,"context_line":"    \"Xilinx Corporation Device [10ee:5001]\""},{"line_number":28,"context_line":"]"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def fake_output(arg\u003d(\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027)):"},{"line_number":32,"context_line":"    if list(arg) \u003d\u003d [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027]:"},{"line_number":33,"context_line":"        return XILINX_FPGA_INFO[0]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class TestXilinxFPGADriver(base.TestCase):"},{"line_number":37,"context_line":"    def setUp(self):"},{"line_number":38,"context_line":"        super().setUp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @mock.patch(\u0027cyborg.accelerator.common.utils.lspci_privileged\u0027)"},{"line_number":41,"context_line":"    def test_discover(self, mock_devices_for_vendor):"},{"line_number":42,"context_line":"        mock_devices_for_vendor.side_effect \u003d fake_output"},{"line_number":43,"context_line":"        self.set_defaults(host\u003d\u0027fake-host\u0027, debug\u003dTrue)"},{"line_number":44,"context_line":"        fpga_list \u003d XilinxFPGADriver().discover()"},{"line_number":45,"context_line":"        self.assertEqual(1, len(fpga_list))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1c457563_56861541","line":42,"range":{"start_line":21,"start_character":1,"end_line":42,"end_character":57},"updated":"2026-08-05 19:05:54.000000000","message":"i feel like this can be simplified\n\nbefore lspci_privileged took a command to run and we were mocking it directly with a conditional since it could be called via get_pci_devices\nor via def _get_pf_type(device):\n\n\nso the command could change betwen `\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027` and `\u0027lspci\u0027, \u0027-k\u0027, \u0027-s\u0027, device`\n\nthe first was handed the second was not\n\n\n\n\n```suggestion\n\n\nXILINX_FPGA_INFO \u003d \"\"\"\\\n0000:3b:00.0 Processing accelerators [1200]: \\\nXilinx Corporation Device [10ee:5000]\n0000:3b:00.1 Processing accelerators [1200]: \\\nXilinx Corporation Device [10ee:5001]\"\"\"\n\n\nclass TestXilinxFPGADriver(base.TestCase):\n    def setUp(self):\n        super().setUp()\n        @mock.patch(\u0027cyborg.accelerator.common.utils.lspci_privileged\u0027, return_value\u003dXILINX_FPGA_INFO)\n    def test_discover(self, mock_devices_for_vendor):\n```\n\nmaybe somethign like this","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":39344,"name":"Gihong Lee","display_name":"gamio","email":"gh9231@gmail.com","username":"gamio"},"change_message_id":"069112f1a7a8e71c7619d7e1c8abab26a220f35e","unresolved":true,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from cyborg.accelerator.drivers.fpga.xilinx.driver import XilinxFPGADriver"},{"line_number":20,"context_line":"from cyborg.tests import base"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"XILINX_FPGA_INFO \u003d ["},{"line_number":24,"context_line":"    \"0000:3b:00.0 Processing accelerators [1200]: \""},{"line_number":25,"context_line":"    \"Xilinx Corporation Device [10ee:5000]\\n\""},{"line_number":26,"context_line":"    \"0000:3b:00.1 Processing accelerators [1200]: \""},{"line_number":27,"context_line":"    \"Xilinx Corporation Device [10ee:5001]\""},{"line_number":28,"context_line":"]"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def fake_output(arg\u003d(\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027)):"},{"line_number":32,"context_line":"    if list(arg) \u003d\u003d [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027]:"},{"line_number":33,"context_line":"        return XILINX_FPGA_INFO[0]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class TestXilinxFPGADriver(base.TestCase):"},{"line_number":37,"context_line":"    def setUp(self):"},{"line_number":38,"context_line":"        super().setUp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @mock.patch(\u0027cyborg.accelerator.common.utils.lspci_privileged\u0027)"},{"line_number":41,"context_line":"    def test_discover(self, mock_devices_for_vendor):"},{"line_number":42,"context_line":"        mock_devices_for_vendor.side_effect \u003d fake_output"},{"line_number":43,"context_line":"        self.set_defaults(host\u003d\u0027fake-host\u0027, debug\u003dTrue)"},{"line_number":44,"context_line":"        fpga_list \u003d XilinxFPGADriver().discover()"},{"line_number":45,"context_line":"        self.assertEqual(1, len(fpga_list))"}],"source_content_type":"text/x-python","patch_set":6,"id":"7038c55e_5bfe1d00","line":42,"range":{"start_line":21,"start_character":1,"end_line":42,"end_character":57},"in_reply_to":"1c457563_56861541","updated":"2026-08-06 14:08:46.000000000","message":"Agreed, this is worth simplifying now that the privileged helpers take fixed commands. I will do it as a separate test cleanup followup.","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f7ceb8903877113b815821a50954e504ee9f653","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from cyborg.accelerator.drivers.fpga.xilinx.driver import XilinxFPGADriver"},{"line_number":20,"context_line":"from cyborg.tests import base"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"XILINX_FPGA_INFO \u003d ["},{"line_number":24,"context_line":"    \"0000:3b:00.0 Processing accelerators [1200]: \""},{"line_number":25,"context_line":"    \"Xilinx Corporation Device [10ee:5000]\\n\""},{"line_number":26,"context_line":"    \"0000:3b:00.1 Processing accelerators [1200]: \""},{"line_number":27,"context_line":"    \"Xilinx Corporation Device [10ee:5001]\""},{"line_number":28,"context_line":"]"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def fake_output(arg\u003d(\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027)):"},{"line_number":32,"context_line":"    if list(arg) \u003d\u003d [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027]:"},{"line_number":33,"context_line":"        return XILINX_FPGA_INFO[0]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class TestXilinxFPGADriver(base.TestCase):"},{"line_number":37,"context_line":"    def setUp(self):"},{"line_number":38,"context_line":"        super().setUp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @mock.patch(\u0027cyborg.accelerator.common.utils.lspci_privileged\u0027)"},{"line_number":41,"context_line":"    def test_discover(self, mock_devices_for_vendor):"},{"line_number":42,"context_line":"        mock_devices_for_vendor.side_effect \u003d fake_output"},{"line_number":43,"context_line":"        self.set_defaults(host\u003d\u0027fake-host\u0027, debug\u003dTrue)"},{"line_number":44,"context_line":"        fpga_list \u003d XilinxFPGADriver().discover()"},{"line_number":45,"context_line":"        self.assertEqual(1, len(fpga_list))"}],"source_content_type":"text/x-python","patch_set":6,"id":"cd221799_9ee5e94d","line":42,"range":{"start_line":21,"start_character":1,"end_line":42,"end_character":57},"in_reply_to":"7038c55e_5bfe1d00","updated":"2026-08-06 15:39:09.000000000","message":"Acknowledged","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"736862b9e41a9aacdc22640f9a5cb8fa8a1bb5b9","unresolved":false,"context_lines":[{"line_number":28,"context_line":"]"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def fake_output(arg\u003d(\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027)):"},{"line_number":32,"context_line":"    if list(arg) \u003d\u003d [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027]:"},{"line_number":33,"context_line":"        return XILINX_FPGA_INFO[0]"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"213ae490_6ea922eb","line":31,"updated":"2026-08-06 15:41:34.000000000","message":"The fake_output helper in the xilinx test still references the old [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027] command format via a default argument, but lspci_privileged() is now called with no arguments. The conditional is always True and the side_effect indirection serves no purpose.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Future maintainers will be confused by the stale command-format check and the side_effect indirection, potentially believing the command arguments still matter. This adds unnecessary cognitive load when maintaining or debugging the xilinx discovery test.\n\n**Recommendation**:\nReplace the side_effect \u003d fake_output pattern with a direct return value: mock_devices_for_vendor.return_value \u003d XILINX_FPGA_INFO[0]. Remove the fake_output function entirely since it no longer serves a purpose now that lspci_privileged takes no arguments.","commit_id":"9ee937cb7731a0711ae68d4e832aba064c8571bc"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd28ae58d7f9b73a1dac0de0343949febc1af2f7","unresolved":true,"context_lines":[{"line_number":28,"context_line":"]"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def fake_output(arg\u003d(\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027)):"},{"line_number":32,"context_line":"    if list(arg) \u003d\u003d [\u0027lspci\u0027, \u0027-nnn\u0027, \u0027-D\u0027]:"},{"line_number":33,"context_line":"        return XILINX_FPGA_INFO[0]"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"0c09da91_7608ece2","line":31,"in_reply_to":"213ae490_6ea922eb","updated":"2026-08-06 15:46:42.000000000","message":"hum this is valid alhtougb because tis default it passes, we coudl defer this to the followup test cleanup patch since this should go away then or fix it now.","commit_id":"9ee937cb7731a0711ae68d4e832aba064c8571bc"}],"cyborg/tests/unit/accelerator/drivers/gpu/test_utils.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":true,"context_lines":[{"line_number":66,"context_line":"    \u0027nvidia-319\u0027,"},{"line_number":67,"context_line":"    \u0027nvidia-320\u0027,"},{"line_number":68,"context_line":"    \u0027nvidia-321\u0027,"},{"line_number":69,"context_line":"]"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"BUILTIN \u003d \u0027__builtin__\u0027 if (sys.version_info[0] \u003c 3) else \u0027__builtins__\u0027"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"class stdout:"}],"source_content_type":"text/x-python","patch_set":6,"id":"83e4c659_36fe2e8b","line":71,"range":{"start_line":69,"start_character":1,"end_line":71,"end_character":72},"updated":"2026-08-05 19:05:54.000000000","message":"nit: \n\nthis is technialy unrelated but we do not supprot python 2 anymore so this is irrelevent but i also think its never used in this file?\n\nso we can proably just delete this.","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e17c022b0e9e60c149fa96c19f28920ad579ab3b","unresolved":true,"context_lines":[{"line_number":94,"context_line":"    def setUp(self):"},{"line_number":95,"context_line":"        super().setUp()"},{"line_number":96,"context_line":"        self.p \u003d p()"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    @mock.patch(\u0027cyborg.accelerator.common.utils.lspci_privileged\u0027)"},{"line_number":99,"context_line":"    def test_discover_vendors(self, mock_devices):"},{"line_number":100,"context_line":"        mock_devices.return_value \u003d self.p.stdout.readlines()[0]"},{"line_number":101,"context_line":"        gpu_vendors \u003d utils.discover_vendors()"},{"line_number":102,"context_line":"        self.assertEqual(1, len(gpu_vendors))"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"22bcfdeb_3cc4ff72","line":100,"range":{"start_line":97,"start_character":1,"end_line":100,"end_character":64},"updated":"2026-08-05 19:05:54.000000000","message":"nit: this can also be simplifed now\n\ntechnically lspci_privileged is nolonger called  get_pci_devices is so it woudl be better to mock this by mockign the returned dict rather then the lower level emulation fo the stdout returned by the process exec call.\n\nbut lspci_privileged never needed to emulate the interla of the subproces call\nit could have just been\n\n```\n    @mock.patch(\n        \u0027cyborg.accelerator.common.utils.lspci_privileged\u0027\n        return_value\u003dNVIDIA_GPU_INFO)\n    def test_discover_vendors(self, mock_devices):\n```\n\nwe can then remove the the stdout and p classes and self.p entirly\nyou just need to fix the other mocks to use the constnats directly.\n\ni guess we should do that cleanup in a followup since it\nreally a sperate prexisting issue not one strictly related to this patch.\nif we keep that as a sperate commit it also show we are not acidentlly regressing the current test coverage\n\nultimately we shoudl avoid very deep mocking in unit tests\n\nwe plan to buiild out proper functional test in the future which would have diffent mockign behvior but unit test shoudl not really look through multiple layers of funciton and emulate the behavior fo subprocess like this","commit_id":"6f375f19ef438b103c15e5ab2a60d92a44ad5ad5"}]}