)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7bd89d23f20da43edd937a0d946f79ac4ba8d3fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62353936_37a1b0cd","updated":"2022-08-24 14:39:41.000000000","message":"LGTM","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"}],"os_brick/initiator/linuxrbd.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7bd89d23f20da43edd937a0d946f79ac4ba8d3fb","unresolved":true,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ed03be18_3363a955","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"updated":"2022-08-24 14:39:41.000000000","message":"from the description of IOBase.close[1], i think it already has this check as the description says \"This method has no effect if the file is already closed.\" but I don\u0027t mind extra checks.\n\n[1] https://docs.python.org/3/library/io.html#io.IOBase.close","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d34acdcc5bc684e2d57aedcdcaef7988aad170ab","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"db92d1a5_08528bfb","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"in_reply_to":"5c7ebd57_72bce5b7","updated":"2022-08-24 19:00:37.000000000","message":"What i was trying to convey is that this condition (if not self.closed) also exists in the IOBase implementation of close method[1] so it makes it redundant here IIUC.\nJust calling super().close() should suffice right?\n\nNot sure about the garbage collection finalizer.\n\n[1] https://github.com/python/cpython/blob/main/Lib/_pyio.py#L401","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"6f84298080696b67fae3379381bd915c0ebb16b0","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"0735b8c4_8d12f642","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"in_reply_to":"6a0f7823_7e4fab4c","updated":"2022-08-26 06:28:26.000000000","message":"Sorry to revive this again, but I still have a doubt regarding doing it like,\n\nsuper().close()\nself.rbd_image.close()\n\nwithout the if not self.closed check.\nIf IOBase is closed then the first call won\u0027t do anything and rbd_image is always closed after the IOBase is closed.","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1797f9828067e9c76c1ad73a5a4190e0300048f0","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"6a0f7823_7e4fab4c","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"in_reply_to":"db8b1cc7_5c19e496","updated":"2022-08-25 07:17:50.000000000","message":"Ack, thanks for the the detailed explanation!","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"321c9587a48c8bc23636f28eddcde2b3e1375cce","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"db8b1cc7_5c19e496","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"in_reply_to":"db92d1a5_08528bfb","updated":"2022-08-24 19:46:17.000000000","message":"Sorry, I don\u0027t follow.  As far as I can see there is no redundancy that can be avoided.\n\nThe fact that the base method also checks \"if closed\" doesn\u0027t help us avoid calling \"self.rbd_image.close()\" multiple times.\n\nWe cannot write:\n\n  super().close()\n  if not self.closed:\n      self.rbd_image.close()\n\nBecause in that code the if will always evaluate to False (parent class close sets closed to True).\n\nAnd changing the order is no good either:\n\n  if not self.closed:\n      self.rbd_image.close()\n  super().close()\n\nBecause the parent\u0027s class close will call flush, which shouldn\u0027t be called on a closed file.  Which reminds me that we should also call parent\u0027s method on flush to fail if file is already closed.  Will update this patch.\n\nAnd the parent\u0027s implementation on its own is not sufficient because it doesn\u0027t call self.rbd_image.close.","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61d22d22705c81c11b874e4d8743935c4a92f618","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        raise IOError(_(\"fileno() not supported by RBD()\"))"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def close(self):"},{"line_number":243,"context_line":"        if not self.closed:"},{"line_number":244,"context_line":"            # Can\u0027t set closed attribute ourselves, call parent to flush and"},{"line_number":245,"context_line":"            # change it."},{"line_number":246,"context_line":"            super().close()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5c7ebd57_72bce5b7","line":243,"range":{"start_line":243,"start_character":8,"end_line":243,"end_character":27},"in_reply_to":"ed03be18_3363a955","updated":"2022-08-24 18:31:21.000000000","message":"What that doc is saying is that the base implementation\u0027s close can be called multiple times and it won\u0027t return an error.\n\nWhich is exactly what our close on the IOWrapper does now, it has no effect if it\u0027s already closed.\n\nIf I remember correctly, even if self.closed is True this method will be called by the finalizer on garbage collection.","commit_id":"7eb8a2c9d76844ce633f0c89ff0e46abea3bdb0c"}]}