)]}'
{"nova/network/neutronv2/api.py":[{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"251c93ac331157cb29e9e6fd20b342bdbec342ef","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":152,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":153,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":154,"context_line":"        admin\u003dadmin or (context.is_admin and not context.auth_token))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_426be6b6","line":154,"updated":"2017-03-13 15:05:19.000000000","message":"Looks ok, because of the same condition upper, but \ntwo questions:\n\n- Why could context.auth_token be false? I thought after successful auth it had to contain token, right?\n\n- You are turning off context.is_admin if not auth_token, does it affect exceptions only or some logic also?","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"6ad877a994e980e5d98f0485a91391b3988c298e","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":152,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":153,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":154,"context_line":"        admin\u003dadmin or (context.is_admin and not context.auth_token))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_6bc93a78","line":154,"in_reply_to":"1a1ced50_c61dfb67","updated":"2017-03-20 14:13:22.000000000","message":"Done","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"12f3a36b25839d0a6ef706a1f68dc508aa03aacd","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":152,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":153,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":154,"context_line":"        admin\u003dadmin or (context.is_admin and not context.auth_token))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_c61dfb67","line":154,"in_reply_to":"5a22e58a_2e4b262d","updated":"2017-03-17 15:45:12.000000000","message":"Can we put a comment in this code explaining the logic turducken we now have here?\n\nhttps://en.wikipedia.org/wiki/Turducken","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"4264fc544ba04f9400706abed48931e01d5473b0","unresolved":false,"context_lines":[{"line_number":151,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":152,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":153,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":154,"context_line":"        admin\u003dadmin or (context.is_admin and not context.auth_token))"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a22e58a_2e4b262d","line":154,"in_reply_to":"9a30ddce_426be6b6","updated":"2017-03-14 09:41:40.000000000","message":"This is done specifically, so that we raise the correct exception in ClientWrapper, when the token expires - i.e. Unauthorized at #107 and *not* NeutronAdminCredentialConfigurationInvalid at #115.\n\nCondition `(context.is_admin and not context.auth_token)` denotes a request to Neutron issued from *within* Nova using the \n*service credentials*  from nova.conf. \n\n`context.is_admin and context.auth_token`, on the other hand, would be a a request to nova-api performed by a user with admin role (e.g. ceilometer polling agent), that also needs to go to neutron-sever on behalf of that user, i.e. using the given `auth_token`.\n\nThe problem here was that we raised an incorrect exception in the case when a token nova-api was given expired right at the moment when it reached neutron-server: instead of propagating 401 back to the user, who did the request (in my case - ceilometer-polling-agent), we mistakenly treated that as a configuration error, as if the neutron credentials in nova.conf were incorrect (although we did not use them at all for this kind of request to neutron-server).","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e9f6dfcfae9759916f865a9eea362537f6a5f631","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    Wraps the callable methods, catches Unauthorized,Forbidden from Neutron and"},{"line_number":82,"context_line":"    convert it to a 401,403 for Nova clients."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :param base_client: an initialized neutron client instance"},{"line_number":85,"context_line":"    :param admin: True, if neutron service credentials from nova.conf are used."},{"line_number":86,"context_line":"                  False, if a request to neutron is performed on behalf of a"},{"line_number":87,"context_line":"                  user with admin role using the passed auth token - in this"},{"line_number":88,"context_line":"                  case 401 Unauthorized will be propagated back to the user"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    \"\"\""},{"line_number":91,"context_line":"    def __init__(self, base_client, admin):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_832e27a3","line":88,"range":{"start_line":84,"start_character":4,"end_line":88,"end_character":75},"updated":"2017-05-05 13:58:40.000000000","message":"This belongs in the docstring for __init__, not the class.","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"295434002ec9416a0759578a3058562f244092bf","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    Wraps the callable methods, catches Unauthorized,Forbidden from Neutron and"},{"line_number":82,"context_line":"    convert it to a 401,403 for Nova clients."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    :param base_client: an initialized neutron client instance"},{"line_number":85,"context_line":"    :param admin: True, if neutron service credentials from nova.conf are used."},{"line_number":86,"context_line":"                  False, if a request to neutron is performed on behalf of a"},{"line_number":87,"context_line":"                  user with admin role using the passed auth token - in this"},{"line_number":88,"context_line":"                  case 401 Unauthorized will be propagated back to the user"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    \"\"\""},{"line_number":91,"context_line":"    def __init__(self, base_client, admin):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_b463981a","line":88,"range":{"start_line":84,"start_character":4,"end_line":88,"end_character":75},"in_reply_to":"3f044301_832e27a3","updated":"2017-05-05 15:18:20.000000000","message":"Done","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8098e627f3f0674fca3d9b8aab17dc6bef475a41","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            try:"},{"line_number":106,"context_line":"                ret \u003d obj(*args, **kwargs)"},{"line_number":107,"context_line":"            except neutron_client_exc.Unauthorized:"},{"line_number":108,"context_line":"                if not self.admin:"},{"line_number":109,"context_line":"                    # Token is expired so Neutron is raising a"},{"line_number":110,"context_line":"                    # unauthorized exception, we should convert it to"},{"line_number":111,"context_line":"                    # raise a 401 to make client to handle a retry by"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_7ef3b09f","line":108,"range":{"start_line":108,"start_character":28,"end_line":108,"end_character":33},"updated":"2017-05-05 14:18:48.000000000","message":"Hmm, maybe is_token_from_config would be a better name for this?","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"295434002ec9416a0759578a3058562f244092bf","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            try:"},{"line_number":106,"context_line":"                ret \u003d obj(*args, **kwargs)"},{"line_number":107,"context_line":"            except neutron_client_exc.Unauthorized:"},{"line_number":108,"context_line":"                if not self.admin:"},{"line_number":109,"context_line":"                    # Token is expired so Neutron is raising a"},{"line_number":110,"context_line":"                    # unauthorized exception, we should convert it to"},{"line_number":111,"context_line":"                    # raise a 401 to make client to handle a retry by"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_d75e2611","line":108,"range":{"start_line":108,"start_character":28,"end_line":108,"end_character":33},"in_reply_to":"3f044301_7ef3b09f","updated":"2017-05-05 15:18:20.000000000","message":"Done","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8098e627f3f0674fca3d9b8aab17dc6bef475a41","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        auth_plugin \u003d _ADMIN_AUTH"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    elif context.auth_token:"},{"line_number":159,"context_line":"        auth_plugin \u003d service_auth.get_auth_plugin(context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    if not auth_plugin:"},{"line_number":162,"context_line":"        # We did not get a user token and we should not be using"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_deebe4ad","line":159,"updated":"2017-05-05 14:18:48.000000000","message":"By the way, if you enable the service token stuff, you can\u0027t ever hit the user token expiry problem you talk about.","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"295434002ec9416a0759578a3058562f244092bf","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        auth_plugin \u003d _ADMIN_AUTH"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"    elif context.auth_token:"},{"line_number":159,"context_line":"        auth_plugin \u003d service_auth.get_auth_plugin(context)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    if not auth_plugin:"},{"line_number":162,"context_line":"        # We did not get a user token and we should not be using"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_f76362db","line":159,"in_reply_to":"3f044301_deebe4ad","updated":"2017-05-05 15:18:20.000000000","message":"Yeah, good point. IIRC, I saw this problem on Mitaka, so this seemed to be an easier solution, but overall service token sounds like the best way to solve this in the long term perspective (maybe Ceilo already does that, I haven\u0027t checked master).","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8098e627f3f0674fca3d9b8aab17dc6bef475a41","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":169,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":170,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":171,"context_line":"        admin\u003duse_service_credentials)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_9e9f9c4f","line":171,"updated":"2017-05-05 14:18:48.000000000","message":"So this is good.\n\nBut maybe we should rename the admin param to something like suppress_401_errors or is_token_from_config?","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"295434002ec9416a0759578a3058562f244092bf","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":169,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":170,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":171,"context_line":"        admin\u003duse_service_credentials)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_7414f04e","line":171,"in_reply_to":"3f044301_94d53c46","updated":"2017-05-05 15:18:20.000000000","message":"Done","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"295434002ec9416a0759578a3058562f244092bf","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":169,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":170,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":171,"context_line":"        admin\u003duse_service_credentials)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_941d3c75","line":171,"in_reply_to":"3f044301_9e9f9c4f","updated":"2017-05-05 15:18:20.000000000","message":"Done","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8120797f12e08b82a7c3e5da986b4a91a42fd409","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                         auth\u003dauth_plugin,"},{"line_number":169,"context_line":"                         endpoint_override\u003dCONF.neutron.url,"},{"line_number":170,"context_line":"                         region_name\u003dCONF.neutron.region_name),"},{"line_number":171,"context_line":"        admin\u003duse_service_credentials)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"def _is_not_duplicate(item, items, items_list_name, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f044301_94d53c46","line":171,"in_reply_to":"3f044301_9e9f9c4f","updated":"2017-05-05 14:39:37.000000000","message":"sdague suggested calling it \"auth_from_config\" in the original change that introduced this code:\n\nhttps://review.openstack.org/#/c/312014/14/nova/network/neutronv2/api.py","commit_id":"2abce5dc004340c3d6d8203ca04e243dd5a19ce7"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e427a9cb71bfd05c592c5a4c9039d47a480b90cf","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        \"\"\"Initialize a ClientWrapper instance."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        :param base_client: an initialized neutron client instance"},{"line_number":89,"context_line":"        :param auth_from_config: True, if neutron service credentials from"},{"line_number":90,"context_line":"           nova.conf are used.  False, if a request to neutron is performed"},{"line_number":91,"context_line":"           on behalf of a user with admin role using the passed auth token -"},{"line_number":92,"context_line":"           in this case 401 Unauthorized will be propagated back to the user"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_e9ea2b73","line":89,"range":{"start_line":89,"start_character":37,"end_line":89,"end_character":38},"updated":"2017-06-23 17:43:28.000000000","message":"grammatically-incorrect comma usage. Just remove it.","commit_id":"5a30322f78a9c6d02d9eb312628165c82bb64ac7"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e427a9cb71bfd05c592c5a4c9039d47a480b90cf","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        :param base_client: an initialized neutron client instance"},{"line_number":89,"context_line":"        :param auth_from_config: True, if neutron service credentials from"},{"line_number":90,"context_line":"           nova.conf are used.  False, if a request to neutron is performed"},{"line_number":91,"context_line":"           on behalf of a user with admin role using the passed auth token -"},{"line_number":92,"context_line":"           in this case 401 Unauthorized will be propagated back to the user"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_0999e71d","line":90,"range":{"start_line":90,"start_character":29,"end_line":90,"end_character":31},"updated":"2017-06-23 17:43:28.000000000","message":"semicolon and a single space would be better here.","commit_id":"5a30322f78a9c6d02d9eb312628165c82bb64ac7"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e427a9cb71bfd05c592c5a4c9039d47a480b90cf","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        :param base_client: an initialized neutron client instance"},{"line_number":89,"context_line":"        :param auth_from_config: True, if neutron service credentials from"},{"line_number":90,"context_line":"           nova.conf are used.  False, if a request to neutron is performed"},{"line_number":91,"context_line":"           on behalf of a user with admin role using the passed auth token -"},{"line_number":92,"context_line":"           in this case 401 Unauthorized will be propagated back to the user"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_49e0ff96","line":92,"range":{"start_line":90,"start_character":32,"end_line":92,"end_character":76},"updated":"2017-06-23 17:43:28.000000000","message":"this wording around False isn\u0027t the whole story... just say \"False otherwise\", which is both sufficient and more correct.","commit_id":"5a30322f78a9c6d02d9eb312628165c82bb64ac7"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e427a9cb71bfd05c592c5a4c9039d47a480b90cf","unresolved":false,"context_lines":[{"line_number":90,"context_line":"           nova.conf are used.  False, if a request to neutron is performed"},{"line_number":91,"context_line":"           on behalf of a user with admin role using the passed auth token -"},{"line_number":92,"context_line":"           in this case 401 Unauthorized will be propagated back to the user"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # Expose all attributes from the base_client instance"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_69505b5e","line":93,"updated":"2017-06-23 17:43:28.000000000","message":"remove blank line","commit_id":"5a30322f78a9c6d02d9eb312628165c82bb64ac7"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e427a9cb71bfd05c592c5a4c9039d47a480b90cf","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    #  1) get_client() was explicitly called with admin\u003dTrue"},{"line_number":150,"context_line":"    #  2) admin context without an auth token is passed (this can happen e.g."},{"line_number":151,"context_line":"    #     when nova-metadata-api goes to neutron for info on networks)"},{"line_number":152,"context_line":"    # In the long term perspective we should probably remove the latter and"},{"line_number":153,"context_line":"    # require admin\u003dTrue to be passed explicitly."},{"line_number":154,"context_line":"    admin_context_without_token \u003d context.is_admin and not context.auth_token"},{"line_number":155,"context_line":"    auth_from_config \u003d admin or admin_context_without_token"}],"source_content_type":"text/x-python","patch_set":5,"id":"5f201791_09c207ea","line":152,"range":{"start_line":152,"start_character":6,"end_line":152,"end_character":34},"updated":"2017-06-23 17:43:28.000000000","message":"s/In the long term perspective/Long term/","commit_id":"5a30322f78a9c6d02d9eb312628165c82bb64ac7"},{"author":{"_account_id":19156,"name":"Leehom Li","email":"feli5_li@163.com","username":"leehom"},"change_message_id":"2eea90c9af6165fed00781ff7720e052ea641997","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    #  1) get_client() was explicitly called with admin\u003dTrue"},{"line_number":147,"context_line":"    #  2) admin context without an auth token is passed (this can happen e.g."},{"line_number":148,"context_line":"    #     when nova-metadata-api goes to neutron for info on networks)"},{"line_number":149,"context_line":"    # Long term perspective we should probably remove the latter and"},{"line_number":150,"context_line":"    # require admin\u003dTrue to be passed explicitly."},{"line_number":151,"context_line":"    admin_context_without_token \u003d context.is_admin and not context.auth_token"},{"line_number":152,"context_line":"    auth_from_config \u003d admin or admin_context_without_token"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_331f595c","line":149,"updated":"2017-09-07 09:28:33.000000000","message":"Maybe we cannot remove the later one, as right now we are leveraging the later one to handle case when \"token has expired\".","commit_id":"4e5986712620aa94e3b7b4ee85d8c6c0296bf82c"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"a912eb6da461a25645899548811b30b131d4c641","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    #  1) get_client() was explicitly called with admin\u003dTrue"},{"line_number":147,"context_line":"    #  2) admin context without an auth token is passed (this can happen e.g."},{"line_number":148,"context_line":"    #     when nova-metadata-api goes to neutron for info on networks)"},{"line_number":149,"context_line":"    # Long term perspective we should probably remove the latter and"},{"line_number":150,"context_line":"    # require admin\u003dTrue to be passed explicitly."},{"line_number":151,"context_line":"    admin_context_without_token \u003d context.is_admin and not context.auth_token"},{"line_number":152,"context_line":"    auth_from_config \u003d admin or admin_context_without_token"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_bd6af216","line":149,"in_reply_to":"7f287b81_331f595c","updated":"2017-09-07 13:45:27.000000000","message":"if auth was done with user/password, then the session should automatically get a new token when the current one expires. Only if auth was done with a token should we need to call get_client again when the token expires, and we can only really do that if we want an admin token because we don\u0027t have user/pwd for anything else, so we should explicitly pass admin\u003dTrue there. So I think it\u0027s right that long-term we would probably remove (2).","commit_id":"4e5986712620aa94e3b7b4ee85d8c6c0296bf82c"},{"author":{"_account_id":19156,"name":"Leehom Li","email":"feli5_li@163.com","username":"leehom"},"change_message_id":"7bfb40312573ca5ec84392b6ee6a789e79ffdc41","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    #  1) get_client() was explicitly called with admin\u003dTrue"},{"line_number":147,"context_line":"    #  2) admin context without an auth token is passed (this can happen e.g."},{"line_number":148,"context_line":"    #     when nova-metadata-api goes to neutron for info on networks)"},{"line_number":149,"context_line":"    # Long term perspective we should probably remove the latter and"},{"line_number":150,"context_line":"    # require admin\u003dTrue to be passed explicitly."},{"line_number":151,"context_line":"    admin_context_without_token \u003d context.is_admin and not context.auth_token"},{"line_number":152,"context_line":"    auth_from_config \u003d admin or admin_context_without_token"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_f3ff78fe","line":149,"in_reply_to":"7f287b81_bd6af216","updated":"2017-09-08 02:04:31.000000000","message":"The problem is sometimes we do not explicitly pass admin\u003dTrue when call get_client(In my environment _get_available_networks). When token expired, as context.auth_token still contains token id, if we remove(2), these API calls will fail with exception NeutronAdminCredentialConfigurationInvalid.","commit_id":"4e5986712620aa94e3b7b4ee85d8c6c0296bf82c"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"ed153531984535fcbbb6bbf7bbaa464ba665e7e1","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    #  1) get_client() was explicitly called with admin\u003dTrue"},{"line_number":147,"context_line":"    #  2) admin context without an auth token is passed (this can happen e.g."},{"line_number":148,"context_line":"    #     when nova-metadata-api goes to neutron for info on networks)"},{"line_number":149,"context_line":"    # Long term perspective we should probably remove the latter and"},{"line_number":150,"context_line":"    # require admin\u003dTrue to be passed explicitly."},{"line_number":151,"context_line":"    admin_context_without_token \u003d context.is_admin and not context.auth_token"},{"line_number":152,"context_line":"    auth_from_config \u003d admin or admin_context_without_token"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_f36098a7","line":149,"in_reply_to":"7f287b81_f3ff78fe","updated":"2017-09-08 02:28:16.000000000","message":"it wouldn\u0027t be removed without updating places like that to explicitly pass admin\u003dTrue.","commit_id":"4e5986712620aa94e3b7b4ee85d8c6c0296bf82c"}],"nova/tests/unit/network/test_neutronv2.py":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"e669211b16b0031dcae38b1e240f56ebbad7b9b8","unresolved":false,"context_lines":[{"line_number":5594,"context_line":"    def test_get_client_for_admin_context_with_id(self):"},{"line_number":5595,"context_line":"        self._test_get_client_for_admin(use_id\u003dTrue, admin_context\u003dTrue)"},{"line_number":5596,"context_line":""},{"line_number":5597,"context_line":"    @mock.patch.object(client.Client, \"list_networks\","},{"line_number":5598,"context_line":"                       side_effect\u003dexceptions.Unauthorized())"},{"line_number":5599,"context_line":"    def test_Unauthorized_admin(self, mock_list_networks):"},{"line_number":5600,"context_line":"        self.flags(url\u003d\u0027http://anyhost/\u0027, group\u003d\u0027neutron\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_9002593d","line":5597,"updated":"2017-03-17 15:08:08.000000000","message":"is there a reason this has to be here and not up with the other test? Would be nice to keep them together if we can.","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"6ad877a994e980e5d98f0485a91391b3988c298e","unresolved":false,"context_lines":[{"line_number":5594,"context_line":"    def test_get_client_for_admin_context_with_id(self):"},{"line_number":5595,"context_line":"        self._test_get_client_for_admin(use_id\u003dTrue, admin_context\u003dTrue)"},{"line_number":5596,"context_line":""},{"line_number":5597,"context_line":"    @mock.patch.object(client.Client, \"list_networks\","},{"line_number":5598,"context_line":"                       side_effect\u003dexceptions.Unauthorized())"},{"line_number":5599,"context_line":"    def test_Unauthorized_admin(self, mock_list_networks):"},{"line_number":5600,"context_line":"        self.flags(url\u003d\u0027http://anyhost/\u0027, group\u003d\u0027neutron\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a1ced50_cbd6ae26","line":5597,"in_reply_to":"1a1ced50_9002593d","updated":"2017-03-20 14:13:22.000000000","message":"Unfortunately, I do not remember the details right now, but when I first proposed this patch I was struggling to make unit tests work due to a problem with the config fixture. Curiously, it works for me now (at least all the test cases in this module pass), if I copy lines #5517-#5519 to the test case...\n\nI don\u0027t have a strong preference. If there are no problems with the config, I can update the change to move this test case back.","commit_id":"46f3e487630167dbcd3f8eb6f441b01f6f40ceb2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0b477740163a24423f1c2ab886fd6ff3d68f6acc","unresolved":false,"context_lines":[{"line_number":5814,"context_line":"        client \u003d neutronapi.get_client(my_context, admin\u003dTrue)"},{"line_number":5815,"context_line":"        self.assertRaises("},{"line_number":5816,"context_line":"            exception.NeutronAdminCredentialConfigurationInvalid,"},{"line_number":5817,"context_line":"            client.list_networks)"},{"line_number":5818,"context_line":""},{"line_number":5819,"context_line":""},{"line_number":5820,"context_line":"class TestNeutronPortSecurity(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f2577a7_cee696e9","line":5817,"updated":"2017-09-11 20:45:39.000000000","message":"Any reason why you\u0027re locating that test into a very specific separate test class while you could just have it done in the same class than the other test you fixed ? \nI\u0027m not particularly wanting to nitpick on that, I just want to make sure the coverage we make for the same proxy wrapper is quite identical. \n\nFor example, why are you needing to define the flags for that test?","commit_id":"4e5986712620aa94e3b7b4ee85d8c6c0296bf82c"}]}