)]}'
{"cinderlib/objects.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"14bc03b46a4343a51869b271c36fcce6de9f932e","unresolved":false,"context_lines":[{"line_number":615,"context_line":"                   connector\u003dconnector,"},{"line_number":616,"context_line":"                   volume\u003dvolume,"},{"line_number":617,"context_line":"                   status\u003d\u0027attached\u0027,"},{"line_number":618,"context_line":"                   attach_mode\u003d\u0027rw\u0027,"},{"line_number":619,"context_line":"                   connection_info\u003d{\u0027conn\u0027: conn_info},"},{"line_number":620,"context_line":"                   **kwargs)"},{"line_number":621,"context_line":"        conn.connector_info \u003d connector"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_e4ed60b7","side":"PARENT","line":618,"range":{"start_line":618,"start_character":18,"end_line":618,"end_character":35},"updated":"2019-03-13 14:57:03.000000000","message":"Since we want it defined in all cases we may use something like:\n\n  attach_mode\u003dkwargs.get(\u0027attach_mode\u0027, \u0027rw\u0027)","commit_id":"f9dab3d6fb520c1d97279c5b154da3b08de2f5fc"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c718d37cd1ef3d69b68a57f372a47b18afd5aaa6","unresolved":false,"context_lines":[{"line_number":615,"context_line":"                   connector\u003dconnector,"},{"line_number":616,"context_line":"                   volume\u003dvolume,"},{"line_number":617,"context_line":"                   status\u003d\u0027attached\u0027,"},{"line_number":618,"context_line":"                   attach_mode\u003d\u0027rw\u0027,"},{"line_number":619,"context_line":"                   connection_info\u003d{\u0027conn\u0027: conn_info},"},{"line_number":620,"context_line":"                   **kwargs)"},{"line_number":621,"context_line":"        conn.connector_info \u003d connector"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_78f8237a","side":"PARENT","line":618,"range":{"start_line":618,"start_character":18,"end_line":618,"end_character":35},"in_reply_to":"5fc1f717_e4ed60b7","updated":"2019-03-14 10:55:59.000000000","message":"That would not be correct, as it would break in the same way it does now, because attach_mode is passed twice.\n\nThese are 2 other ways we could achieve the same idea that would work:\n\n  attach_mode\u003dkwargs.pop(\u0027attach_mode\u0027, \u0027rw\u0027)\n\nor\n\n  kwargs.setdefault(\u0027attach_mode\u0027, \u0027rw\u0027)\n\n\nBut I still prefer to leave the decision of setting a default to other parts of the code.","commit_id":"f9dab3d6fb520c1d97279c5b154da3b08de2f5fc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"14bc03b46a4343a51869b271c36fcce6de9f932e","unresolved":false,"context_lines":[{"line_number":666,"context_line":"    def conn_info(self):"},{"line_number":667,"context_line":"        conn_info \u003d self._ovo.connection_info"},{"line_number":668,"context_line":"        if conn_info:"},{"line_number":669,"context_line":"            conn \u003d conn_info.get(\u0027conn\u0027)"},{"line_number":670,"context_line":"            data \u003d conn.get(\u0027data\u0027)"},{"line_number":671,"context_line":"            if data is not None:"},{"line_number":672,"context_line":"                data[\u0027access_mode\u0027] \u003d self._ovo.attach_mode"},{"line_number":673,"context_line":"            return conn"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"        return {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_a43758ff","line":672,"range":{"start_line":669,"start_character":0,"end_line":672,"end_character":59},"updated":"2019-03-13 14:57:03.000000000","message":"I would suggest to move this in __init__ so we avoid any issue if someone try to get conn_info from self._ovo.connection_info. Also that is avoiding to recalculate this each time this \"property\" is called.","commit_id":"7b05dd548ee5b4abcef4a8aeb0b42bd2b3dad90a"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c718d37cd1ef3d69b68a57f372a47b18afd5aaa6","unresolved":false,"context_lines":[{"line_number":666,"context_line":"    def conn_info(self):"},{"line_number":667,"context_line":"        conn_info \u003d self._ovo.connection_info"},{"line_number":668,"context_line":"        if conn_info:"},{"line_number":669,"context_line":"            conn \u003d conn_info.get(\u0027conn\u0027)"},{"line_number":670,"context_line":"            data \u003d conn.get(\u0027data\u0027)"},{"line_number":671,"context_line":"            if data is not None:"},{"line_number":672,"context_line":"                data[\u0027access_mode\u0027] \u003d self._ovo.attach_mode"},{"line_number":673,"context_line":"            return conn"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"        return {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_783d8329","line":672,"range":{"start_line":669,"start_character":0,"end_line":672,"end_character":59},"in_reply_to":"5fc1f717_a43758ff","updated":"2019-03-14 10:55:59.000000000","message":"The reason why it\u0027s not in the init is because we could have a call to __init__ that doesn\u0027t have the `connection_info` set yet, and at some point it will have the data and we\u0027ll want to add the access_mode.  That\u0027s why it\u0027s here.\n\nI believe the most important case is the load of objects from persistence, so I\u0027m going to double check that\u0027s the only case and look for an alternative to set it in the `__init__` method.","commit_id":"7b05dd548ee5b4abcef4a8aeb0b42bd2b3dad90a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"689058e5c9a07bb36b5af012a4f1d888bcb37177","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        data \u003d self.conn_info.get(\u0027data\u0027)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_d1348d95","line":667,"range":{"start_line":667,"start_character":36,"end_line":667,"end_character":65},"updated":"2019-03-14 14:48:45.000000000","message":"data can be None here, right? You probably need to move this all part of code under line 669.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f3b34013ef07555ce81810bedc3cef9b674b6f64","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        data \u003d self.conn_info.get(\u0027data\u0027)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_628145fb","line":667,"range":{"start_line":667,"start_character":36,"end_line":667,"end_character":65},"in_reply_to":"5fc1f717_078df3d1","updated":"2019-03-14 16:31:29.000000000","message":"OK thanks. Why do you use \u0027self.conn_info.get(\u0027data\u0027)\u0027 it suggests the value returned can be None. Don\u0027t you think we could use \u0027self.conn_info[\u0027data\u0027]\u0027 to avoid misunderstood?\n\nAlso I have question, what will happen if \u0027self._ovo.attach_mode\u0027 is set when \u0027self.attach_mode\u0027 as-well?","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"90044d312092f291e75d087cdf8b9f607d2ea5d0","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        data \u003d self.conn_info.get(\u0027data\u0027)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_7df7918c","line":667,"range":{"start_line":667,"start_character":36,"end_line":667,"end_character":65},"in_reply_to":"5fc1f717_628145fb","updated":"2019-03-15 16:41:47.000000000","message":"`.get(\u0027data\u0027)` suggests the value may not be present, not that when it is present it can be `None`.\n\n`self.attach_mode` is the same as `self._ovo.attach_mode`, the `__getattr__` method does the conversion, so when I write in the if `self.attach_mode` is the short version of `self._ovo.attach_mode`.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6eabaa7f6ffca97ac1a077b711e9db782d35d0e9","unresolved":false,"context_lines":[{"line_number":664,"context_line":"        data \u003d self.conn_info.get(\u0027data\u0027)"},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_078df3d1","line":667,"range":{"start_line":667,"start_character":36,"end_line":667,"end_character":65},"in_reply_to":"5fc1f717_d1348d95","updated":"2019-03-14 15:52:22.000000000","message":"It should never be None, worst case it would be an empty dict, and probably not even that.\n\nAnd moving this below the line you suggest would make us not set a default attach_mode when data is an empty dict.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f3b34013ef07555ce81810bedc3cef9b674b6f64","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_3d112053","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"updated":"2019-03-14 16:31:29.000000000","message":"This part make me worry since if \u0027attach_mode\u0027 is not set an exception will be raised. Also why checking \u0027data\u0027 if at least it will be an empty dict?","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5156be083deb86c958ef986ff80c396e318d3a56","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_2ca3e602","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_19cc2e60","updated":"2019-03-19 11:48:56.000000000","message":"Perfect thanks for taking the time.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"90044d312092f291e75d087cdf8b9f607d2ea5d0","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_7d3c712e","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_3d112053","updated":"2019-03-15 16:41:47.000000000","message":"`attach_mode` will always be set, that\u0027s the whole point of L666-L667.\n\ndata could be None if the key was not present on L664.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8bca29a8a2fa79b788660a189cadd3a212d551e9","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_bcdf0dd5","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_7d3c712e","updated":"2019-03-19 08:41:32.000000000","message":"You are confusing me. It\u0027s basically the point I\u0027m trying to indicate to you, line L667, data could be None. So data.get(...) will fail badly.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f47c6a222033a4f4456bf79b1a0f311a38656735","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_19cc2e60","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_bcdf0dd5","updated":"2019-03-19 11:37:43.000000000","message":"Now I get what you meant. I\u0027ll update L664 to return a default value when not present.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"689058e5c9a07bb36b5af012a4f1d888bcb37177","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        if self._ovo.connection_info is None:"},{"line_number":686,"context_line":"            self._ovo.connection_info \u003d {}"},{"line_number":687,"context_line":"        value[\u0027data\u0027].setdefault(\u0027access_mode\u0027, self.attach_mode)"},{"line_number":688,"context_line":"        self._ovo.connection_info[\u0027conn\u0027] \u003d value"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_51e5fd07","line":687,"updated":"2019-03-14 14:48:45.000000000","message":"Really this looks to hacky and difficult to review for new contributors, can you add at least a comment? Thanks.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f3b34013ef07555ce81810bedc3cef9b674b6f64","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        if self._ovo.connection_info is None:"},{"line_number":686,"context_line":"            self._ovo.connection_info \u003d {}"},{"line_number":687,"context_line":"        value[\u0027data\u0027].setdefault(\u0027access_mode\u0027, self.attach_mode)"},{"line_number":688,"context_line":"        self._ovo.connection_info[\u0027conn\u0027] \u003d value"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_62af057d","line":687,"in_reply_to":"5fc1f717_077bb3bd","updated":"2019-03-14 16:31:29.000000000","message":"Thanks for the reference Gorka. It\u0027s not the function itself. Adding piece of code everywhere just to fix an issue does not not look right and a good way to maintain a library, more of that when you are expecting contributors to involve on it.\n\nThat is why I\u0027m suggesting to you to indicate why your are setting this here.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6eabaa7f6ffca97ac1a077b711e9db782d35d0e9","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        if self._ovo.connection_info is None:"},{"line_number":686,"context_line":"            self._ovo.connection_info \u003d {}"},{"line_number":687,"context_line":"        value[\u0027data\u0027].setdefault(\u0027access_mode\u0027, self.attach_mode)"},{"line_number":688,"context_line":"        self._ovo.connection_info[\u0027conn\u0027] \u003d value"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_077bb3bd","line":687,"in_reply_to":"5fc1f717_51e5fd07","updated":"2019-03-14 15:52:22.000000000","message":"A comment on `setdefault`? It\u0027s basic python, I believe a comment is not warranted? https://docs.python.org/2/library/stdtypes.html#dict.setdefault","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"90044d312092f291e75d087cdf8b9f607d2ea5d0","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        if self._ovo.connection_info is None:"},{"line_number":686,"context_line":"            self._ovo.connection_info \u003d {}"},{"line_number":687,"context_line":"        value[\u0027data\u0027].setdefault(\u0027access_mode\u0027, self.attach_mode)"},{"line_number":688,"context_line":"        self._ovo.connection_info[\u0027conn\u0027] \u003d value"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_ddc9e525","line":687,"in_reply_to":"5fc1f717_62af057d","updated":"2019-03-15 16:41:47.000000000","message":"What I don\u0027t understand is how is it hacky and what the comment should say: \"Setting access_mode if not present in the connection info to make sure we always have it present, even if the connection info is added after initialization\"?\n\nI can add that comment, or one on the lines of it, but I\u0027m not sure it adds that much value.","commit_id":"5b90c4b1b5a7cfeedb64f93500fe184e12fffd35"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8bca29a8a2fa79b788660a189cadd3a212d551e9","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"        if self._ovo.connection_info is None:"},{"line_number":686,"context_line":"            self._ovo.connection_info \u003d {}"},{"line_number":687,"context_line":"        # access_mode in the connection_info is set on __init__, here we ensure"},{"line_number":688,"context_line":"        # it\u0027s also set whenever we change the connection_info out of __init__."},{"line_number":689,"context_line":"        value[\u0027data\u0027].setdefault(\u0027access_mode\u0027, self.attach_mode)"},{"line_number":690,"context_line":"        self._ovo.connection_info[\u0027conn\u0027] \u003d value"},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3ced5d8b","line":689,"range":{"start_line":687,"start_character":0,"end_line":689,"end_character":65},"updated":"2019-03-19 08:41:32.000000000","message":"Thanks for this.","commit_id":"9271a80c5575c3406131bdb47314d8b54bbb88bb"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5156be083deb86c958ef986ff80c396e318d3a56","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_cc5ee211","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"updated":"2019-03-19 11:48:56.000000000","message":"Do we really need the condition now? What about to just set it in all case?","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5ebeba0bf19c4bf2d1dde8b17916d29cbcfee596","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_cf8c9481","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_cc5ee211","updated":"2019-03-19 12:06:16.000000000","message":"We can set in all cases, but when the \u0027data\u0027 key is not present we will be adding it to a local dictionary, so it will not be really saved in the connection information, which may be confusing when debugging things.\n\nEither way looks good to me.  One is more \"technically correct\" and the other is more readable.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"216bd114f2e04a18d7a8dbfaa447e04924f0edc8","unresolved":false,"context_lines":[{"line_number":666,"context_line":"        if not (self._ovo.obj_attr_is_set(\u0027attach_mode\u0027) and self.attach_mode):"},{"line_number":667,"context_line":"            self._ovo.attach_mode \u003d data.get(\u0027access_mode\u0027, \u0027rw\u0027)"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        if data:"},{"line_number":670,"context_line":"            data[\u0027access_mode\u0027] \u003d self.attach_mode"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    @property"},{"line_number":673,"context_line":"    def conn_info(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_54690689","line":670,"range":{"start_line":669,"start_character":0,"end_line":670,"end_character":50},"in_reply_to":"5fc1f717_cf8c9481","updated":"2019-03-20 07:56:36.000000000","message":"OK your choice.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"}],"cinderlib/tests/unit/objects/test_connection.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"216bd114f2e04a18d7a8dbfaa447e04924f0edc8","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_connect(self):"},{"line_number":115,"context_line":"        init_conn \u003d self.backend.driver.initialize_connection"},{"line_number":116,"context_line":"        init_conn.return_value \u003d {\u0027data\u0027: {}}"},{"line_number":117,"context_line":"        connector \u003d {\u0027my_c\u0027: \u0027v\u0027}"},{"line_number":118,"context_line":"        conn \u003d self.conn.connect(self.vol, connector)"},{"line_number":119,"context_line":"        init_conn.assert_called_once_with(self.vol, connector)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_f4e7d245","line":116,"updated":"2019-03-20 07:56:36.000000000","message":"This line is not necessary right?","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"48aebf2376521100f4354e2c8bdb17107811c035","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_connect(self):"},{"line_number":115,"context_line":"        init_conn \u003d self.backend.driver.initialize_connection"},{"line_number":116,"context_line":"        init_conn.return_value \u003d {\u0027data\u0027: {}}"},{"line_number":117,"context_line":"        connector \u003d {\u0027my_c\u0027: \u0027v\u0027}"},{"line_number":118,"context_line":"        conn \u003d self.conn.connect(self.vol, connector)"},{"line_number":119,"context_line":"        init_conn.assert_called_once_with(self.vol, connector)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_f050350d","line":116,"in_reply_to":"5fc1f717_b744ec75","updated":"2019-03-20 14:14:00.000000000","message":"I see, yes that makes sense. My point was to articulate the code when \u0027data\u0027 is not defined.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"69abf245058050ce81b647e340c639ca44611eae","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_connect(self):"},{"line_number":115,"context_line":"        init_conn \u003d self.backend.driver.initialize_connection"},{"line_number":116,"context_line":"        init_conn.return_value \u003d {\u0027data\u0027: {}}"},{"line_number":117,"context_line":"        connector \u003d {\u0027my_c\u0027: \u0027v\u0027}"},{"line_number":118,"context_line":"        conn \u003d self.conn.connect(self.vol, connector)"},{"line_number":119,"context_line":"        init_conn.assert_called_once_with(self.vol, connector)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_b744ec75","line":116,"in_reply_to":"5fc1f717_f4e7d245","updated":"2019-03-20 10:36:54.000000000","message":"It is.  As you can see by the use of \"return_value\",  initialize_connection is a Mock (actually, the whole self.backend.driver is a Mock), so it returns a Mock, which will break the code under test as it is not a valid return value for that method.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"216bd114f2e04a18d7a8dbfaa447e04924f0edc8","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                         self.conn._ovo.connection_info[\u0027conn\u0027])"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def test_conn_info_setter_clear(self):"},{"line_number":197,"context_line":"        self.conn.conn_info \u003d {\u0027data\u0027: {}}"},{"line_number":198,"context_line":"        self.conn.conn_info \u003d {}"},{"line_number":199,"context_line":"        self.assertIsNone(self.conn._ovo.connection_info)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_d4f656fa","line":197,"updated":"2019-03-20 07:56:36.000000000","message":"same here, this should be not necessary.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"48aebf2376521100f4354e2c8bdb17107811c035","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                         self.conn._ovo.connection_info[\u0027conn\u0027])"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def test_conn_info_setter_clear(self):"},{"line_number":197,"context_line":"        self.conn.conn_info \u003d {\u0027data\u0027: {}}"},{"line_number":198,"context_line":"        self.conn.conn_info \u003d {}"},{"line_number":199,"context_line":"        self.assertIsNone(self.conn._ovo.connection_info)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_703c45bf","line":197,"in_reply_to":"5fc1f717_1a51650f","updated":"2019-03-20 14:14:00.000000000","message":"ACK","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"69abf245058050ce81b647e340c639ca44611eae","unresolved":false,"context_lines":[{"line_number":194,"context_line":"                         self.conn._ovo.connection_info[\u0027conn\u0027])"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def test_conn_info_setter_clear(self):"},{"line_number":197,"context_line":"        self.conn.conn_info \u003d {\u0027data\u0027: {}}"},{"line_number":198,"context_line":"        self.conn.conn_info \u003d {}"},{"line_number":199,"context_line":"        self.assertIsNone(self.conn._ovo.connection_info)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_1a51650f","line":197,"in_reply_to":"5fc1f717_d4f656fa","updated":"2019-03-20 10:36:54.000000000","message":"This tests case is testing the conn_info property setter when we clear the property.  Since it defaults to None, we first make sure we have data, then we \"clear\" it setting it to an empty dict, and confirm that it sets to None.\n\nWe cannot leave the sentinel as it was before because we have added new code that uses the provided value to add the access_mode if possible and expects the argument to be either None or a dict, not a sentinel.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"216bd114f2e04a18d7a8dbfaa447e04924f0edc8","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        self.assertIsNone(self.conn._ovo.connection_info)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def test_conn_info_getter_default_attach_mode(self):"},{"line_number":202,"context_line":"        self.conn.conn_info \u003d {\u0027data\u0027: {}}"},{"line_number":203,"context_line":"        self.assertEqual({\u0027data\u0027: {\u0027access_mode\u0027: \u0027rw\u0027}}, self.conn.conn_info)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_conn_info_getter_ro(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_74dbe27c","line":202,"updated":"2019-03-20 07:56:36.000000000","message":"ditto.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"69abf245058050ce81b647e340c639ca44611eae","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        self.assertIsNone(self.conn._ovo.connection_info)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def test_conn_info_getter_default_attach_mode(self):"},{"line_number":202,"context_line":"        self.conn.conn_info \u003d {\u0027data\u0027: {}}"},{"line_number":203,"context_line":"        self.assertEqual({\u0027data\u0027: {\u0027access_mode\u0027: \u0027rw\u0027}}, self.conn.conn_info)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    def test_conn_info_getter_ro(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_3a49c15f","line":202,"in_reply_to":"5fc1f717_74dbe27c","updated":"2019-03-20 10:36:54.000000000","message":"conn_info is None here, so we set it to something to test that the getter returns what we have.\n\nWe cannot leave it with the sentinel as it was because we have added new code that uses the provided value to add the access_mode if possible.\n\nI will change it so that it\u0027s more clear we are getting what we are setting (since we are already checking the default access_mode on test_conn_info_setter)","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"216bd114f2e04a18d7a8dbfaa447e04924f0edc8","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def test_protocol(self):"},{"line_number":216,"context_line":"        self.conn.conn_info \u003d {\u0027driver_volume_type\u0027: mock.sentinel.iscsi,"},{"line_number":217,"context_line":"                               \u0027data\u0027: {}}"},{"line_number":218,"context_line":"        self.assertEqual(mock.sentinel.iscsi, self.conn.protocol)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def test_connector_info_setter(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_34e55a39","line":217,"updated":"2019-03-20 07:56:36.000000000","message":"ditto.","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"69abf245058050ce81b647e340c639ca44611eae","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def test_protocol(self):"},{"line_number":216,"context_line":"        self.conn.conn_info \u003d {\u0027driver_volume_type\u0027: mock.sentinel.iscsi,"},{"line_number":217,"context_line":"                               \u0027data\u0027: {}}"},{"line_number":218,"context_line":"        self.assertEqual(mock.sentinel.iscsi, self.conn.protocol)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def test_connector_info_setter(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_3aaa2119","line":217,"in_reply_to":"5fc1f717_34e55a39","updated":"2019-03-20 10:36:54.000000000","message":"Done","commit_id":"65bca34b3dfb9de1849052ebcf574783dc2a6014"}]}