)]}'
{"/COMMIT_MSG":[{"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":"81ad08ba62bc2470cea25ca369f2cad780fc39cb","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     8bb87356 (Add online data migration and upgrade check for device_state backfill)"},{"line_number":2,"context_line":"Author:     Chandan Kumar (raukadah) \u003cchkumar@redhat.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-08-04 15:18:44 +0530"},{"line_number":4,"context_line":"Commit:     chandan kumar \u003cchkumar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"00661dbe_84a39182","line":1,"updated":"2026-08-24 08:38:48.000000000","message":"The commit message states \u0027Add unit tests that verify default no-op behavior on each base class\u0027, but new tests were added only for GenericDriver, FPGADriver, GPUDriver, NICDriver, QATDriver, and SSDDriver. The two remaining base classes that received stubs, PciDriver (cyborg/accelerator/drivers/pci/base.py:27-32) and SPDKDRIVER (cyborg/accelerator/drivers/spdk/spdk.py:41-46), have no tests touching init_host() or cleanup(). The existing test trees only cover vendor drivers: cyborg/tests/unit/accelerator/drivers/pci/pci/test_sysinfo.py and cyborg/tests/unit/accelerator/drivers/spdk/{nvmf,vhost}/, none of which exercise the new hooks.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: The permanent change history overstates verification coverage for the new driver-interface hooks. A future regression in the PciDriver or SPDKDRIVER stubs (for example, a subclass override raising NotImplementedError, which is the existing convention for discover() in those same classes) would not be caught by the suite, and reviewers relying on the message would assume it is.\n\n**Suggestion**:\nAdd the two missing no-op tests, e.g. test_init_host_default_noop/test_cleanup_default_noop for PciDriver in cyborg/tests/unit/accelerator/drivers/pci/ and for SPDKDRIVER in cyborg/tests/unit/accelerator/drivers/spdk/, mirroring the fpga/gpu/nic/qat/ssd tests. Alternatively, reword the commit message to list exactly which base classes the tests cover.","commit_id":"e75e9ed20f7fb65d56b058bee2675e3507a47b80"},{"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":"3e89020b8b28d320ffd0ee1a53d55622ef19ab49","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     eec56f97 (Add online data migration and upgrade check for device_state backfill)"},{"line_number":2,"context_line":"Author:     Chandan Kumar (raukadah) \u003cchkumar@redhat.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2026-08-04 15:18:44 +0530"},{"line_number":4,"context_line":"Commit:     chandan kumar \u003cchkumar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"50f439a0_d217829b","line":1,"updated":"2026-08-31 10:20:51.000000000","message":"The commit message states \u0027Add unit tests that verify default no-op behavior on each base class\u0027, but tests were added only for GenericDriver, FPGADriver, GPUDriver, NICDriver, QATDriver and SSDDriver. No test exercises the new init_host()/cleanup() stubs on PciDriver or SPDKDRIVER.\n\n**Severity**: WARNING | **Confidence**: 0.95\n\n**Impact**: The permanent change history overstates the verification performed, and any later regression confined to the PciDriver or SPDKDRIVER stubs (e.g. a signature change or accidental exception introduced while overriding) has no failing test to catch it.\n\n**Suggestion**:\nEither add the same test_init_host_default_noop/test_cleanup_default_noop pair for PciDriver (new cyborg/tests/unit/accelerator/drivers/pci/test_base.py) and SPDKDRIVER (new cyborg/tests/unit/accelerator/drivers/spdk/test_spdk.py), or reword the commit message to name the base classes actually covered.","commit_id":"d08b782219d0e975a9405124f808022ad6a00e9e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"9ec96820a41d80d7a397918b91a91beaf00777c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"cf87ba56_a3ab223f","updated":"2026-08-20 06:26:30.000000000","message":"Updated the review by addressing the teim-ci suggestions.","commit_id":"b5cbe96cd85bd29a94ba40077a95937a0d883f35"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"811e14628c4e1e5f97e85775ebba4cb8bb167add","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"d1259b7b_263127af","updated":"2026-08-21 17:28:16.000000000","message":"we need to add init_host to all the other dirvers here\nhttps://review.opendev.org/c/openstack/cyborg/+/999938/comment/6da18d0c_7fcfc754/","commit_id":"7753ad316ed491dd6365f13ffd3b59466245e138"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"80873f172cfbcdeb086e47a8e67938a596538f22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"6e15a018_452a9075","in_reply_to":"3e652d93_744d4200","updated":"2026-08-24 06:26:43.000000000","message":"Done Added init_host and cleanup stub to all drivers.","commit_id":"7753ad316ed491dd6365f13ffd3b59466245e138"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"96559f65289248d1fa93682070c03fd6dd608fea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"3e652d93_744d4200","in_reply_to":"d1259b7b_263127af","updated":"2026-08-21 17:29:17.000000000","message":"and cleanup for that matter other wise we need to call them condtionally but tis better to do ti proprly in thei patch","commit_id":"7753ad316ed491dd6365f13ffd3b59466245e138"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"bf649d3b48cd32eda1f5cb016a42e9a77261cc4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"4ab1d9d1_d4288128","updated":"2026-08-24 15:14:15.000000000","message":"need to resolve commnts from here to above","commit_id":"cfb57620bdc1ab3979f31c71c4decc327d2b91d6"}],"cyborg/accelerator/drivers/driver.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":"6aedeea2da2f9ad17490675c02f485a171b89f53","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def cleanup(self, device):"},{"line_number":26,"context_line":"        \"\"\"Clean up a device after deallocation."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"        Called by the agent when a device needs sanitization."},{"line_number":29,"context_line":"        Default is no-op. Subclasses may override to run device-specific"},{"line_number":30,"context_line":"        cleanup (e.g. nvme-cli sanitize)."},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"58e4b6ea_29b94dba","line":28,"updated":"2026-08-12 08:14:38.000000000","message":"The commit message states \u0027NVMeDriver overrides both\u0027 and the cleanup docstring references \u0027nvme-cli sanitize\u0027, but no NVMeDriver class exists anywhere in the cyborg codebase. This makes the commit message misleading about what the change accomplishes.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Future maintainers reading the commit message or docstring may search for NVMeDriver and waste time, or assume cleanup functionality exists when it does not. The docstring example implies nvme-cli sanitize is an existing override pattern when none exists.\n\n**Recommendation**:\nEither adjust the commit message to clarify NVMeDriver is planned but not yet implemented, or reword the cleanup docstring example to use a generic example rather than naming a specific non-existent driver subclass.","commit_id":"ac2b447968d72960df12113251c44d5be87bfcf4"},{"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":"d3a70202c6f9e5f944bb78419c22274b6355884d","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"class GenericDriver(metaclass\u003dabc.ABCMeta):"},{"line_number":18,"context_line":"    def init_host(self):"},{"line_number":19,"context_line":"        \"\"\"Validate driver prerequisites at agent startup."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        Called once per enabled driver before the first discover()."}],"source_content_type":"text/x-python","patch_set":11,"id":"5df3c943_1b084708","line":18,"updated":"2026-08-14 14:44:01.000000000","message":"Docstrings for the new hooks state they are \u0027Called once per enabled driver before the first discover()\u0027 and \u0027Called by the agent when a device needs sanitization\u0027, but no agent code calls these driver-level hooks.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Contributors implementing drivers will assume the agent invokes these hooks and rely on sanitization that never runs; misleading for the upcoming NVME cleanup work this series targets.\n\n**Suggestion**:\nEither land the agent call sites in this series before these stubs, or reword the docstrings to say the hooks are defined but not yet invoked by the agent.","commit_id":"ae378d3ba7545d5373a748d28d3498dc73a10c82"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"101b98ce7306c41e68046de9dbc69ece4580cf1f","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"class GenericDriver(metaclass\u003dabc.ABCMeta):"},{"line_number":18,"context_line":"    def init_host(self):"},{"line_number":19,"context_line":"        \"\"\"Validate driver prerequisites at agent startup."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        Called once per enabled driver before the first discover()."}],"source_content_type":"text/x-python","patch_set":13,"id":"9ccd5f59_185412ab","line":18,"updated":"2026-08-18 11:44:46.000000000","message":"these are not marked as abstract so while you can override them there is not protection to enfoce that you provide them\n\nim ok with having these be concreate functions for nwo but i really do need to get around to fixign the type inheritance of all the drivers","commit_id":"b5cbe96cd85bd29a94ba40077a95937a0d883f35"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"b91d80913ec552b6dc6f1577f1becaf72106fbb7","unresolved":true,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"class GenericDriver(metaclass\u003dabc.ABCMeta):"},{"line_number":18,"context_line":"    def init_host(self):"},{"line_number":19,"context_line":"        \"\"\"Validate driver prerequisites at agent startup."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        Called once per enabled driver before the first discover()."}],"source_content_type":"text/x-python","patch_set":13,"id":"4161b285_0fa0818c","line":18,"in_reply_to":"9ccd5f59_185412ab","updated":"2026-08-20 06:27:17.000000000","message":"Acknowledged!\nI totally agree here on fixing the type inheritance for all drivers.","commit_id":"b5cbe96cd85bd29a94ba40077a95937a0d883f35"}],"cyborg/accelerator/drivers/pci/base.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":"7bc8889666ebfc368a574e562c8b5655ac814939","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def init_host(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def cleanup(self, *args, **kwargs):"},{"line_number":31,"context_line":"        pass"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def discover(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"e70414ae_44ba431c","line":30,"updated":"2026-08-13 04:27:17.000000000","message":"GenericDriver.cleanup(self, device) declares an explicit positional parameter, while PciDriver.cleanup(self, *args, **kwargs) uses catch-all args. This creates an inconsistent calling contract for the same lifecycle method across the two driver hierarchies. Callers iterating over heterogeneous dr...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If agent code or conductor code calls driver.cleanup(device) on drivers from both hierarchies interchangeably, PciDriver subclasses that rely on *args/**kwargs without unpacking could silently ignore the device argument, or conversely GenericDriver subclasses expecting a positional device argumen...\n\n**Suggestion**:\nAlign PciDriver.cleanup to accept the same explicit device parameter as GenericDriver.cleanup: def cleanup(self, device): pass. This ensures both base classes present the same interface to callers. If PciDriver needs to stay backward-compatible, at least add a docstring noting the expected argument.","commit_id":"f451a8aec696eb9f7cca9d66045d806fd93eb6fc"},{"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":"d3a70202c6f9e5f944bb78419c22274b6355884d","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    def init_host(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def cleanup(self, *args, **kwargs):"},{"line_number":31,"context_line":"        pass"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def discover(self):"}],"source_content_type":"text/x-python","patch_set":11,"id":"550b2ce1_846b55c1","line":30,"updated":"2026-08-14 14:44:01.000000000","message":"GenericDriver.cleanup takes an explicit device parameter while the mirrored PciDriver.cleanup uses *args, **kwargs, so the same lifecycle hook has two different contracts.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A generic caller iterating heterogeneous drivers cannot rely on one calling convention; PCI subclasses implementing cleanup(self, device) will mask positional differences or accept arbitrary calls, allowing interface drift to go undetected.\n\n**Suggestion**:\nUse the same signature (device) in both base classes, matching the documented :param device: contract.","commit_id":"ae378d3ba7545d5373a748d28d3498dc73a10c82"},{"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":"f60831e042681aff9adefcb81d197be896178678","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":25,"context_line":"        pass"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def init_host(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def cleanup(self, device):"}],"source_content_type":"text/x-python","patch_set":17,"id":"995ba82b_29485acd","line":27,"updated":"2026-08-24 08:02:28.000000000","message":"The patch adds init_host()/cleanup() stubs to eight driver base classes but adds default no-op tests for only six (GenericDriver, FPGADriver, GPUDriver, NICDriver, QATDriver, SSDDriver). The stubs on PciDriver (cyborg/accelerator/drivers/pci/base.py:27-32) and SPDKDRIVER (cyborg/accelerator/drivers/spdk/spdk.py:41-46) are untested, so the stated goal of letting a follow-up patch call these hooks on every driver without hasattr checks is unverified for those two classes.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Impact**: A future edit that makes PciDriver.init_host/cleanup or SPDKDRIVER.init_host/cleanup raise, or an accidental signature change, would not be caught by any test, weakening the uniform-interface guarantee the follow-up agent patch depends on. Impact is limited to test coverage of no-op declarations; no runtime behavior is broken.\n\n**Recommendation**:\nAdd the same two no-op tests for PciDriver and SPDKDRIVER, e.g. test_init_host_default_noop/test_cleanup_default_noop using the existing test homes (cyborg/tests/unit/accelerator/drivers/pci/ and cyborg/tests/unit/accelerator/drivers/spdk/test_spdk.py or test_nvmf.py), mirroring the pattern already added for the other bases: instantiate the driver, call init_host() and cleanup(mock.Mock()), and rely on absence of exception.","commit_id":"716dd11527251f3e2d431677e6798a6ea02e1a42"},{"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":"ad2d8072a59805c96d3ef13f2a5183a7619489e3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":25,"context_line":"        pass"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def init_host(self):"},{"line_number":28,"context_line":"        pass"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def cleanup(self, device):"}],"source_content_type":"text/x-python","patch_set":19,"id":"2d2875a0_2daf2c66","line":27,"updated":"2026-08-24 10:18:05.000000000","message":"The commit message states \u0027Add unit tests that verify default no-op behavior on each base class\u0027, and the class list includes PciDriver and SPDKDRIVER. The added tests cover only GenericDriver, FPGADriver, GPUDriver, NICDriver, QATDriver, and SSDDriver. No test exercises PciDriver.init_host/cleanup (pci/base.py:27-32) or SPDKDRIVER.init_host/cleanup (spdk/spdk.py:41-46); the existing pci test module only covers pci utils and the spdk tests cover nvmf/vhost.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The stated verification for PciDriver and SPDKDRIVER default behavior is absent, so the commit history overstates test coverage. A future edit that breaks these stubs (e.g. raising in init_host) would not be caught for these two drivers.\n\n**Suggestion**:\nAdd matching test_init_host_default_noop/test_cleanup_default_noop cases for PciDriver and SPDKDRIVER (in test_driver.py or new pci/spdk test modules), or narrow the commit-message claim. Prefer adding the tests since the pattern is already established in this patch.","commit_id":"cfb57620bdc1ab3979f31c71c4decc327d2b91d6"}]}
