)]}'
{"cinder/api/v1/volumes.py":[{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"d007713f4394019fabe1d2b4133845b67d56a01c","unresolved":false,"context_lines":[{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError):"},{"line_number":305,"context_line":"            # May be an image name"},{"line_number":306,"context_line":"            pass"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":309,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8c0949_aefd813d","line":306,"updated":"2014-11-05 10:44:03.000000000","message":"Atleast debug logs","commit_id":"6e9d12abb4f0dcebd3a049184dad65c19281d949"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"19b3d245ecfa1c2df7e3472c868cd5c530d122bc","unresolved":false,"context_lines":[{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError):"},{"line_number":305,"context_line":"            # May be an image name"},{"line_number":306,"context_line":"            pass"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":309,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8c0949_a93c9bc0","line":306,"in_reply_to":"7a8c0949_aefd813d","updated":"2014-11-05 10:54:56.000000000","message":"Thanks Navneet. I added original exception handling back in since this would still be a problem even if image name is provided.","commit_id":"6e9d12abb4f0dcebd3a049184dad65c19281d949"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"384009a4409b972207e1d82ffc3271bf2ee546a4","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        image_uuid \u003d None"},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError):"},{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_fbe58aa8","line":304,"updated":"2014-11-05 12:25:50.000000000","message":"See comment in cinder/api/v2/volumes.py","commit_id":"f0b44734f3451f32f81e696350f0316e18c86ee7"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c1c575742658cd27b2283f4083bb3fb30f2c3fcf","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        image_uuid \u003d None"},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError):"},{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_1e47a44c","line":304,"in_reply_to":"7a8c0949_fbe58aa8","updated":"2014-11-05 12:43:28.000000000","message":"Done","commit_id":"f0b44734f3451f32f81e696350f0316e18c86ee7"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"690974d098704aa79b30d728a8de2fb4690c40fd","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        image_uuid \u003d None"},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError, KeyError):"},{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_aae1d89b","line":304,"updated":"2014-11-06 13:35:24.000000000","message":"What is KeyError for?  I can\u0027t figure it out and your unit tests don\u0027t cover the case?","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"1fa17daf8311b3b6145d94fe947835ded3be82de","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        image_uuid \u003d None"},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError, KeyError):"},{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_4a0dd43d","line":304,"in_reply_to":"7a8c0949_aae1d89b","updated":"2014-11-06 13:45:34.000000000","message":"See comments from v2 source.","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":4355,"name":"Avishay Traeger","email":"avishay@stratoscale.com","username":"avishay-il"},"change_message_id":"690974d098704aa79b30d728a8de2fb4690c40fd","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":309,"context_line":"            image_service \u003d glance.get_default_image_service()"},{"line_number":310,"context_line":"            params \u003d {\u0027filters\u0027: {\u0027name\u0027: image_ref}}"},{"line_number":311,"context_line":"            images \u003d list(image_service.detail(context, **params))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_ca9624f0","line":308,"updated":"2014-11-06 13:35:24.000000000","message":"For the first condition, when would \u0027not image_uuid\u0027 evaluate to True?  For the second, what happens if the name of my image looks like a UUID?","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"1fa17daf8311b3b6145d94fe947835ded3be82de","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":309,"context_line":"            image_service \u003d glance.get_default_image_service()"},{"line_number":310,"context_line":"            params \u003d {\u0027filters\u0027: {\u0027name\u0027: image_ref}}"},{"line_number":311,"context_line":"            images \u003d list(image_service.detail(context, **params))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_a563c9da","line":308,"in_reply_to":"7a8c0949_ca9624f0","updated":"2014-11-06 13:45:34.000000000","message":"The first condition is both to protect from None being passed in somehow and to handle an empty string. No need to look for a matching image name if it is empty.\n\nThe second one is interesting. That is a valid concern, but I couldn\u0027t figure out a better way. is_uuid_like attempts to actually make it a UUID, so I think it would need to be named as a valid UUID format. If an end user is naming their images with UUIDs then that makes differentiating between IDs and names very problematic. I\u0027m open to suggestions if anyone has any recommendations.","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"9e04a54e306dcd1c5db3d3d93146a0e6d82eb548","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        image_uuid \u003d None"},{"line_number":302,"context_line":"        try:"},{"line_number":303,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":304,"context_line":"        except (TypeError, AttributeError, KeyError, IndexError):"},{"line_number":305,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":306,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":307,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a8c0949_653ba1d1","line":304,"updated":"2014-11-06 13:41:40.000000000","message":"Sean, I think you will need to remove KeyError and TypeError here or explain the situation when you expect them. Same for v1.","commit_id":"93d02683c04bdb751e5d92106509b4fee45da050"}],"cinder/api/v2/volumes.py":[{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"d007713f4394019fabe1d2b4133845b67d56a01c","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError):"},{"line_number":260,"context_line":"            # May be an image name"},{"line_number":261,"context_line":"            pass"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":264,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8c0949_6eec7990","line":261,"updated":"2014-11-05 10:44:03.000000000","message":"Atleast debug logs.","commit_id":"6e9d12abb4f0dcebd3a049184dad65c19281d949"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"19b3d245ecfa1c2df7e3472c868cd5c530d122bc","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError):"},{"line_number":260,"context_line":"            # May be an image name"},{"line_number":261,"context_line":"            pass"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":264,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a8c0949_c94ba757","line":261,"in_reply_to":"7a8c0949_6eec7990","updated":"2014-11-05 10:54:56.000000000","message":"Done","commit_id":"6e9d12abb4f0dcebd3a049184dad65c19281d949"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"384009a4409b972207e1d82ffc3271bf2ee546a4","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError):"},{"line_number":260,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":261,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":264,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_7b5b5a87","line":261,"updated":"2014-11-05 12:25:50.000000000","message":"I am little worried about the exceptions you are catching, popping a list may not give you TypeError but IndexError.","commit_id":"f0b44734f3451f32f81e696350f0316e18c86ee7"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"c1c575742658cd27b2283f4083bb3fb30f2c3fcf","unresolved":false,"context_lines":[{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError):"},{"line_number":260,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":261,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        if not image_uuid or not uuidutils.is_uuid_like(image_uuid):"},{"line_number":264,"context_line":"            image_service \u003d glance.get_default_image_service()"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_fe62d8d6","line":261,"in_reply_to":"7a8c0949_7b5b5a87","updated":"2014-11-05 12:43:28.000000000","message":"Thanks, I had just put back what was previously there. But you are right, if it is empty it will not be one of these.\n\nAccording to the docs, this could actually be a KeyError if empty:\n\nhttps://docs.python.org/2/library/stdtypes.html","commit_id":"f0b44734f3451f32f81e696350f0316e18c86ee7"},{"author":{"_account_id":6094,"name":"Navneet Singh","email":"navsinghnitd@gmail.com","username":"singn"},"change_message_id":"ab67530c44632b1fd957107f0aeba7b6c037e78e","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        image_uuid \u003d None"},{"line_number":257,"context_line":"        try:"},{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError, KeyError):"},{"line_number":260,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":261,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_8707b51b","line":259,"updated":"2014-11-06 13:02:54.000000000","message":"Split is a string function which gives back list. List.pop will give you IndexError not KeyError. When is TypeError expected?\n\nPlease also see in api/v1","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"1fa17daf8311b3b6145d94fe947835ded3be82de","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        image_uuid \u003d None"},{"line_number":257,"context_line":"        try:"},{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError, KeyError):"},{"line_number":260,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":261,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_2a408855","line":259,"in_reply_to":"7a8c0949_6a3610e9","updated":"2014-11-06 13:45:34.000000000","message":"Odd that the API reference I linked to only mentions KeyError on this. I\u0027ve kept that and added IndexError to the list.\n\nTypeError appears to be included based on the description here for AttributeError:\n\nhttps://docs.python.org/2/library/exceptions.html\n\nI am just guessing. This is the original code and I did nothing other than remove it and add it back in.","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"7d4e966e1c14e181242916c5c564a40394f343af","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        image_uuid \u003d None"},{"line_number":257,"context_line":"        try:"},{"line_number":258,"context_line":"            image_uuid \u003d image_ref.split(\u0027/\u0027).pop()"},{"line_number":259,"context_line":"        except (TypeError, AttributeError, KeyError):"},{"line_number":260,"context_line":"            msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":261,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7a8c0949_6a3610e9","line":259,"in_reply_to":"7a8c0949_8707b51b","updated":"2014-11-06 13:07:15.000000000","message":"agree with Navneet Singh, you should expect IndexError","commit_id":"db438c02cfa1c78f7bda0864185cd77273ce75d8"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"6c7b5444c4c28de90f232193ebbfffa7911dc79b","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                msg \u003d _(\"Multiple matches found for \u0027%s\u0027, use an ID to be more\""},{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_7a1624f3","line":284,"updated":"2014-11-19 15:43:27.000000000","message":"Why are we doing \u0027for\u0027 here?  If we are using the name shouldn\u0027t there be only one value and you do a check to make sure there is only one on the line above.  This seems confusing.","commit_id":"899af75e50c689076fb51d6480f703b16ac5926c"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"9176d2a97132bc0394017c1c2865bb3c4a863f95","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                msg \u003d _(\"Multiple matches found for \u0027%s\u0027, use an ID to be more\""},{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"}],"source_content_type":"text/x-python","patch_set":10,"id":"5a890539_ddaab284","line":284,"in_reply_to":"5a890539_7a1624f3","updated":"2014-11-19 16:09:12.000000000","message":"Sorry Jay, I didn\u0027t see this before my last commit.\n\nThe reason I did a for loop was a way to handle an empty list being returned. If we have one then it will return that one entry. If it is empty it will pass over and be handled below. It seemed more concise than adding another if statement to test for len()\u003d\u003d0, then returning images[0]. If you feel strongly about that I can certainly change the way it is handled. Just let me know.","commit_id":"899af75e50c689076fb51d6480f703b16ac5926c"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7651dd4187d7434c68e45ab2432edafb8e8b7583","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            if len(images) \u003e 1:"},{"line_number":281,"context_line":"                msg \u003d _(\"Multiple matches found for \u0027%s\u0027, use an ID to be more\""},{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_5c0856d9","line":283,"updated":"2014-11-20 17:09:25.000000000","message":"This is a well-formed request, so something like HTTPConflict is probably more appropriate to return in this circumstance.","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"efe8d0499f5f6f2e79e26fd0b8ee89cc900f3356","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            if len(images) \u003e 1:"},{"line_number":281,"context_line":"                msg \u003d _(\"Multiple matches found for \u0027%s\u0027, use an ID to be more\""},{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPBadRequest(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_3f4d3c73","line":283,"in_reply_to":"5a890539_5c0856d9","updated":"2014-11-20 17:39:31.000000000","message":"Done","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":11880,"name":"Gloria Gu","email":"gloriafanggu@gmail.com","username":"gugl"},"change_message_id":"db50e185ee7200c4225f45b53b4b608f1fccbf52","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":291,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_b3ac2e8e","line":288,"updated":"2014-11-19 22:35:00.000000000","message":"I checked the glance code for detail, it could come back with an empty list which could be caused by an authentication or authorization issue, it would be nice to spill out the error, instead of just fall into invalid imageref.","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"efe8d0499f5f6f2e79e26fd0b8ee89cc900f3356","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":291,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_3fa1bca9","line":288,"in_reply_to":"5a890539_b3ac2e8e","updated":"2014-11-20 17:39:31.000000000","message":"Unfortunately it appears the glance code hides some of that detail a little so we can\u0027t really tell if there were no matches, there was an auth problem, or the image was not public. I\u0027ll update the error per your next comment. Let me know if that\u0027s OK to address this concern.","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":11880,"name":"Gloria Gu","email":"gloriafanggu@gmail.com","username":"gugl"},"change_message_id":"db50e185ee7200c4225f45b53b4b608f1fccbf52","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":291,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_c592d0f5","line":290,"updated":"2014-11-19 22:35:00.000000000","message":"It would be better to have some specifics as part of the error message instead of such a generic one. This is more for client call later.","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"efe8d0499f5f6f2e79e26fd0b8ee89cc900f3356","unresolved":false,"context_lines":[{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid imageRef provided.\")"},{"line_number":291,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":11,"id":"5a890539_7fab4489","line":290,"in_reply_to":"5a890539_c592d0f5","updated":"2014-11-20 17:39:31.000000000","message":"Done","commit_id":"fbad909ddfa6a49a09591d2a8ee6817a9ae8fb16"},{"author":{"_account_id":5538,"name":"Rushi Agrawal","email":"rushi.openstack@gmail.com","username":"rushiagr"},"change_message_id":"ba40026fbcb8648b6949a4c6548fdc4ac6f1fd16","unresolved":false,"context_lines":[{"line_number":269,"context_line":"                image \u003d image_service.show(context, image_uuid)"},{"line_number":270,"context_line":"                if \u0027id\u0027 in image:"},{"line_number":271,"context_line":"                    return image[\u0027id\u0027]"},{"line_number":272,"context_line":"            except Exception:"},{"line_number":273,"context_line":"                # Pass and see if there is a matching image name"},{"line_number":274,"context_line":"                pass"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_8ca50d49","line":272,"updated":"2014-11-21 12:30:11.000000000","message":"I would say it would make more sense of a more specific exception is thrown here. We would not want to catch any exception at this point and just let it pass. Or am I missing something?","commit_id":"9cb61a870710974e70546c0122341ae698ef1f07"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"8a3903ca3b08a2d2eb4aea82680568df77fa0169","unresolved":false,"context_lines":[{"line_number":269,"context_line":"                image \u003d image_service.show(context, image_uuid)"},{"line_number":270,"context_line":"                if \u0027id\u0027 in image:"},{"line_number":271,"context_line":"                    return image[\u0027id\u0027]"},{"line_number":272,"context_line":"            except Exception:"},{"line_number":273,"context_line":"                # Pass and see if there is a matching image name"},{"line_number":274,"context_line":"                pass"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_d63b23e9","line":272,"in_reply_to":"5a890539_8ca50d49","updated":"2014-11-21 14:04:16.000000000","message":"We don\u0027t really care at this point what the exception was. We are just seeing if we can find an image with an ID that matches what was passed in. Passing on any exception will just fall into the next step of trying to match based on name rather than ID.\n\nI had considered catching authorization exceptions separately here, but decided that treating that differently could expose internal information. For example, passing in random UUIDs until there is a match to discover what is out there that the current account doesn\u0027t have access to. Unlikely, but still no reason to expose that.\n\nMake sense?","commit_id":"9cb61a870710974e70546c0122341ae698ef1f07"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"bf825c5118a7236f4140298dca376c342530563d","unresolved":false,"context_lines":[{"line_number":269,"context_line":"                image \u003d image_service.show(context, image_uuid)"},{"line_number":270,"context_line":"                if \u0027id\u0027 in image:"},{"line_number":271,"context_line":"                    return image[\u0027id\u0027]"},{"line_number":272,"context_line":"            except Exception:"},{"line_number":273,"context_line":"                # Pass and see if there is a matching image name"},{"line_number":274,"context_line":"                pass"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5a890539_66938f5b","line":272,"in_reply_to":"5a890539_d63b23e9","updated":"2014-11-21 20:44:51.000000000","message":"Makes sense to me.","commit_id":"9cb61a870710974e70546c0122341ae698ef1f07"},{"author":{"_account_id":170,"name":"Mike Perez","email":"thingee@gmail.com","username":"thingee"},"change_message_id":"398bb8daabe765c8f1ca1dc2318ae879764bc297","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPConflict(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a890539_11c81554","line":285,"updated":"2014-11-24 23:11:09.000000000","message":"return images[0][id] instead of a for loop.","commit_id":"01ac8436761d5d1297cfd2555e573c1bfd576e72"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"08ea7ecafd1b3150997637f4e1c368dc56977c20","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                        \" specific.\") % image_ref"},{"line_number":283,"context_line":"                raise exc.HTTPConflict(msg)"},{"line_number":284,"context_line":"            for img in images:"},{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a890539_8c3228a5","line":285,"in_reply_to":"5a890539_11c81554","updated":"2014-11-24 23:51:26.000000000","message":"This was done in a for loop to allow falling through to handle not being found rather than having an additional check of if len(images) \u003d\u003d 0.","commit_id":"01ac8436761d5d1297cfd2555e573c1bfd576e72"},{"author":{"_account_id":170,"name":"Mike Perez","email":"thingee@gmail.com","username":"thingee"},"change_message_id":"398bb8daabe765c8f1ca1dc2318ae879764bc297","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid image identifier or unable to \""},{"line_number":291,"context_line":"                \"access requested image.\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a890539_b1e4a1e0","line":288,"updated":"2014-11-24 23:11:09.000000000","message":"If there are issues with glance and the client raises, we\u0027ll just raise a bad request?","commit_id":"01ac8436761d5d1297cfd2555e573c1bfd576e72"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"08ea7ecafd1b3150997637f4e1c368dc56977c20","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                return img[\u0027id\u0027]"},{"line_number":286,"context_line":"        except Exception:"},{"line_number":287,"context_line":"            # Pass and let default not found error handling take care of it"},{"line_number":288,"context_line":"            pass"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        msg \u003d _(\"Invalid image identifier or unable to \""},{"line_number":291,"context_line":"                \"access requested image.\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a890539_4c992078","line":288,"in_reply_to":"5a890539_b1e4a1e0","updated":"2014-11-24 23:51:26.000000000","message":"So yeah, to avoid having to get too deep in to glance we just care if we found the image or not. There are some exceptions like authorization errors that we could catch, but my thought was we don\u0027t actually want to expose that up to the client since it\u0027s a possible way they could find out more about the system setup than they should. So to keep it safe and simple just see if we can find the image, and if we can\u0027t for any reason just return the small pointer of the error message below.","commit_id":"01ac8436761d5d1297cfd2555e573c1bfd576e72"}]}
