)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2016-03-04 16:45:06 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"initial commit"},{"line_number":8,"context_line":"changes made in shell.py to authenticate the tenant-id"},{"line_number":9,"context_line":"Closes-Bug: 1551103"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I185dc37289dbf786b2750b5626de3e25094d1b51"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7af24918_2cf7b1f6","line":8,"updated":"2016-03-04 23:08:56.000000000","message":"This commit message needs a summary line and a paragraph describing the change in detail.  Please see [1] for details on how we like commit messages formatted.\n\n[1] https://wiki.openstack.org/wiki/GitCommitMessages","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"}],"novaclient/v2/shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import time"},{"line_number":30,"context_line":"import warnings"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"import keystoneclient.v2_0.client as ksclient"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from oslo_utils import encodeutils"},{"line_number":35,"context_line":"from oslo_utils import netutils"}],"source_content_type":"text/x-python","patch_set":1,"id":"7af24918_4c403d08","line":32,"updated":"2016-03-04 23:08:56.000000000","message":"This is a third-party import, so it should be grouped together with the imports from oslo_utils and six, in proper alphabetical order.  Additionally, keystoneclient is not a dependency of novaclient, so an update to requirements.txt will be required.\n\n(I\u0027m not convinced that this is what you should be using, as it hard-codes the client version; novaclient already uses keystoneauth1 as a dependency, and I would expect that to be used.)","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"b2e00b8679448f7967e4a37fd1bb376aed78108f","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import time"},{"line_number":30,"context_line":"import warnings"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"import keystoneclient.v2_0.client as ksclient"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"from oslo_utils import encodeutils"},{"line_number":35,"context_line":"from oslo_utils import netutils"}],"source_content_type":"text/x-python","patch_set":1,"id":"5aef4532_644d4072","line":32,"in_reply_to":"7af24918_4c403d08","updated":"2016-03-07 17:13:08.000000000","message":"I was asked for clarification in private correspondence regarding my comments here.  I have three or four basic comments:\n\n1. This import is in the wrong place; see [1] for an explanation of how imports should be laid out in OpenStack projects.  The simple solution here is to simply remove the blank line on line 33.\n\n2. The python-keystoneclient package is not listed in python-novaclient\u0027s requirements.txt.  That means that this import will always fail tests because the package is not installed.  If we are to go this route, using keystoneclient directly this way, then the dependency must be added.  (Note that the line added to requirements.txt must be identical to the line as shown in the OpenStack \"requirements\" repository, listed in \"global-requirements.txt\".)\n\n3. My parenthetical comment relates to the fact that python-novaclient already has a dependency on keystoneauth1, which is a smaller package that solely deals with authentication.  If that package does not have the necessary support for the operation you need, then a comment to that effect should be included in the commit message.\n\n4. I\u0027m also concerned that you\u0027re hardcoding the usage of version 2.0 of the python-keystoneclient API.  I would expect that python-keystoneclient includes a non-versioned API that returns an appropriate Client, similar to novaclient.client.Client; that\u0027s the interface we should use here, unless there\u0027s a good reason not to.\n\n[1] http://docs.openstack.org/developer/hacking/#import-order-template","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":4404,"context_line":"    columns \u003d [\u0027Quota\u0027, \u0027Limit\u0027]"},{"line_number":4405,"context_line":"    utils.print_list(quota_list, columns)"},{"line_number":4406,"context_line":""},{"line_number":4407,"context_line":"def get_keystone_client(os_username\u003dNone,"},{"line_number":4408,"context_line":"                        os_password\u003dNone,"},{"line_number":4409,"context_line":"                        os_tenant_name\u003dNone,"},{"line_number":4410,"context_line":"                        os_tenant_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7af24918_0c5d95ec","line":4407,"updated":"2016-03-04 23:08:56.000000000","message":"PEP8 calls for each top-level construct to be preceded by 2 blank lines, not 1 as you have here.  Further, I would expect a docstring explaining the purpose of this function.","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":4425,"context_line":"        os_tenant_name\u003dos.environ.get(\u0027OS_TENANT_NAME\u0027),"},{"line_number":4426,"context_line":"        os_tenant_id\u003dos.environ.get(\u0027OS_TENANT_ID\u0027),"},{"line_number":4427,"context_line":"        os_auth_url\u003dos.environ.get(\u0027OS_AUTH_URL\u0027)"},{"line_number":4428,"context_line":"        #os_auth_url\u003dargs.os_auth_url"},{"line_number":4429,"context_line":"    )"},{"line_number":4430,"context_line":"    dict_1 \u003d client.tenants.list()"},{"line_number":4431,"context_line":"    for x in dict_1:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7af24918_8c71a57a","line":4428,"updated":"2016-03-04 23:08:56.000000000","message":"Please don\u0027t add commented-out code.","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":4440,"context_line":"        if tenant_id \u003d\u003d tId:"},{"line_number":4441,"context_line":"                flag \u003d 1"},{"line_number":4442,"context_line":"    if flag \u003d\u003d 0:"},{"line_number":4443,"context_line":"        msg \u003d \"\\\u0027%s\\\u0027 is not a valid tenant-id\" %tenant_id"},{"line_number":4444,"context_line":"        raise exceptions.CommandError(msg)"},{"line_number":4445,"context_line":"    "},{"line_number":4446,"context_line":"    updates \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"7af24918_0cd2b534","line":4443,"updated":"2016-03-04 23:08:56.000000000","message":"1. There\u0027s no need to escape the single-quotes—this is a double-quoted string.\n\n2. I would expect this to be appropriately internationalized.\n\n3. PEP8 requires a single space surrounding any binary operator, such as the \u0027%\u0027 operator you use here.","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1de4259d7fc8862cf98d68060b907cfa5aeeda45","unresolved":false,"context_lines":[{"line_number":4442,"context_line":"    if flag \u003d\u003d 0:"},{"line_number":4443,"context_line":"        msg \u003d \"\\\u0027%s\\\u0027 is not a valid tenant-id\" %tenant_id"},{"line_number":4444,"context_line":"        raise exceptions.CommandError(msg)"},{"line_number":4445,"context_line":"    "},{"line_number":4446,"context_line":"    updates \u003d {}"},{"line_number":4447,"context_line":"    for resource in _quota_resources:"},{"line_number":4448,"context_line":"        val \u003d getattr(args, resource, None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7af24918_acff41a9","line":4445,"updated":"2016-03-04 23:08:56.000000000","message":"Trailing whitespace on a line is also verboten as far as PEP8 is concerned.","commit_id":"64a947e7599419d94ba924ecb20186f011936c2f"}]}