)]}'
{"tempest/common/fixed_network.py":[{"author":{"_account_id":7020,"name":"Joe Hakim Rahme","email":"jhakimra@redhat.com","username":"rahmu"},"change_message_id":"3376dc6c51bac8b1a74f910a37538054e2406b3e","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                else:"},{"line_number":69,"context_line":"                    msg \u003d \"Configured fixed_network_name not found\""},{"line_number":70,"context_line":"                    raise exceptions.InvalidConfiguration(msg)"},{"line_number":71,"context_line":"                # To be consistent with network isolation, add name is only"},{"line_number":72,"context_line":"                # label is available"},{"line_number":73,"context_line":"                name \u003d network.get(\u0027name\u0027, network.get(\u0027label\u0027))"},{"line_number":74,"context_line":"                if name:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_a1c5ff8e","line":71,"updated":"2015-04-14 14:21:31.000000000","message":"s/is/if/\n\n?","commit_id":"14117175d2a0a9068d4f99f91376a509d2756a59"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"0514c631c586cce42a5fedec239298225436e6b0","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                else:"},{"line_number":69,"context_line":"                    msg \u003d \"Configured fixed_network_name not found\""},{"line_number":70,"context_line":"                    raise exceptions.InvalidConfiguration(msg)"},{"line_number":71,"context_line":"                # To be consistent with network isolation, add name is only"},{"line_number":72,"context_line":"                # label is available"},{"line_number":73,"context_line":"                name \u003d network.get(\u0027name\u0027, network.get(\u0027label\u0027))"},{"line_number":74,"context_line":"                if name:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_1fef6e88","line":71,"in_reply_to":"7aaa499b_a1c5ff8e","updated":"2015-04-14 15:02:14.000000000","message":"Yeah this comment is worded weirdly, we can fix this in a separate patch","commit_id":"14117175d2a0a9068d4f99f91376a509d2756a59"},{"author":{"_account_id":7020,"name":"Joe Hakim Rahme","email":"jhakimra@redhat.com","username":"rahmu"},"change_message_id":"3376dc6c51bac8b1a74f910a37538054e2406b3e","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    if caller:"},{"line_number":92,"context_line":"        LOG.info(\u0027(%s) %s\u0027 % (caller, msg))"},{"line_number":93,"context_line":"    else:"},{"line_number":94,"context_line":"        LOG.info(msg)"},{"line_number":95,"context_line":"    return network"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_41ed1bee","line":94,"updated":"2015-04-14 14:21:31.000000000","message":"Do you think that these lines (L91-94) can be defined as a reusable function, maybe in tempest_lib.common.utils?","commit_id":"14117175d2a0a9068d4f99f91376a509d2756a59"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"0514c631c586cce42a5fedec239298225436e6b0","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    if caller:"},{"line_number":92,"context_line":"        LOG.info(\u0027(%s) %s\u0027 % (caller, msg))"},{"line_number":93,"context_line":"    else:"},{"line_number":94,"context_line":"        LOG.info(msg)"},{"line_number":95,"context_line":"    return network"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_df5a2683","line":94,"in_reply_to":"7aaa499b_41ed1bee","updated":"2015-04-14 15:02:14.000000000","message":"Yeah that\u0027s a good idea, can you push a patch for that.","commit_id":"14117175d2a0a9068d4f99f91376a509d2756a59"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"175f5a2facf6b42065b81b8c872881d8ac197e98","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    else:"},{"line_number":50,"context_line":"        if fixed_network_name:"},{"line_number":51,"context_line":"            msg \u003d (\u0027No valid network provided or created, defaulting to \u0027"},{"line_number":52,"context_line":"                   \u0027fixed_network_name\u0027)"},{"line_number":53,"context_line":"            if caller:"},{"line_number":54,"context_line":"                LOG.debug(\u0027(%s) %s\u0027 % (caller, msg))"},{"line_number":55,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_cf0498c1","line":52,"updated":"2015-04-15 08:05:43.000000000","message":"How about appending caller here if that exist instead of checking many places and then log msg directly.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"9b44d4012ab5a3e111c9d284dfb87ea8d68917fc","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    else:"},{"line_number":50,"context_line":"        if fixed_network_name:"},{"line_number":51,"context_line":"            msg \u003d (\u0027No valid network provided or created, defaulting to \u0027"},{"line_number":52,"context_line":"                   \u0027fixed_network_name\u0027)"},{"line_number":53,"context_line":"            if caller:"},{"line_number":54,"context_line":"                LOG.debug(\u0027(%s) %s\u0027 % (caller, msg))"},{"line_number":55,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aa745d1_294da872","line":52,"in_reply_to":"7aaa499b_423f3287","updated":"2015-04-15 23:50:06.000000000","message":"Ah I see. make sense to me too.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"390d373b5d1048490053461a4497a0f19349eb76","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    else:"},{"line_number":50,"context_line":"        if fixed_network_name:"},{"line_number":51,"context_line":"            msg \u003d (\u0027No valid network provided or created, defaulting to \u0027"},{"line_number":52,"context_line":"                   \u0027fixed_network_name\u0027)"},{"line_number":53,"context_line":"            if caller:"},{"line_number":54,"context_line":"                LOG.debug(\u0027(%s) %s\u0027 % (caller, msg))"},{"line_number":55,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_423f3287","line":52,"in_reply_to":"7aaa499b_cf0498c1","updated":"2015-04-15 13:50:46.000000000","message":"So it\u0027s a log formatting issue, having the caller at the beginning of the log message is what\u0027s expected, so appending it doesn\u0027t really help. Additionally, I thought it would be better to not print \u003cNone\u003e or an empty parenthesis in the log which is why I did all the checks.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":7020,"name":"Joe Hakim Rahme","email":"jhakimra@redhat.com","username":"rahmu"},"change_message_id":"195e34f43cd12e6cb9ba755e04102c78450d6f68","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                else:"},{"line_number":69,"context_line":"                    msg \u003d \"Configured fixed_network_name not found\""},{"line_number":70,"context_line":"                    if caller:"},{"line_number":71,"context_line":"                        msg \u003d \u0027(%s) %s\u0027 % (caller, msg)"},{"line_number":72,"context_line":"                    raise exceptions.InvalidConfiguration(msg)"},{"line_number":73,"context_line":"                # To be consistent with network isolation, add name is only"},{"line_number":74,"context_line":"                # label is available"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_bd497b62","line":71,"updated":"2015-04-15 14:08:45.000000000","message":"Here\u0027s a slightest of nitpicks, but you\u0027re being inconsistent with the rest of the changes in this patch. To make it consistent, it should be:\n\n    if caller:\n        raise exceptions.InvalidConfiguration(\u0027(%s) %s\u0027 % (caller, msg))\n    else:\n        raise exceptions.InvalidConfiguration(msg)\n\nor change the call for logging:\n\n\n    if caller:\n        msg \u003d \u0027(%s) %s\u0027 % (caller, msg)\n    LOG.info(msg)","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"badb908f8e91094f3a2058f71995623eeacad45c","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                if name:"},{"line_number":77,"context_line":"                    network[\u0027name\u0027] \u003d name"},{"line_number":78,"context_line":"                else:"},{"line_number":79,"context_line":"                    raise lib_exc.NotFound()"},{"line_number":80,"context_line":"            except lib_exc.NotFound:"},{"line_number":81,"context_line":"                # In case of nova network, if the fixed_network_name is not"},{"line_number":82,"context_line":"                # owned by the tenant, and the network client is not an admin"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aa745d1_f9489c89","line":79,"updated":"2015-04-16 13:39:47.000000000","message":"Using the NotFound RestClientException for indicating anything else than 404 is confusing.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"27126231a2e0500a134f07a9375fc3de296d3b4f","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                if name:"},{"line_number":77,"context_line":"                    network[\u0027name\u0027] \u003d name"},{"line_number":78,"context_line":"                else:"},{"line_number":79,"context_line":"                    raise lib_exc.NotFound()"},{"line_number":80,"context_line":"            except lib_exc.NotFound:"},{"line_number":81,"context_line":"                # In case of nova network, if the fixed_network_name is not"},{"line_number":82,"context_line":"                # owned by the tenant, and the network client is not an admin"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aa745d1_200ebb34","line":79,"in_reply_to":"5aa745d1_07c2be85","updated":"2015-04-16 16:38:58.000000000","message":"I agree with @afazekas on this one, NotFound makes people assume it was a 404. Much better to not overload this.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"52fb50ebab8be755ecb9aeb26012730d368e5c34","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                if name:"},{"line_number":77,"context_line":"                    network[\u0027name\u0027] \u003d name"},{"line_number":78,"context_line":"                else:"},{"line_number":79,"context_line":"                    raise lib_exc.NotFound()"},{"line_number":80,"context_line":"            except lib_exc.NotFound:"},{"line_number":81,"context_line":"                # In case of nova network, if the fixed_network_name is not"},{"line_number":82,"context_line":"                # owned by the tenant, and the network client is not an admin"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aa745d1_cd587059","line":79,"in_reply_to":"5aa745d1_07c2be85","updated":"2015-04-16 16:37:42.000000000","message":"L65, OK :)","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"2bb4b3ec9109c777b0518d2365c820b2d54afcdd","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                if name:"},{"line_number":77,"context_line":"                    network[\u0027name\u0027] \u003d name"},{"line_number":78,"context_line":"                else:"},{"line_number":79,"context_line":"                    raise lib_exc.NotFound()"},{"line_number":80,"context_line":"            except lib_exc.NotFound:"},{"line_number":81,"context_line":"                # In case of nova network, if the fixed_network_name is not"},{"line_number":82,"context_line":"                # owned by the tenant, and the network client is not an admin"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aa745d1_07c2be85","line":79,"in_reply_to":"5aa745d1_f9489c89","updated":"2015-04-16 14:19:55.000000000","message":"Is it really? In this case we\u0027re saying if there is no name returned from the list treat it like a 404, because the error path is the same. That doesn\u0027t seem confusing to me at all, especially given the next line here is literally \u0027except lib_exc.NotFound\u0027 Also this pattern was already used above, see L65.\n\nI mean sure we could raise a second exception type here and above and then change the except to look for both. If you feel strongly enough about this can you push a change at the top of this branch to make the logical flow look like what you think is better.","commit_id":"a47c3edc2d22e693a7c3d214658021c869d3bfae"}]}