)]}'
{"glance/api/v1/images.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3a2d0bad01775f3beb3b3826890239061f472991","unresolved":false,"context_lines":[{"line_number":872,"context_line":""},{"line_number":873,"context_line":"            return {\u0027image_meta\u0027: image_meta}"},{"line_number":874,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":875,"context_line":"            msg \u003d (_(\"Authentication failed during image update: %s\") %"},{"line_number":876,"context_line":"                   utils.exception_to_str(e))"},{"line_number":877,"context_line":"            LOG.warn(msg)"},{"line_number":878,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3acd31a7_728dbb7a","line":875,"updated":"2015-04-30 07:01:11.000000000","message":"update \u003e\u003e create??","commit_id":"dbff0a1276b1c62b103c51f2fbe934c719d3a0e2"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"bc1711a28214493ce47b61c27d2ff73b71ffcf90","unresolved":false,"context_lines":[{"line_number":872,"context_line":""},{"line_number":873,"context_line":"            return {\u0027image_meta\u0027: image_meta}"},{"line_number":874,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":875,"context_line":"            msg \u003d (_(\"Authentication failed during image update: %s\") %"},{"line_number":876,"context_line":"                   utils.exception_to_str(e))"},{"line_number":877,"context_line":"            LOG.warn(msg)"},{"line_number":878,"context_line":"            raise HTTPBadRequest(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3acd31a7_8c3cbb94","line":875,"in_reply_to":"3acd31a7_728dbb7a","updated":"2015-04-30 09:45:51.000000000","message":"Ooops, overlooked that. Thanks!","commit_id":"dbff0a1276b1c62b103c51f2fbe934c719d3a0e2"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"4fe6ab900c5e260ea3e068cf899859c3202df5b5","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                 request\u003dreq,"},{"line_number":573,"context_line":"                                 content_type\u003d\"text/plain\")"},{"line_number":574,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":575,"context_line":"            msg \u003d (_(\"Failed to reserve image. Got error: %s\") %"},{"line_number":576,"context_line":"                   utils.exception_to_str(e))"},{"line_number":577,"context_line":"            LOG.exception(msg)"},{"line_number":578,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aca2d91_3fcb2ca7","line":575,"updated":"2015-05-08 17:40:13.000000000","message":"I don\u0027t see the point of catching an exception here, especially with ambiguous message, because you do it on the upper level (see line 871)","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"4454e2a5fda5b8174a6f6450f30e012ddc31b570","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                 request\u003dreq,"},{"line_number":573,"context_line":"                                 content_type\u003d\"text/plain\")"},{"line_number":574,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":575,"context_line":"            msg \u003d (_(\"Failed to reserve image. Got error: %s\") %"},{"line_number":576,"context_line":"                   utils.exception_to_str(e))"},{"line_number":577,"context_line":"            LOG.exception(msg)"},{"line_number":578,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg,"}],"source_content_type":"text/x-python","patch_set":4,"id":"1aca2d91_a3abaa74","line":575,"in_reply_to":"1aca2d91_3fcb2ca7","updated":"2015-05-08 18:14:54.000000000","message":"Let me disagree with you here: the message is not ambiguous, as you can tell from it that authentication failed precisely during image reservation state (code 401 and \u0027some msg\u0027 !\u003d code 400 and \u0027some msg\u0027).\nIt also goes along with the _reserve() func\u0027s exception handling logic: all types of exceptions that can appear during reservation process clearly state that they appeared at that stage.\n\nSo, to be honest, I see no point in removing NotAuthenticated handler here. Yes, if removed, the exception will be caught in _upload() func\u0027s try/catch block, but it will give the general info and won\u0027t mention the reservation stage.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"71da4700d82da8144e17cf28aa7e5482950d02cf","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                 request\u003dreq,"},{"line_number":573,"context_line":"                                 content_type\u003d\"text/plain\")"},{"line_number":574,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":575,"context_line":"            msg \u003d (_(\"Failed to reserve image. Got error: %s\") %"},{"line_number":576,"context_line":"                   utils.exception_to_str(e))"},{"line_number":577,"context_line":"            LOG.exception(msg)"},{"line_number":578,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ad8896b_f1b562d0","line":575,"in_reply_to":"1aca2d91_a3abaa74","updated":"2015-05-11 16:44:44.000000000","message":"I think I agree with Inessa here. More detail is better in my opinion.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"a0ecaf6721cdf22ef53d6c56065885e0b08e5942","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                 request\u003dreq,"},{"line_number":573,"context_line":"                                 content_type\u003d\"text/plain\")"},{"line_number":574,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":575,"context_line":"            msg \u003d (_(\"Failed to reserve image. Got error: %s\") %"},{"line_number":576,"context_line":"                   utils.exception_to_str(e))"},{"line_number":577,"context_line":"            LOG.exception(msg)"},{"line_number":578,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_3c95fbe3","line":575,"in_reply_to":"7ad8896b_f1b562d0","updated":"2015-05-12 11:31:56.000000000","message":"First of all, I don\u0027t have anything about more details, but if Inessa decides to catch this exception several times then she should do it in the other places where it may occur. For example, in _upload() method on line 634 (registry.update_image_metadata(req.context, image_id,)\n\nSecond, what this message may say to the end user, who wants to create a new image? And also this text exactly the same as above, so it\u0027s better to combine them together in this case \nexcept (exception.Invalid, exception.NotAuthenticated) as e:","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"91f9a2fdd931eff466e61407efd63531b2b6499f","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                 request\u003dreq,"},{"line_number":573,"context_line":"                                 content_type\u003d\"text/plain\")"},{"line_number":574,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":575,"context_line":"            msg \u003d (_(\"Failed to reserve image. Got error: %s\") %"},{"line_number":576,"context_line":"                   utils.exception_to_str(e))"},{"line_number":577,"context_line":"            LOG.exception(msg)"},{"line_number":578,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_02e772e5","line":575,"in_reply_to":"9a0c5dc1_3c95fbe3","updated":"2015-05-12 12:21:12.000000000","message":"Mike, I see no point in catching the exception everywhere - I\u0027m adding the exception handling according to the existing exception handling logic. If _reserve() has detailed exception explanation - we should keep it there, but that doesn\u0027t mean that we should refactor the logic of other func\u0027s exception handling only because of the fact we \"catch smth twice in some other func\" (I\u0027d like to remind you that we are talking about would-be-deprecated-once v1 api)\n\nAs for your refactor suggestion, I\u0027ll emphasize the difference of (400, \u0027some msg\u0027) and (401, \u0027some msg\u0027) once more: \n\nyour suggestion to combine the logging here won\u0027t increase readability (and can result in more lines of code), as long as in case of 400 response we\u0027ll need to raise HTTPBadRequest, and in case of 401 - exception.NotAuthenticated.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"a0ecaf6721cdf22ef53d6c56065885e0b08e5942","unresolved":false,"context_lines":[{"line_number":883,"context_line":""},{"line_number":884,"context_line":"            return {\u0027image_meta\u0027: image_meta}"},{"line_number":885,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":886,"context_line":"            msg \u003d (_(\"Authentication failed during image creation: %s\") %"},{"line_number":887,"context_line":"                   utils.exception_to_str(e))"},{"line_number":888,"context_line":"            LOG.warn(msg)"},{"line_number":889,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_7caa531b","line":886,"updated":"2015-05-12 11:31:56.000000000","message":"Third, here Authentication and Authorization are messed. NotAuthenticated means that we are not able to get a auth_token, because there is no information about user credentials, Unauthorized means the system knows about user, but he is not authorized to perform the request. So raise HTTPUnauthorized with \u0027Authentication failed\u0027 message is wrong. \u0027Authorization failed with the given token\u0027 is the better choice, imo.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"91f9a2fdd931eff466e61407efd63531b2b6499f","unresolved":false,"context_lines":[{"line_number":883,"context_line":""},{"line_number":884,"context_line":"            return {\u0027image_meta\u0027: image_meta}"},{"line_number":885,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":886,"context_line":"            msg \u003d (_(\"Authentication failed during image creation: %s\") %"},{"line_number":887,"context_line":"                   utils.exception_to_str(e))"},{"line_number":888,"context_line":"            LOG.warn(msg)"},{"line_number":889,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_edc9efad","line":886,"in_reply_to":"9a0c5dc1_7caa531b","updated":"2015-05-12 12:21:12.000000000","message":"Correct me if I\u0027m wrong: imo, HTTPUnauthorized (401) is the only way for a glance-service to tell the user something is wrong with the token. Here I\u0027m just wrapping the more detailed, inner exception message in the expected glance-service response for the same reasons I\u0027ve kept the exception handling in _reserve() method - for the sake of more information.\n\nOf course I could make a substitution s/authentication/authorization here, but it will only look nice and consistent and hide the main cause of the problem - the non-atomic nature of image creation stage in terms of token validation.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"a0ecaf6721cdf22ef53d6c56065885e0b08e5942","unresolved":false,"context_lines":[{"line_number":1032,"context_line":"                               request\u003dreq,"},{"line_number":1033,"context_line":"                               content_type\u003d\u0027text/plain\u0027)"},{"line_number":1034,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":1035,"context_line":"            msg \u003d (_(\"Authentication failed during image update: %s\") %"},{"line_number":1036,"context_line":"                   utils.exception_to_str(e))"},{"line_number":1037,"context_line":"            LOG.warn(msg)"},{"line_number":1038,"context_line":"            raise HTTPUnauthorized(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a0c5dc1_ff08752a","line":1035,"updated":"2015-05-12 11:31:56.000000000","message":"Here the same. And also in \u0027index\u0027 and \u0027detail\u0027 methods you don\u0027t write message. I think there should be a consistency - either you write it anywhere or nowhere.","commit_id":"19f1086c0f43135b820ce82bc80257c9fab0e690"}],"glance/tests/functional/v1/test_api.py":[{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"bc1711a28214493ce47b61c27d2ff73b71ffcf90","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    \"\"\"Functional tests using httplib2 against the API server\"\"\""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def test_not_authenticated_in_registry_on_ops(self):"},{"line_number":39,"context_line":"        # https://bugs.launchpad.net/fuel/+bug/1443913"},{"line_number":40,"context_line":"        self.cleanup()"},{"line_number":41,"context_line":"        self.registry_server.deployment_flavor \u003d \u0027fakeauth\u0027"},{"line_number":42,"context_line":"        # this configuration guarantees that authentication succeeds in"}],"source_content_type":"text/x-python","patch_set":1,"id":"3acd31a7_ccc5135c","line":39,"updated":"2015-04-30 09:45:51.000000000","message":"I think I have to correct this link as well: https://bugs.launchpad.net/glance/+bug/1450041","commit_id":"dbff0a1276b1c62b103c51f2fbe934c719d3a0e2"},{"author":{"_account_id":455,"name":"Stuart McLaren","email":"stuart.mclaren@hpe.com","username":"stuart-mclaren"},"change_message_id":"949ca09add5230be08cfce2591ef4adddd186376","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        response, content \u003d http.request("},{"line_number":53,"context_line":"            _url(\u0027/v1/images\u0027), \u0027GET\u0027, headers\u003dheaders)"},{"line_number":54,"context_line":"        # no 500, just an ordinary NotAuthorized"},{"line_number":55,"context_line":"        self.assertEqual(400, response.status)"},{"line_number":56,"context_line":"        response, content \u003d http.request("},{"line_number":57,"context_line":"            _url(\u0027/v1/images\u0027), \u0027POST\u0027, headers\u003dheaders, body\u003d\"*\" * FIVE_KB)"},{"line_number":58,"context_line":"        # no 500, just an ordinary NotAuthorized"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_5f09a094","line":55,"updated":"2015-04-30 11:31:40.000000000","message":"Shouldn\u0027t we be returning 401 to the user rather than 400 for all these cases?\n\n401 is what we will get if we send no token to the api for example.\n\nTypically 400 indicates a malformed request, ie one that that retrying with a valid token would have no effect on, whereas 401 is what would be expected with a timed out token -- retrying is appropriate.","commit_id":"bb5df2133e22524303bc241da41ba3db29304104"},{"author":{"_account_id":13717,"name":"Inessa Vasilevskaya","email":"ivasilevskaya@mirantis.com","username":"ivasilevskaya"},"change_message_id":"1afbb3603d2fd7d30180ebe7cc79aa2c0b54f148","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        response, content \u003d http.request("},{"line_number":53,"context_line":"            _url(\u0027/v1/images\u0027), \u0027GET\u0027, headers\u003dheaders)"},{"line_number":54,"context_line":"        # no 500, just an ordinary NotAuthorized"},{"line_number":55,"context_line":"        self.assertEqual(400, response.status)"},{"line_number":56,"context_line":"        response, content \u003d http.request("},{"line_number":57,"context_line":"            _url(\u0027/v1/images\u0027), \u0027POST\u0027, headers\u003dheaders, body\u003d\"*\" * FIVE_KB)"},{"line_number":58,"context_line":"        # no 500, just an ordinary NotAuthorized"}],"source_content_type":"text/x-python","patch_set":2,"id":"3acd31a7_7301c569","line":55,"in_reply_to":"3acd31a7_5f09a094","updated":"2015-05-05 13:50:38.000000000","message":"Choosing between 400 and 401 I clang to the first variant as it looked less troublesome to implement. Now I doubt that it has been the correct choice, so it will be fixed the way you suggest in PS 3.","commit_id":"bb5df2133e22524303bc241da41ba3db29304104"}]}