)]}'
{"tempest/lib/services/identity/v3/role_assignments_client.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"83b427273388cd4aac582d218c69d8842e216674","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_project_effective_assignments("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_cb3b47ea","line":23,"range":{"start_line":23,"start_character":8,"end_line":23,"end_character":47},"updated":"2016-11-08 08:31:46.000000000","message":"we should rename this method before moving to lib how about this-\nlist_user_effective_role_assignments_on_project","commit_id":"1317d2eda17a84212662c83d6cc8ca111aaafc21"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"e4a08d4d91492997a900f8dcab4baba20cb5c9c1","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_project_effective_assignments("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_5bbb570e","line":23,"range":{"start_line":23,"start_character":8,"end_line":23,"end_character":47},"in_reply_to":"9a629dbe_22277865","updated":"2016-11-09 00:06:15.000000000","message":"After migrating the code under tempest.lib, it is difficult to rename. That should be done before migrating.","commit_id":"1317d2eda17a84212662c83d6cc8ca111aaafc21"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"564d071d70aecd750e58eda8bfdad549c1a93870","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_project_effective_assignments("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_b1d5e21c","line":23,"range":{"start_line":23,"start_character":8,"end_line":23,"end_character":47},"in_reply_to":"9a629dbe_5bbb570e","updated":"2016-11-11 23:28:32.000000000","message":"renamed to list_user_effective_assignments_on_project (removed the _role_, otherwise would be too big and difficult to handle pep8 constraints)","commit_id":"1317d2eda17a84212662c83d6cc8ca111aaafc21"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"c57c53bde8919cbd6636f412b89974baf58cc96c","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_project_effective_assignments("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":3,"id":"9a629dbe_22277865","line":23,"range":{"start_line":23,"start_character":8,"end_line":23,"end_character":47},"in_reply_to":"9a629dbe_cb3b47ea","updated":"2016-11-08 12:59:55.000000000","message":"makes sense, was just following the pattern in other lib files. Maybe change this in a follow up patch? What do you think?","commit_id":"1317d2eda17a84212662c83d6cc8ca111aaafc21"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"32ca94e184797f83682afccec33530f585e26540","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_a34ae5aa","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"updated":"2016-11-14 03:55:39.000000000","message":"\"role\" in method name will be better as current one does not give that clarity. \n\nThere is no pep8 issue with name i mentioned in previous comment.","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"c7ca161066961f31cc73ed7ce7d42afce0899832","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_675d2a48","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"in_reply_to":"7a77a97e_6292c64d","updated":"2016-11-21 13:39:48.000000000","message":"I get your point and I\u0027m pretty sure is a good one, but I still don\u0027t agree that creating a super generic method is the correct approach. We don\u0027t use all the capabilities of the API - why bother adding the support to them?\n\nIf, in the future, we start to have more and more methods in this client, we can think in a refactor - but as of today, I can\u0027t see a real advantage.\n\nBTW, thanks for being on top of this and keeping the reviews updated :)","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"ba6ac3dd0c01505ca942afcf4601600f679a03fa","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_6292c64d","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"in_reply_to":"7a77a97e_f45b9452","updated":"2016-11-18 09:14:17.000000000","message":"I am completely agree with you about complexity of this API. But in Tempest service client we do not need to have those if condition as done in keystoneclient. As there we need to provide simple arg options to user(not to ask user to prepare complex query).\n\nTempest service client is little bit different from those, here test cases can prepare query dict very easily and pass it as kwargs. And this client method just use urllib.urlencode to prepare query url from kwargs as shown below.\n\nThis is something we need in service client no other if condition etc. \n\nTaking effective_role as arg as query in API url is strange and go without value. \n\n    def list_role_assignments(self, effective_role\u003dFalse, **params):\n        \"\"\"List the role assignments for a user in a project.\"\"\"\n        url \u003d \u0027role_assignments\u0027\n        if params:\n            url +\u003d \u0027?%s\u0027 % urllib.urlencode(params)\n        if effective_role:\n            url +\u003d \u0027\u0026effective\u0027\n        resp, body \u003d self.get(url)\n        self.expected_success(200, resp.status)\n        body \u003d json.loads(body)\n        return rest_client.ResponseBody(resp, body)\n\nTest side-\nTest method can prepare the dict like below and use this method-\n        param \u003d {\u0027scope.project.id\u0027: self.project[\u0027id\u0027],\n                 \u0027user.id\u0027: self.user[\u0027id\u0027]}\n        assignments \u003d (\n            self.role_assignments.list_role_assignments(\n                effective_role\u003dTrue, **param))[\u0027role_assignments\u0027]\n\nI tested this and worked fine.","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"95d942f35fbe85f1c60b5fab712cd4b70b7e7112","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"9a629dbe_f66c99d2","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"in_reply_to":"9a629dbe_a34ae5aa","updated":"2016-11-14 05:40:02.000000000","message":"After looking at API ref[1], role_assignments is an alternate API for listing the roles with queried param (user, group on domain, project and with effective or direct)\n\nWe should not add separate method for each query combination of url. \nLet\u0027s keep lib method generic and tests can query what actually they want \n- list user role on domain\n- list user role on project\n- list group role on domain\n- list group role on project\n- effective role\nAll these can be fetched with same url with different query param.\n\nSo how about this- \n\ndef list_role_assignments(self, **param):\n        \"\"\"List the effective role assignments for a user in a project.\"\"\"\n        url \u003d \u0027role_assignments\u0027\n        if params:\n            url +\u003d \u0027?%s\u0027 % urllib.urlencode(params)\n        resp, body \u003d self.get(url)\n        self.expected_success(200, resp.status)\n        body \u003d json.loads(body)\n        return rest_client.ResponseBody(resp, body)\n\n..[1] http://developer.openstack.org/api-ref/identity/v3/index.html?expanded\u003d#roles","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"dd5d65b51304c010ab313ce2488e35dbfef9b7ac","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_14301001","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"in_reply_to":"9a629dbe_a34ae5aa","updated":"2016-11-15 12:41:55.000000000","message":"with too long names, we can easily pass the 79 length pep8 limit per line due indentation.","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"dd5d65b51304c010ab313ce2488e35dbfef9b7ac","unresolved":false,"context_lines":[{"line_number":20,"context_line":"class RoleAssignmentsClient(rest_client.RestClient):"},{"line_number":21,"context_line":"    api_version \u003d \"v3\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def list_user_effective_assignments_on_project("},{"line_number":24,"context_line":"            self, project_id, user_id):"},{"line_number":25,"context_line":"        \"\"\"List the effective role assignments for a user in a project.\"\"\""},{"line_number":26,"context_line":"        resp, body \u003d self.get("}],"source_content_type":"text/x-python","patch_set":4,"id":"7a77a97e_f45b9452","line":23,"range":{"start_line":23,"start_character":7,"end_line":23,"end_character":50},"in_reply_to":"9a629dbe_f66c99d2","updated":"2016-11-15 12:41:55.000000000","message":"this is a good suggestion, but I think it might not be the best option. In tempest we want to have the simplest calls we can have, the role_assignments API is not simple, actually, is the most \"complex\" API on keystone if I can say. Being a keystone core for a while, this is something I\u0027m trying to avoid to happen again.\n\nI have implemented this feature on keystoneclient myself some years ago and it looks like that [2]. To implement the method in the way you are suggesting, we would require the same amount of conditionals or the usage of a dictionary - don\u0027t think either of the options can lead to a clean client to be called on tests.\n\nAnother problem is that we don\u0027t have any tests that uses the capabilities from this API, that\u0027s the reason we have only one method.\n\n[2] https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/v3/role_assignments.py#L49-L101","commit_id":"abf3f5535b136a16f1c411c5c4be3304f15af63d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a224ef2dcfe2826c80372125cc3b069b5d5e46b0","unresolved":false,"context_lines":[{"line_number":12,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":13,"context_line":"# limitations under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"from six.moves.urllib import parse as urllib"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_serialization import jsonutils as json"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from tempest.lib.common import rest_client"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_ed8aa4be","line":17,"range":{"start_line":15,"start_character":0,"end_line":17,"end_character":48},"updated":"2016-11-25 00:21:58.000000000","message":"these import should be together","commit_id":"312cb83b31a6baed8132bbc4c27f0bf18fbdd7be"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"a224ef2dcfe2826c80372125cc3b069b5d5e46b0","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        \"\"\"List role assignments.\"\"\""},{"line_number":27,"context_line":"        url \u003d \u0027role_assignments\u0027"},{"line_number":28,"context_line":"        if kwargs:"},{"line_number":29,"context_line":"            # NOTE(rodrigods): \"effective\" is a key-only query parameter and"},{"line_number":30,"context_line":"            # is treated below."},{"line_number":31,"context_line":"            if \u0027effective\u0027 in kwargs:"},{"line_number":32,"context_line":"                del kwargs[\u0027effective\u0027]"},{"line_number":33,"context_line":"            url +\u003d \u0027?%s\u0027 % urllib.urlencode(kwargs)"},{"line_number":34,"context_line":"        if effective:"},{"line_number":35,"context_line":"            url +\u003d \u0027\u0026effective\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a74a57a_ad6eecfe","line":32,"range":{"start_line":29,"start_character":0,"end_line":32,"end_character":39},"updated":"2016-11-25 00:21:58.000000000","message":"this actually not needed, this can hide wrong usage. We should explain this param in doc string.","commit_id":"312cb83b31a6baed8132bbc4c27f0bf18fbdd7be"}]}