)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30615,"name":"Tushar Trambak Gite","email":"tushargite96@gmail.com","username":"tushargite96"},"change_message_id":"fefb2add8fee1b03332620654b2c5255a5e16e77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d1731127_2b5c3ff4","updated":"2022-01-20 06:54:22.000000000","message":"LGTM thanks for the patch","commit_id":"e456c8b87d823db5986f12620086ad19668ee0e2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9636e5afb76fbc4426b1834531fd2b49421b90ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ebc60da3_aa1362b7","updated":"2022-01-31 17:56:13.000000000","message":"Question inline.","commit_id":"e456c8b87d823db5986f12620086ad19668ee0e2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c4652c7dc9ade9763cde773fc9b95340418a9ddc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"48641acd_b86406c3","updated":"2022-02-18 07:05:42.000000000","message":"Thanks Brian for the review, please find my comment inline.","commit_id":"77c004c2dd25cda5933ba03814093b28826bc042"}],"os_brick/initiator/connectors/base.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9636e5afb76fbc4426b1834531fd2b49421b90ad","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        if not driver:"},{"line_number":39,"context_line":"            driver \u003d host_driver.HostDriver()"},{"line_number":40,"context_line":"        self.set_driver(driver)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        super(BaseLinuxConnector, self).__init__(root_helper, execute\u003dexecute,"},{"line_number":43,"context_line":"                                                 *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"472781a0_121b491d","side":"PARENT","line":40,"range":{"start_line":40,"start_character":8,"end_line":40,"end_character":31},"updated":"2022-01-31 17:56:13.000000000","message":"It looks like all the subclasses of BaseLinuxConnector take a \u0027driver\u0027 argument to their __init__() and then pass it on to super().__init__() ... so I think you need to keep this, something like:\n\n  if driver:\n      self.set_driver(driver)\n\nThat being said, the InitiatorConnector metaclass __init__() also takes a driver argument but doesn\u0027t do anything with it.  I wonder whether the driver parameter should be removed from InitiatorConnector.__init__() ... it looks like this was the only subclass that actually did anything with it.  Similarly, instead of calling set_driver() here, we could just remove the driver parameter completely, because I don\u0027t think any of the subclasses of BaseLinuxConnector actually use it, either.\n\nI guess what I\u0027m saying is that your patch either goes too far (by completely ignoring the driver parameter) or not far enough (by not removing the driver parameter completely).","commit_id":"7a6a09fc84a779c3ee08d122664f941195eeab8f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c4652c7dc9ade9763cde773fc9b95340418a9ddc","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        if not driver:"},{"line_number":39,"context_line":"            driver \u003d host_driver.HostDriver()"},{"line_number":40,"context_line":"        self.set_driver(driver)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        super(BaseLinuxConnector, self).__init__(root_helper, execute\u003dexecute,"},{"line_number":43,"context_line":"                                                 *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d0c2139c_c3afcef3","side":"PARENT","line":40,"range":{"start_line":40,"start_character":8,"end_line":40,"end_character":31},"in_reply_to":"472781a0_121b491d","updated":"2022-02-18 07:05:42.000000000","message":"The reason for not removing the driver parameter is that it is part of the method signature of connectors and is at least called by nova[1] (and all other driver connector implementations[2]), glance[3], ironic(maybe) and other projects? Removing it seems unsafe if some part of code (legacy) is still passing the driver parameter (even when we aren\u0027t doing anything with it) and it will break those projects.\nThe main aim of this patch is to remove the HostDriver so I do not intent to do any major changes to the brick interface (exposed to other projects) in this patch.\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/volume/fibrechannel.py#L35\n[2] https://github.com/openstack/nova/tree/master/nova/virt/libvirt/volume\n[3] https://github.com/openstack/glance_store/blob/master/glance_store/_drivers/cinder.py#L755","commit_id":"7a6a09fc84a779c3ee08d122664f941195eeab8f"}],"os_brick/initiator/initiator_connector.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9636e5afb76fbc4426b1834531fd2b49421b90ad","unresolved":true,"context_lines":[{"line_number":35,"context_line":"                                                 *args, **kwargs)"},{"line_number":36,"context_line":"        self.device_scan_attempts \u003d device_scan_attempts"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def set_driver(self, driver):"},{"line_number":39,"context_line":"        \"\"\"The driver is used to find used LUNs.\"\"\""},{"line_number":40,"context_line":"        self.driver \u003d driver"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    @staticmethod"},{"line_number":43,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"a43ba3a6_20ce8ecb","line":40,"range":{"start_line":38,"start_character":0,"end_line":40,"end_character":28},"updated":"2022-01-31 17:56:13.000000000","message":"I think you are removing the only code that calls this function.","commit_id":"e456c8b87d823db5986f12620086ad19668ee0e2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c4652c7dc9ade9763cde773fc9b95340418a9ddc","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                                                 *args, **kwargs)"},{"line_number":36,"context_line":"        self.device_scan_attempts \u003d device_scan_attempts"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def set_driver(self, driver):"},{"line_number":39,"context_line":"        \"\"\"The driver is used to find used LUNs.\"\"\""},{"line_number":40,"context_line":"        self.driver \u003d driver"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    @staticmethod"},{"line_number":43,"context_line":"    @abc.abstractmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"fad4059a_788d3416","line":40,"range":{"start_line":38,"start_character":0,"end_line":40,"end_character":28},"in_reply_to":"a43ba3a6_20ce8ecb","updated":"2022-02-18 07:05:42.000000000","message":"Done","commit_id":"e456c8b87d823db5986f12620086ad19668ee0e2"}]}
