)]}'
{"rally/plugins/openstack/context/keystone/roles.py":[{"author":{"_account_id":9545,"name":"Andriy Kurilin","email":"andr.kurilin@gmail.com","username":"akurilin"},"change_message_id":"cea4c24c69c078999df5cefebf4ba43eec5e0bde","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if not self._is_role_assigned(role.id, user[\"id\"],"},{"line_number":100,"context_line":"                                                  user[\"tenant_id\"]):"},{"line_number":101,"context_line":"                        args \u003d (role.id, user[\"id\"], user[\"tenant_id\"])"},{"line_number":102,"context_line":"                        queue.append(args)"},{"line_number":103,"context_line":"                        user[\"role_assigned\"] \u003d True"},{"line_number":104,"context_line":"                    else:"},{"line_number":105,"context_line":"                        user[\"role_assigned\"] \u003d False"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        broker.run(publish, self._get_consumer(\"add_role\"), threads)"},{"line_number":108,"context_line":"        self.context[\"roles\"] \u003d roles_dict"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f91af0f_55205fac","line":105,"range":{"start_line":99,"start_character":20,"end_line":105,"end_character":53},"updated":"2018-01-10 10:54:45.000000000","message":"There is a case, when more then one role should be assigned to the user. For each role, list_roles will be called. It is redundant. Also, in case if one role is already assigned and another is not, there will be a conflict.\n\nI suggest to do the next thing:\n\n     for user in self.context[\"users\"]:\n         if \"roles\" not in user:\n             # a list of all user roles\n             user[\"roles\"] \u003d list_roles()\n             # a list of assigned roles, which can be used in cleanup\n             user[\"assigned_roles\"] \u003d []\n\n         if role.id not in user[\"roles\"]:\n             queue.append(args)\n             user[\"roles\"].append(role.id)\n             user[\"assigned_roles\"].append(role.id)","commit_id":"fddaf5330e97c0d574edb5d39f94eb55a6aebfeb"},{"author":{"_account_id":26025,"name":"chenxu","email":"xuchen1990xx@gmail.com","username":"chenxu"},"change_message_id":"96335596fab5537f52adaba5bc789457aa7233cb","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self.context[\"roles\"] \u003d roles_dict"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def cleanup(self):"},{"line_number":111,"context_line":"        \"\"\"Remove all roles from users.\"\"\""},{"line_number":112,"context_line":"        threads \u003d self.workers"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        def publish(queue):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f91af0f_dc3901f2","line":111,"range":{"start_line":111,"start_character":8,"end_line":111,"end_character":42},"updated":"2018-01-10 03:12:23.000000000","message":"This comment also should be changed as \"removing assigned roles from all user\".","commit_id":"fddaf5330e97c0d574edb5d39f94eb55a6aebfeb"},{"author":{"_account_id":26025,"name":"chenxu","email":"xuchen1990xx@gmail.com","username":"chenxu"},"change_message_id":"96335596fab5537f52adaba5bc789457aa7233cb","unresolved":false,"context_lines":[{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        def publish(queue):"},{"line_number":115,"context_line":"            for role_id in self.context[\"roles\"]:"},{"line_number":116,"context_line":"                LOG.debug(\"Removing role %s from all users\" % role_id)"},{"line_number":117,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":118,"context_line":"                    if user.get(\"role_assigned\", True):"},{"line_number":119,"context_line":"                        args \u003d (role_id, user[\"id\"], user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f91af0f_3c202dac","line":116,"range":{"start_line":116,"start_character":27,"end_line":116,"end_character":58},"updated":"2018-01-10 03:12:23.000000000","message":"and here :)","commit_id":"fddaf5330e97c0d574edb5d39f94eb55a6aebfeb"},{"author":{"_account_id":26025,"name":"chenxu","email":"xuchen1990xx@gmail.com","username":"chenxu"},"change_message_id":"96335596fab5537f52adaba5bc789457aa7233cb","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            for role_id in self.context[\"roles\"]:"},{"line_number":116,"context_line":"                LOG.debug(\"Removing role %s from all users\" % role_id)"},{"line_number":117,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":118,"context_line":"                    if user.get(\"role_assigned\", True):"},{"line_number":119,"context_line":"                        args \u003d (role_id, user[\"id\"], user[\"tenant_id\"])"},{"line_number":120,"context_line":"                        queue.append(args)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f91af0f_701b3972","line":118,"range":{"start_line":118,"start_character":23,"end_line":118,"end_character":54},"updated":"2018-01-10 03:12:23.000000000","message":"All user have \"role_assigned\" item, does it need a default \"True\" ?","commit_id":"fddaf5330e97c0d574edb5d39f94eb55a6aebfeb"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"06cd6ff194ec98f34c4185f6994319c505ff718c","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_837c6e72","line":99,"updated":"2018-01-18 04:38:04.000000000","message":"Not sure that I understand this line. \n\nIf you take a look at the users context, user has only 3 fields: id, credential, tenant_id, or in other words this condition is always True\n\nseems like a bug..","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":21528,"name":"chenhb","email":"chen.haibing1@zte.com.cn","username":"chenhb"},"change_message_id":"e67f2d90702774fbf713c4990f5192eac251aa70","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_9e928456","line":99,"in_reply_to":"7f96bb07_7bd22a2a","updated":"2018-01-18 07:08:33.000000000","message":"it is here. add the \"roles\" to user at first loop.\nOf course, looks it is not a nice way, it is hard to understand.","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":27587,"name":"Srinivas Sakhamuri","email":"ssakhamuri@bloomberg.net","username":"ssakhamuri"},"change_message_id":"2a656f5e253040ca9c576272703c9c87ca68a50c","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_54204d9b","line":99,"in_reply_to":"7f96bb07_8053f96a","updated":"2018-01-18 15:21:49.000000000","message":"You have simplified 2 step, we also need to consider logic L91-92 which is essentially another loop. I think current logic is optimized and does things on the fly when necessary.","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":21528,"name":"chenhb","email":"chen.haibing1@zte.com.cn","username":"chenhb"},"change_message_id":"52a678f74a61df7755feb494426ad0ab2d7a221d","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_e9a0b3f6","line":99,"in_reply_to":"7f96bb07_837c6e72","updated":"2018-01-18 05:54:42.000000000","message":"@boris due to this is a nested `for`, so ... :)","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":9545,"name":"Andriy Kurilin","email":"andr.kurilin@gmail.com","username":"akurilin"},"change_message_id":"084d8992a59bc18db04c119228292ea73bc69558","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_a03f1585","line":99,"in_reply_to":"7f96bb07_837c6e72","updated":"2018-01-18 10:13:49.000000000","message":"There is a case when several roles should be assigned to a user.\n\nIn the first loop(L90), the user doesn\u0027t have \"roles\" key and L100 will add it.\nFor the second loop(L90), the user will have \"roles\" and no additional call to keystone will be needed.\nSo it is a cache","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"8122dc44e10ec1199c8d2601428892948041608a","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_8053f96a","line":99,"in_reply_to":"7f96bb07_a03f1585","updated":"2018-01-18 10:26:48.000000000","message":"I think that the logic is going to be drastically simplified if we use 2 separated loops \n\nsetup: \n1) 1 loop: iterate over users and save roles \n2) 2 loop: iterate over users and add missing roles \n\ncleanup: \n1) iterate over users, iterate over roles if roles not in existed_before delete it.","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"9d9f4b27cf85ed5934ccd579498eff65c815bba2","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_7bd22a2a","line":99,"in_reply_to":"7f96bb07_c9913749","updated":"2018-01-18 06:38:21.000000000","message":"can you show me the place where \"roles\" are added to user?","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"aba417623f19b76273c4ba1720ffe03b4c57e4bd","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                             \"role_id\": role.id,"},{"line_number":97,"context_line":"                             \"threads\": threads})"},{"line_number":98,"context_line":"                for user in self.context[\"users\"]:"},{"line_number":99,"context_line":"                    if \"roles\" not in user:"},{"line_number":100,"context_line":"                        user[\"roles\"] \u003d self._get_user_role_ids("},{"line_number":101,"context_line":"                            user[\"id\"],"},{"line_number":102,"context_line":"                            user[\"tenant_id\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f96bb07_c9913749","line":99,"in_reply_to":"7f96bb07_e9a0b3f6","updated":"2018-01-18 06:12:09.000000000","message":"and? every element of \"users\"  has only those 3 keys...","commit_id":"a1d40cd733360be15d6a38c66385cf475ea4feef"}]}