)]}'
{"manila/share/drivers/cephfs/cephfs_native.py":[{"author":{"_account_id":13998,"name":"Daniel Gonzalez Nothnagel","email":"daniel.gonzalez.nothnagel@sap.com","username":"dgonzalez"},"change_message_id":"f78d93c9395d3a0904d31e4130497c362d2c0e09","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            access[\u0027access_to\u0027],"},{"line_number":242,"context_line":"            volume_path\u003dself._share_path(share))"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def update_access(self, context, share, access_rules, add_rules,"},{"line_number":245,"context_line":"                      delete_rules, share_server\u003dNone):"},{"line_number":246,"context_line":"        access_keys \u003d {}"},{"line_number":247,"context_line":"        access_rules \u003d access_rules or []"},{"line_number":248,"context_line":"        add_rules \u003d add_rules or []"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_7e882223","line":245,"range":{"start_line":244,"start_character":44,"end_line":245,"end_character":34},"updated":"2016-08-09 07:06:55.000000000","message":"You could set [] as default value here and remove lines 247-249 for it.","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"d2464b96b729080938770d7b7902ac7618d16f4d","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            access[\u0027access_to\u0027],"},{"line_number":242,"context_line":"            volume_path\u003dself._share_path(share))"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def update_access(self, context, share, access_rules, add_rules,"},{"line_number":245,"context_line":"                      delete_rules, share_server\u003dNone):"},{"line_number":246,"context_line":"        access_keys \u003d {}"},{"line_number":247,"context_line":"        access_rules \u003d access_rules or []"},{"line_number":248,"context_line":"        add_rules \u003d add_rules or []"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_95a96c7e","line":245,"range":{"start_line":244,"start_character":44,"end_line":245,"end_character":34},"in_reply_to":"9ad45d7e_7e882223","updated":"2016-08-09 10:48:09.000000000","message":"As Zhongjun has pointed out, I shouldn\u0027t bother about them being anything other than a list.","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":15100,"name":"zhongjun","email":"jun.zhongjun2@gmail.com","username":"zhongjun2"},"change_message_id":"5fd0f6ec166b4c0596be905bcdc0b8da20a53f2d","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def update_access(self, context, share, access_rules, add_rules,"},{"line_number":245,"context_line":"                      delete_rules, share_server\u003dNone):"},{"line_number":246,"context_line":"        access_keys \u003d {}"},{"line_number":247,"context_line":"        access_rules \u003d access_rules or []"},{"line_number":248,"context_line":"        add_rules \u003d add_rules or []"},{"line_number":249,"context_line":"        delete_rules \u003d delete_rules or []"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        if not (add_rules or delete_rules):  # recovery/maintenance mode"},{"line_number":252,"context_line":"            add_rules \u003d access_rules"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_c8ceb059","line":249,"range":{"start_line":247,"start_character":8,"end_line":249,"end_character":41},"updated":"2016-08-09 09:51:34.000000000","message":"The default value is \u0027[]\u0027, please see the following links. so these three lines could be removed.  Is there a scenario? the default value could not be empty list(\u0027[]\u0027)?\nhttps://github.com/openstack/manila/blob/master/manila/share/driver.py#L530\n\nhttps://github.com/openstack/manila/blob/master/manila/share/access.py#L54","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"d2464b96b729080938770d7b7902ac7618d16f4d","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    def update_access(self, context, share, access_rules, add_rules,"},{"line_number":245,"context_line":"                      delete_rules, share_server\u003dNone):"},{"line_number":246,"context_line":"        access_keys \u003d {}"},{"line_number":247,"context_line":"        access_rules \u003d access_rules or []"},{"line_number":248,"context_line":"        add_rules \u003d add_rules or []"},{"line_number":249,"context_line":"        delete_rules \u003d delete_rules or []"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        if not (add_rules or delete_rules):  # recovery/maintenance mode"},{"line_number":252,"context_line":"            add_rules \u003d access_rules"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_95c04c2c","line":249,"range":{"start_line":247,"start_character":8,"end_line":249,"end_character":41},"in_reply_to":"9ad45d7e_c8ceb059","updated":"2016-08-09 10:48:09.000000000","message":"You\u0027re right. Thanks! The docstring of ShareDriver\u0027s update_access() says that add_rules and delete_rules can either be an empty or non-empty list. And seems like even access_rules can either be an empty or non-empty list.","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":18643,"name":"John Spray","email":"jspray@redhat.com","username":"jcsp"},"change_message_id":"3655542376ff52ad681012ccca32f21d9d42db31","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            add_rules \u003d access_rules"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"            existing_auths \u003d ("},{"line_number":252,"context_line":"                self.volume_client.get_authorized_ids(self._share_path(share)))"},{"line_number":253,"context_line":"            if existing_auths:"},{"line_number":254,"context_line":"                existing_auth_ids \u003d set("},{"line_number":255,"context_line":"                    [auth[0] for auth in existing_auths])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_d998f6c2","line":252,"updated":"2016-08-10 12:55:33.000000000","message":"Don\u0027t we need to do a volume client version check here?  Version \u00270\u0027 (i.e. when getattr(CephFSVolumeClient, \u0027version\u0027, None) is None) does not have the get_authorized_ids method.","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"9ea07b9f28a46d34cb0d5ec867d9d2741580575f","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            add_rules \u003d access_rules"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"            existing_auths \u003d ("},{"line_number":252,"context_line":"                self.volume_client.get_authorized_ids(self._share_path(share)))"},{"line_number":253,"context_line":"            if existing_auths:"},{"line_number":254,"context_line":"                existing_auth_ids \u003d set("},{"line_number":255,"context_line":"                    [auth[0] for auth in existing_auths])"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_d425225c","line":252,"in_reply_to":"9ad45d7e_d998f6c2","updated":"2016-08-11 12:21:00.000000000","message":"Done","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":9521,"name":"Csaba Henk","email":"chenk@redhat.com","username":"chenk"},"change_message_id":"ceecee39816cdf8e15585e8cfe917911c3b48a7f","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        for rule in add_rules:"},{"line_number":266,"context_line":"            access_key \u003d self._allow_access(context, share, rule)"},{"line_number":267,"context_line":"            access_keys.update({rule[\u0027id\u0027]: access_key})"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        for rule in delete_rules:"},{"line_number":270,"context_line":"            self._deny_access(context, share, rule)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_209e3b63","line":267,"updated":"2016-08-10 12:09:41.000000000","message":"There is no point in checking for the intersection of (ids of) add_rules and existing_auth_ids and take them out from the update sequence? Probably (I can speculate) updating rules is cheap \u0026 idempotent?\n\nProbably it would help the reader if she could spared from such guesswork and elaborate on this choice in a comment.","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":18643,"name":"John Spray","email":"jspray@redhat.com","username":"jcsp"},"change_message_id":"3655542376ff52ad681012ccca32f21d9d42db31","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        for rule in add_rules:"},{"line_number":266,"context_line":"            access_key \u003d self._allow_access(context, share, rule)"},{"line_number":267,"context_line":"            access_keys.update({rule[\u0027id\u0027]: access_key})"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        for rule in delete_rules:"},{"line_number":270,"context_line":"            self._deny_access(context, share, rule)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_b9126255","line":267,"in_reply_to":"9ad45d7e_209e3b63","updated":"2016-08-10 12:55:33.000000000","message":"I would imagine this is done so that in recovery mode we are learning all the access keys.  But yes, we should have a comment to explain that.","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"9ea07b9f28a46d34cb0d5ec867d9d2741580575f","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        for rule in add_rules:"},{"line_number":266,"context_line":"            access_key \u003d self._allow_access(context, share, rule)"},{"line_number":267,"context_line":"            access_keys.update({rule[\u0027id\u0027]: access_key})"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        for rule in delete_rules:"},{"line_number":270,"context_line":"            self._deny_access(context, share, rule)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_7466f651","line":267,"in_reply_to":"9ad45d7e_b9126255","updated":"2016-08-11 12:21:00.000000000","message":"Done","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":9521,"name":"Csaba Henk","email":"chenk@redhat.com","username":"chenk"},"change_message_id":"ceecee39816cdf8e15585e8cfe917911c3b48a7f","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        for rule in delete_rules:"},{"line_number":270,"context_line":"            self._deny_access(context, share, rule)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        return access_keys"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def delete_share(self, context, share, share_server\u003dNone):"},{"line_number":275,"context_line":"        extra_specs \u003d share_types.get_extra_specs_from_share(share)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_00c05729","line":272,"updated":"2016-08-10 12:09:41.000000000","message":"Why do you return stuff here when earlier no stuff was returned?","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"9ea07b9f28a46d34cb0d5ec867d9d2741580575f","unresolved":false,"context_lines":[{"line_number":269,"context_line":"        for rule in delete_rules:"},{"line_number":270,"context_line":"            self._deny_access(context, share, rule)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        return access_keys"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def delete_share(self, context, share, share_server\u003dNone):"},{"line_number":275,"context_line":"        extra_specs \u003d share_types.get_extra_specs_from_share(share)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad45d7e_e056e35a","line":272,"in_reply_to":"9ad45d7e_00c05729","updated":"2016-08-11 12:21:00.000000000","message":"Csaba, this is needed once Manila can add these returned access_keys to its DB and expose it to the user. The Manila \u0027core\u0027 patch to do so is in review, https://review.openstack.org/#/c/343306/\nThe intention is to use this patch to help functionally test\nthe manila \u0027core\u0027 patch.\n\nI mention the need for this new return value in the commit message.","commit_id":"614bf48d4e9535b344e46e88a9a2af773ecd676b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"fd5ef88279012aae7dcf236643118fdc05c35d08","unresolved":false,"context_lines":[{"line_number":272,"context_line":"                delete_auth_ids \u003d existing_auth_ids.difference("},{"line_number":273,"context_line":"                    want_auth_ids)"},{"line_number":274,"context_line":"                for delete_auth_id in delete_auth_ids:"},{"line_number":275,"context_line":"                    delete_rules.append("},{"line_number":276,"context_line":"                        {"},{"line_number":277,"context_line":"                            \u0027access_to\u0027: delete_auth_id,"},{"line_number":278,"context_line":"                            \u0027access_type\u0027: CEPHX_ACCESS_TYPE,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9ad45d7e_5fa494b8","line":275,"range":{"start_line":275,"start_character":0,"end_line":275,"end_character":40},"updated":"2016-08-11 19:55:27.000000000","message":"OK, looks like ShareInstanceAccess.update_access_rules ensures that delete_rules will be [] rather than None.\n\n(update: now see you folks already covered that discussing patch set 3).","commit_id":"ed9935aad8bcfb0289d53b9fba1ab703214d0dcc"}],"manila/tests/share/drivers/cephfs/test_cephfs_native.py":[{"author":{"_account_id":15100,"name":"zhongjun","email":"jun.zhongjun2@gmail.com","username":"zhongjun2"},"change_message_id":"5fd0f6ec166b4c0596be905bcdc0b8da20a53f2d","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        access_keys \u003d self._driver.update_access(self._context, self._share,"},{"line_number":256,"context_line":"                                                 access_rules\u003d[alice],"},{"line_number":257,"context_line":"                                                 add_rules\u003dNone,"},{"line_number":258,"context_line":"                                                 delete_rules\u003dNone)"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        self.assertEqual({\u0027accessid1\u0027: \u0027abc123\u0027}, access_keys)"},{"line_number":261,"context_line":"        (self._driver._volume_client.get_authorized_ids."}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_af4c2171","line":258,"range":{"start_line":257,"start_character":49,"end_line":258,"end_character":66},"updated":"2016-08-09 09:51:34.000000000","message":"The default value is empty list?https://github.com/openstack/manila/blob/master/manila/share/driver.py#L530\n\nhttps://github.com/openstack/manila/blob/master/manila/share/access.py#L54","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"d2464b96b729080938770d7b7902ac7618d16f4d","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        access_keys \u003d self._driver.update_access(self._context, self._share,"},{"line_number":256,"context_line":"                                                 access_rules\u003d[alice],"},{"line_number":257,"context_line":"                                                 add_rules\u003dNone,"},{"line_number":258,"context_line":"                                                 delete_rules\u003dNone)"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        self.assertEqual({\u0027accessid1\u0027: \u0027abc123\u0027}, access_keys)"},{"line_number":261,"context_line":"        (self._driver._volume_client.get_authorized_ids."}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad45d7e_d58be41c","line":258,"range":{"start_line":257,"start_character":49,"end_line":258,"end_character":66},"in_reply_to":"9ad45d7e_af4c2171","updated":"2016-08-09 10:48:09.000000000","message":"Done","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"fd5ef88279012aae7dcf236643118fdc05c35d08","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        auth_key \u003d self._driver._allow_access("},{"line_number":178,"context_line":"            self._context, self._share, rule)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self.assertEqual(\"abc123\", auth_key)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        if not volume_client_version:"},{"line_number":183,"context_line":"            self._driver._volume_client.authorize.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":8,"id":"9ad45d7e_dbe6d742","line":180,"range":{"start_line":180,"start_character":25,"end_line":180,"end_character":33},"updated":"2016-08-11 19:55:27.000000000","message":"nit: \n\nIt would be nice to have this as a constant, also used in CephFSVolumeClient.__init__() and in the following tests.  If we do this it should probably be in another patch that also catches other constants used in asserts.","commit_id":"ed9935aad8bcfb0289d53b9fba1ab703214d0dcc"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"4673ed401b5adeb4d3bb45931b9006ae5ab709e4","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        auth_key \u003d self._driver._allow_access("},{"line_number":178,"context_line":"            self._context, self._share, rule)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self.assertEqual(\"abc123\", auth_key)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        if not volume_client_version:"},{"line_number":183,"context_line":"            self._driver._volume_client.authorize.assert_called_once_with("}],"source_content_type":"text/x-python","patch_set":8,"id":"9ad45d7e_27537a6e","line":180,"range":{"start_line":180,"start_character":25,"end_line":180,"end_character":33},"in_reply_to":"9ad45d7e_dbe6d742","updated":"2016-08-12 08:42:36.000000000","message":"Let me do this in a follow-up patch. Thanks!","commit_id":"ed9935aad8bcfb0289d53b9fba1ab703214d0dcc"}],"releasenotes/notes/cephfs-native-enhance-update-access-support-e1a1258084c997ca.yaml":[{"author":{"_account_id":15100,"name":"zhongjun","email":"jun.zhongjun2@gmail.com","username":"zhongjun2"},"change_message_id":"5fd0f6ec166b4c0596be905bcdc0b8da20a53f2d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Using ability to fetch exisiting access rules in"},{"line_number":4,"context_line":"    the Ceph backend, enhanced the the cephfs_native"},{"line_number":5,"context_line":"    driver to fully implement update_access()."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9ad45d7e_6f8a29c1","line":3,"range":{"start_line":3,"start_character":27,"end_line":3,"end_character":36},"updated":"2016-08-09 09:51:34.000000000","message":"s/ exisiting /existing","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"d2464b96b729080938770d7b7902ac7618d16f4d","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Using ability to fetch exisiting access rules in"},{"line_number":4,"context_line":"    the Ceph backend, enhanced the the cephfs_native"},{"line_number":5,"context_line":"    driver to fully implement update_access()."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9ad45d7e_383cf5e1","line":3,"range":{"start_line":3,"start_character":27,"end_line":3,"end_character":36},"in_reply_to":"9ad45d7e_6f8a29c1","updated":"2016-08-09 10:48:09.000000000","message":"Done","commit_id":"d1e3aa4caaacc870778a0b76d21b018ad00d2c83"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d5ecd6fc4e117196249cbebe94b7136027b11237","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Enhanced ``cephfs_native`` driver\u0027s update_access() to,"},{"line_number":4,"context_line":"    a) remove undesired rules existing in the backend during recovery mode."},{"line_number":5,"context_line":"    b) return ``access_keys`` of ceph auth IDs that are allowed access."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"9ad45d7e_bc1787a1","line":5,"range":{"start_line":4,"start_character":3,"end_line":5,"end_character":71},"updated":"2016-08-12 02:55:12.000000000","message":"concatenation removes line breaks; if you want line breaks, you should use: |\n\nfeatures:\n    - |\n      Enhanced ``cephfs_native`` driver\u0027s update_access() to,\n \n      - remove undesired rules existing in the backend during recovery mode.\n      - return ``access_keys`` of ceph auth IDs that are allowed access.","commit_id":"ed9935aad8bcfb0289d53b9fba1ab703214d0dcc"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"4673ed401b5adeb4d3bb45931b9006ae5ab709e4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - Enhanced ``cephfs_native`` driver\u0027s update_access() to,"},{"line_number":4,"context_line":"    a) remove undesired rules existing in the backend during recovery mode."},{"line_number":5,"context_line":"    b) return ``access_keys`` of ceph auth IDs that are allowed access."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"9ad45d7e_47e14e9a","line":5,"range":{"start_line":4,"start_character":3,"end_line":5,"end_character":71},"in_reply_to":"9ad45d7e_bc1787a1","updated":"2016-08-12 08:42:36.000000000","message":"Good to know. Done.","commit_id":"ed9935aad8bcfb0289d53b9fba1ab703214d0dcc"}]}