)]}'
{"glance/registry/api/v1/members.py":[{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"b872b65a2e56ed14042d19c8a660df701d918fa9","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FIXM%3D","side":"PARENT","line":282,"updated":"2012-10-12 20:18:32.000000000","message":"I tend to go more for this try/except approach vs the conditional approach you\u0027re proposing. The existing approach does fewer DB queries. Why not make sure the image_member_delete command is properly raising NotFound and return the HTTPNotFound in the except block (rather than just pass\u0027ing)?","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":1633,"name":"unmesh-gurjar","email":"unmesh.gurjar@hp.com","username":"unmesh-gurjar"},"change_message_id":"a14c444f32ed9880acafb0d55ee52ba6ed612d46","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F%2F4U%3D","side":"PARENT","line":282,"in_reply_to":"AAAALH%2F%2FIT4%3D","updated":"2012-10-22 11:04:20.000000000","message":"@Brian: What I understand from the above comment is, the \"image_member_delete\" method should internally call the private method \"_image_member_find\" (to fetch the image_members table row ref to be deleted).\nWhich also means, changing the method arguments \"image_member_delete(context, memb_id, session)\" to \"image_member_delete(context, image_id, member, session)\".","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"1baad8daed78411df4b08449cb4e21c04244c863","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FISs%3D","side":"PARENT","line":282,"in_reply_to":"AAAALH%2F%2FIT4%3D","updated":"2012-10-12 21:10:30.000000000","message":"ok, sounds good.","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"02150902a4d649cb91912becfa1ca7031a2bf47b","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FIT4%3D","side":"PARENT","line":282,"in_reply_to":"AAAALH%2F%2FIUc%3D","updated":"2012-10-12 21:02:01.000000000","message":"I\u0027m suggesting that we don\u0027t use a \u0027image_member_find\u0027 at all and depend on the \u0027image_member_delete\u0027 to raise the appropriate exception.","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"98aebd7a91b554d1a74b389cff770c83451b7060","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FIUc%3D","side":"PARENT","line":282,"in_reply_to":"AAAALH%2F%2FIXM%3D","updated":"2012-10-12 20:59:37.000000000","message":"agree to what Waldon is saying. db should ideally raise an exception when something is not found but you could use if condition to resolve this issue here. (Basically keep the try clause and add the if members: inside of it). Not sure what would be cleaner way to do this!?","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"bceaaa8b0c2262df765f087a1e96a7c626b79285","unresolved":false,"context_lines":[{"line_number":279,"context_line":"                                                    image_id\u003dimage_id,"},{"line_number":280,"context_line":"                                                    member\u003did)"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        except exception.NotFound:"},{"line_number":283,"context_line":"            pass"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        # Make an appropriate result"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F%2FuQ%3D","side":"PARENT","line":282,"in_reply_to":"AAAALn%2F%2F%2F4U%3D","updated":"2012-10-22 16:03:24.000000000","message":"Ah, I didn\u0027t realize that the image_member_delete function requires a member id. In that case, I think the code that you\u0027ve proposed is ok. I don\u0027t expect you to refactor underlying methods to fix this bug. Feel free to un-abandon your review so we can get the fix in to master.","commit_id":"46d38734ec179298624144cbbffe8e357ecc2c14"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"b8c17ac1542e73a4efb3d6bb9c73d4abcadc19d3","unresolved":false,"context_lines":[{"line_number":276,"context_line":"        session \u003d self.db_api.get_session()"},{"line_number":277,"context_line":"        members \u003d self.db_api.image_member_find(req.context,"},{"line_number":278,"context_line":"                                                image_id\u003dimage_id,"},{"line_number":279,"context_line":"                                                member\u003did)"},{"line_number":280,"context_line":"        if members:"},{"line_number":281,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":282,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FLJE%3D","line":279,"updated":"2012-10-09 11:50:54.000000000","message":"i think you could accommodate \"member\u003did\" in the same line as image_id. we can have one less line change.","commit_id":"1b6cf95213cd48978cfcd5ca6d155786a4b4f62e"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"a10cd1ff293b243a467907f0f5bb44e9fc22c00a","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            msg \u003d _(\"%(id)s is not a member of image %(image_id)s\")"},{"line_number":283,"context_line":"            LOG.info(msg % locals())"},{"line_number":284,"context_line":"            msg \u003d _(\"Membership could not be found.\")"},{"line_number":285,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":286,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F%2Bl8%3D","line":283,"updated":"2012-10-23 18:30:26.000000000","message":"This seems more like debug-level information.","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":1633,"name":"unmesh-gurjar","email":"unmesh.gurjar@hp.com","username":"unmesh-gurjar"},"change_message_id":"0d2a810918652c3229064b039b1b1cb5493b2044","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            msg \u003d _(\"%(id)s is not a member of image %(image_id)s\")"},{"line_number":283,"context_line":"            LOG.info(msg % locals())"},{"line_number":284,"context_line":"            msg \u003d _(\"Membership could not be found.\")"},{"line_number":285,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":286,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F8u8%3D","line":283,"in_reply_to":"AAAALn%2F%2F%2Bl8%3D","updated":"2012-10-25 11:06:52.000000000","message":"I will update the message to \"Member %(id)s not found\", in the next patch set.","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":1633,"name":"unmesh-gurjar","email":"unmesh.gurjar@hp.com","username":"unmesh-gurjar"},"change_message_id":"6ac6bca976f8b3217828be527359ab2405e294fd","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            msg \u003d _(\"%(id)s is not a member of image %(image_id)s\")"},{"line_number":283,"context_line":"            LOG.info(msg % locals())"},{"line_number":284,"context_line":"            msg \u003d _(\"Membership could not be found.\")"},{"line_number":285,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":286,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F7zQ%3D","line":283,"in_reply_to":"AAAALn%2F%2F8g4%3D","updated":"2012-10-26 07:04:05.000000000","message":"Done","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"610199f32c1eda12e94adfc1c6b887b28d585ffa","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            msg \u003d _(\"%(id)s is not a member of image %(image_id)s\")"},{"line_number":283,"context_line":"            LOG.info(msg % locals())"},{"line_number":284,"context_line":"            msg \u003d _(\"Membership could not be found.\")"},{"line_number":285,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003dmsg)"},{"line_number":286,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F8g4%3D","line":283,"in_reply_to":"AAAALn%2F%2F8u8%3D","updated":"2012-10-25 16:21:15.000000000","message":"I\u0027m not necessarily concerned about the content, it just seems like we should log at LOG.debug rather than LOG.info","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"5a96981476f7c6b0147ffb142d0f3aed5dde89b0","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        # Look up an existing membership"},{"line_number":276,"context_line":"        members \u003d self.db_api.image_member_find(req.context,"},{"line_number":277,"context_line":"                                                image_id\u003dimage_id,"},{"line_number":278,"context_line":"                                                member\u003did)"},{"line_number":279,"context_line":"        if members:"},{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAALn%2F%2F7CI%3D","line":278,"updated":"2012-10-26 21:33:53.000000000","message":"we can have \u0027member\u003did\u0027 two lines above, \n\n        members \u003d self.db_api.image_member_find(req.context, member\u003did, \n        ...","commit_id":"950e700b4fe69f001b228d11c8014ebbdfa7b10b"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"d306fa94fd140a7f066bcb99ae8f65fdbc087478","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        # Look up an existing membership"},{"line_number":276,"context_line":"        members \u003d self.db_api.image_member_find(req.context,"},{"line_number":277,"context_line":"                                                image_id\u003dimage_id,"},{"line_number":278,"context_line":"                                                member\u003did)"},{"line_number":279,"context_line":"        if members:"},{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAALn%2F%2F48M%3D","line":278,"in_reply_to":"AAAALn%2F%2F5CU%3D","updated":"2012-10-30 15:49:08.000000000","message":"Both styles are perfectly acceptable, it comes down to what the author thinks is more readable. I\u0027m fine with the style as-is.","commit_id":"950e700b4fe69f001b228d11c8014ebbdfa7b10b"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"44ca01011924f174ea8f82e60a89f87db44b40ec","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        # Look up an existing membership"},{"line_number":276,"context_line":"        members \u003d self.db_api.image_member_find(req.context,"},{"line_number":277,"context_line":"                                                image_id\u003dimage_id,"},{"line_number":278,"context_line":"                                                member\u003did)"},{"line_number":279,"context_line":"        if members:"},{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAALn%2F%2F5CU%3D","line":278,"in_reply_to":"AAAALn%2F%2F5DU%3D","updated":"2012-10-30 14:26:59.000000000","message":"agree that the code is not consistent in all the places. however, we are just adding an extra line for no truly logical use.\n\nAlthough, would suggest to wait for further opinions as some folks do not like to place args/kwargs are random places.\n\nthanks for confirming.","commit_id":"950e700b4fe69f001b228d11c8014ebbdfa7b10b"},{"author":{"_account_id":1633,"name":"unmesh-gurjar","email":"unmesh.gurjar@hp.com","username":"unmesh-gurjar"},"change_message_id":"1d1b4de6b296b50f598bed9252dae292647faac2","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        # Look up an existing membership"},{"line_number":276,"context_line":"        members \u003d self.db_api.image_member_find(req.context,"},{"line_number":277,"context_line":"                                                image_id\u003dimage_id,"},{"line_number":278,"context_line":"                                                member\u003did)"},{"line_number":279,"context_line":"        if members:"},{"line_number":280,"context_line":"            self.db_api.image_member_delete(req.context, members[0][\u0027id\u0027])"},{"line_number":281,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAALn%2F%2F5DU%3D","line":278,"in_reply_to":"AAAALn%2F%2F7CI%3D","updated":"2012-10-30 14:18:28.000000000","message":"I would prefer the current format as it is consistent with the current coding style, even in this module.\nIMO, this is more readable. Do you disagree?","commit_id":"950e700b4fe69f001b228d11c8014ebbdfa7b10b"}],"glance/tests/unit/v1/test_api.py":[{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"98aebd7a91b554d1a74b389cff770c83451b7060","unresolved":false,"context_lines":[{"line_number":1881,"context_line":"        req.method \u003d \u0027DELETE\u0027"},{"line_number":1882,"context_line":""},{"line_number":1883,"context_line":"        res \u003d req.get_response(self.api)"},{"line_number":1884,"context_line":"        self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code)"},{"line_number":1885,"context_line":""},{"line_number":1886,"context_line":""},{"line_number":1887,"context_line":"class TestGlanceAPI(base.IsolatedUnitTest):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALH%2F%2FIUQ%3D","line":1884,"updated":"2012-10-12 20:59:37.000000000","message":"why not check for the message here? would prevent inconsistency if someone decides to change the message in the future","commit_id":"1b6cf95213cd48978cfcd5ca6d155786a4b4f62e"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"a10cd1ff293b243a467907f0f5bb44e9fc22c00a","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        Tests deleting a invalid/non existing member raises right exception"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        self.api \u003d test_utils.FakeAuthMiddleware(rserver.API(self.mapper),"},{"line_number":1881,"context_line":"                                                 is_admin\u003dTrue)"},{"line_number":1882,"context_line":"        req \u003d webob.Request.blank(\u0027/images/%s/members/pattieblack\u0027 % UUID2)"},{"line_number":1883,"context_line":"        req.method \u003d \u0027DELETE\u0027"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F%2Blo%3D","line":1881,"updated":"2012-10-23 18:30:26.000000000","message":"bump this to the right to line up under \u0027rserver...\u0027","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":1633,"name":"unmesh-gurjar","email":"unmesh.gurjar@hp.com","username":"unmesh-gurjar"},"change_message_id":"0d2a810918652c3229064b039b1b1cb5493b2044","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        Tests deleting a invalid/non existing member raises right exception"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        self.api \u003d test_utils.FakeAuthMiddleware(rserver.API(self.mapper),"},{"line_number":1881,"context_line":"                                                 is_admin\u003dTrue)"},{"line_number":1882,"context_line":"        req \u003d webob.Request.blank(\u0027/images/%s/members/pattieblack\u0027 % UUID2)"},{"line_number":1883,"context_line":"        req.method \u003d \u0027DELETE\u0027"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F8vM%3D","line":1881,"in_reply_to":"AAAALn%2F%2F%2Blo%3D","updated":"2012-10-25 11:06:52.000000000","message":"@Brian: This looks like a Gerrit viewability issue, as I can see well indented code in my editor.\nCould you please check out at your end and confirm this.","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":1132,"name":"Brian Waldon","email":"brian@waldon.cc","username":"bcwaldon"},"change_message_id":"610199f32c1eda12e94adfc1c6b887b28d585ffa","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        Tests deleting a invalid/non existing member raises right exception"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        self.api \u003d test_utils.FakeAuthMiddleware(rserver.API(self.mapper),"},{"line_number":1881,"context_line":"                                                 is_admin\u003dTrue)"},{"line_number":1882,"context_line":"        req \u003d webob.Request.blank(\u0027/images/%s/members/pattieblack\u0027 % UUID2)"},{"line_number":1883,"context_line":"        req.method \u003d \u0027DELETE\u0027"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F8gw%3D","line":1881,"in_reply_to":"AAAALn%2F%2F8m0%3D","updated":"2012-10-25 16:21:15.000000000","message":"That\u0027s frustrating, I had this come up in another review as well. I\u0027ll be sure to check locally before making these comments in the future. Sorry!","commit_id":"6892e8013112495bc73bb0216365ef461f205765"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"a11937a4287a2a0d0cc735ac62094c647560f881","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"        Tests deleting a invalid/non existing member raises right exception"},{"line_number":1879,"context_line":"        \"\"\""},{"line_number":1880,"context_line":"        self.api \u003d test_utils.FakeAuthMiddleware(rserver.API(self.mapper),"},{"line_number":1881,"context_line":"                                                 is_admin\u003dTrue)"},{"line_number":1882,"context_line":"        req \u003d webob.Request.blank(\u0027/images/%s/members/pattieblack\u0027 % UUID2)"},{"line_number":1883,"context_line":"        req.method \u003d \u0027DELETE\u0027"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAALn%2F%2F8m0%3D","line":1881,"in_reply_to":"AAAALn%2F%2F8vM%3D","updated":"2012-10-25 14:40:39.000000000","message":"I can see this under \u0027rserver\u0027 i.e. correctly","commit_id":"6892e8013112495bc73bb0216365ef461f205765"}]}