)]}'
{"glance/api/v2/image_data.py":[{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a069b28b77c34b8930e7a05eaea0ba7fe98bc35e","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            old_status \u003d image.status"},{"line_number":53,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":54,"context_line":"            try:"},{"line_number":55,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BNYs%3D","line":52,"updated":"2013-11-29 05:35:00.000000000","message":"Actually, I think only the queued image can be upload a data. Because the image should be deployed with the same content in its whole life cycle. So I think we just need a check before setting the image status.","commit_id":"a8bbc9c66b296c41d88719bc518f253902a9f25d"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"55cba8c1e8045197beb7a455c70fdafecaa334c0","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            old_status \u003d image.status"},{"line_number":53,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":54,"context_line":"            try:"},{"line_number":55,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BM2M%3D","line":52,"in_reply_to":"AAAATn%2F%2BM%2BM%3D","updated":"2013-11-29 09:07:55.000000000","message":"Let me try to answer your two scenarios:\n\n1. \"somebody\" delete the image file/data. I don\u0027t think there is anyway can do that via Glance API.\n\n2. \"somebody\" copied file in to the file location. Did you mean copying image data to the backend directly? If so, I don\u0027t think it makes any sense. Because admin/deployer won\u0027t do that and the other user don\u0027t have the permission do that.\n\nPlease let me know if I missed anything.","commit_id":"a8bbc9c66b296c41d88719bc518f253902a9f25d"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"81dd2d536152f69afe6211ef1263cd7d74b1ffb3","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            old_status \u003d image.status"},{"line_number":53,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":54,"context_line":"            try:"},{"line_number":55,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BM0I%3D","line":52,"in_reply_to":"AAAATn%2F%2BM2M%3D","updated":"2013-11-29 09:19:13.000000000","message":"Regarding (1): I agree - that is not possible through APIs. But perhaps it may happen either through some \"attack\" or perhaps as a result of file loss due to filesystem recovery (from improper shutdown).\n\nRegarding (2): I was thinking of some \"attack\". But then if some attacker can do this then I suppose there are bigger problems to think of!\n\nI agree that both cases are extremely unlikely in real deployments so I suppose we can ignore these two cases. I will upload a new patchset with your suggestions.\n\nThanks for the guidance :).","commit_id":"a8bbc9c66b296c41d88719bc518f253902a9f25d"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"cf5a1c481cfe4fd819141a22d01ba45f12adb370","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            old_status \u003d image.status"},{"line_number":53,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":54,"context_line":"            try:"},{"line_number":55,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BNYM%3D","line":52,"in_reply_to":"AAAATn%2F%2BNYs%3D","updated":"2013-11-29 05:39:31.000000000","message":"+1","commit_id":"a8bbc9c66b296c41d88719bc518f253902a9f25d"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"407248db7e5d185dc0aae5aaec4102a1c67beb31","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            old_status \u003d image.status"},{"line_number":53,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":54,"context_line":"            try:"},{"line_number":55,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BM%2BM%3D","line":52,"in_reply_to":"AAAATn%2F%2BNYs%3D","updated":"2013-11-29 08:39:28.000000000","message":"I have small concern about a simple check here: what if \"somebody\" goes and deletes the image file after successfully uploading it the first time round. Should the user be allowed to \"re-upload\" the image?\n\nIf the user is allowed to re-upload the image then a check here for \"queued\" state would prevent the user from doing this ... we would have to allow the user to go through the whole \"saving -\u003e active\" transition again.\n\nOn the other hand, even if we put a check here, it is possible that \"somebody\" copied some file in the file location, resulting in an exception being thrown when the save is attempted (though the image is in the right state). Should we then disregard and overwrite any file in the backend storage if the image is in the queued status?\n\nShould we try to recover from these inconsistent states (I think we should) or are these two use cases simply not worth bothering about?","commit_id":"a8bbc9c66b296c41d88719bc518f253902a9f25d"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"f4e22cd62add887ebece024198465f739cb1537a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":55,"context_line":"            if image.status and image.status not in self.valid_states:"},{"line_number":56,"context_line":"                msg \u003d (_(\"Invalid image state (%s) for %s\") %"},{"line_number":57,"context_line":"                       (image.status, image_id))"},{"line_number":58,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BMBQ%3D","line":55,"updated":"2013-11-29 15:44:51.000000000","message":"I can\u0027t see any reason why we need this check. Would you mind explaining?","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":5347,"name":"Eddie Sheffield","email":"eddie.sheffield@rackspace.com","username":"eddie-sheffield"},"change_message_id":"66d63aa55e39980336bffd6fc8eb5330ca5ddd80","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":55,"context_line":"            if image.status and image.status not in self.valid_states:"},{"line_number":56,"context_line":"                msg \u003d (_(\"Invalid image state (%s) for %s\") %"},{"line_number":57,"context_line":"                       (image.status, image_id))"},{"line_number":58,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BAZM%3D","line":55,"in_reply_to":"AAAATn%2F%2BLTw%3D","updated":"2013-12-03 15:35:43.000000000","message":"I agree with Fei - we really only need to check if the image is \u0027queued\u0027 at this point. Adding things that don\u0027t contribute to required functionality but just make current tests keep pass are a smell that there\u0027s something wrong with the tests (or they simply aren\u0027t valid anymore.)","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6861712575581760d3d08c0bc51a05f3b463fddb","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":53,"context_line":"        try:"},{"line_number":54,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":55,"context_line":"            if image.status and image.status not in self.valid_states:"},{"line_number":56,"context_line":"                msg \u003d (_(\"Invalid image state (%s) for %s\") %"},{"line_number":57,"context_line":"                       (image.status, image_id))"},{"line_number":58,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLTw%3D","line":55,"in_reply_to":"AAAATn%2F%2BMBQ%3D","updated":"2013-11-29 17:48:35.000000000","message":"I had to do this to make tests/unit/v2/test_image_data_resource.py:TestImagesController.test_upload_invalid work. If the image is not in the correct state then I need to throw an exception that is expected by the test code (since it presumably defines the expected behaviour).","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"f4e22cd62add887ebece024198465f739cb1537a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                       (image.status, image_id))"},{"line_number":58,"context_line":"                LOG.debug(msg)"},{"line_number":59,"context_line":"                raise ValueError(msg)"},{"line_number":60,"context_line":"            elif image.status and image.status !\u003d \"queued\":"},{"line_number":61,"context_line":"                msg \u003d \"Image file upload for image %s in state %s rejected\" % ("},{"line_number":62,"context_line":"                        image_id, image.status)"},{"line_number":63,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BMAE%3D","line":60,"updated":"2013-11-29 15:44:51.000000000","message":"IMHO, it\u0027s not necessary to check image.status, we just need to check if image.status is \u0027queued\u0027 or not.","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6861712575581760d3d08c0bc51a05f3b463fddb","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                       (image.status, image_id))"},{"line_number":58,"context_line":"                LOG.debug(msg)"},{"line_number":59,"context_line":"                raise ValueError(msg)"},{"line_number":60,"context_line":"            elif image.status and image.status !\u003d \"queued\":"},{"line_number":61,"context_line":"                msg \u003d \"Image file upload for image %s in state %s rejected\" % ("},{"line_number":62,"context_line":"                        image_id, image.status)"},{"line_number":63,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLSU%3D","line":60,"in_reply_to":"AAAATn%2F%2BMAE%3D","updated":"2013-11-29 17:48:35.000000000","message":"That\u0027s for a bunch of unit tests in the tests/unit/v2/test_image_data_resource.py:TestImagesController class (test_upload_invalid, test_upload_non_existent_image_during_save_initiates_deletion, test_upload_data_exists) - they create a \"FakeImage\" object but unfortunately they don\u0027t initialize image.status. Therefore if I don\u0027t allow None to pass through, the tests fail (the tests expect different exceptions but I can raise at most one exception here).\n\nPerhaps I should modify the test code instead?","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"f4e22cd62add887ebece024198465f739cb1537a","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                LOG.debug(msg)"},{"line_number":59,"context_line":"                raise ValueError(msg)"},{"line_number":60,"context_line":"            elif image.status and image.status !\u003d \"queued\":"},{"line_number":61,"context_line":"                msg \u003d \"Image file upload for image %s in state %s rejected\" % ("},{"line_number":62,"context_line":"                        image_id, image.status)"},{"line_number":63,"context_line":"                LOG.debug(msg)"},{"line_number":64,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLms%3D","line":61,"updated":"2013-11-29 15:44:51.000000000","message":"Pls follow the right format to support msg translation.","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":5347,"name":"Eddie Sheffield","email":"eddie.sheffield@rackspace.com","username":"eddie-sheffield"},"change_message_id":"66d63aa55e39980336bffd6fc8eb5330ca5ddd80","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                LOG.debug(msg)"},{"line_number":59,"context_line":"                raise ValueError(msg)"},{"line_number":60,"context_line":"            elif image.status and image.status !\u003d \"queued\":"},{"line_number":61,"context_line":"                msg \u003d \"Image file upload for image %s in state %s rejected\" % ("},{"line_number":62,"context_line":"                        image_id, image.status)"},{"line_number":63,"context_line":"                LOG.debug(msg)"},{"line_number":64,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BAXM%3D","line":61,"in_reply_to":"AAAATn%2F%2BLRg%3D","updated":"2013-12-03 15:35:43.000000000","message":"This logic feels a bit too convoluted. I prefer the wording of this message for the error as the existing one below incorrectly implies you can\u0027t upload the *exact* same data, but you could possibly upload different data. This whole area of code could use a rewrite, but for this specific fix I would prefer to see this message fixed per Fei\u0027s comment, and then in the \u0027except\u0027 below just re-raise the exception created here.\n\nAlternatively just raise exceptions.Duplicate here and change the \u0027except\u0027 clause below back to something similar to what it was before, but with an improved message if possible.","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6861712575581760d3d08c0bc51a05f3b463fddb","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                LOG.debug(msg)"},{"line_number":59,"context_line":"                raise ValueError(msg)"},{"line_number":60,"context_line":"            elif image.status and image.status !\u003d \"queued\":"},{"line_number":61,"context_line":"                msg \u003d \"Image file upload for image %s in state %s rejected\" % ("},{"line_number":62,"context_line":"                        image_id, image.status)"},{"line_number":63,"context_line":"                LOG.debug(msg)"},{"line_number":64,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLRg%3D","line":61,"in_reply_to":"AAAATn%2F%2BLms%3D","updated":"2013-11-29 17:48:35.000000000","message":"The reason I didn\u0027t do that is because the exception will be caught in the outer try block and there we have a proper translation-friendly message string.\n\nDo the translation rules apply for debug logs too?","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"f4e22cd62add887ebece024198465f739cb1537a","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        except webob.exc.HTTPConflict:"},{"line_number":91,"context_line":"            # bugfix:1241379 - the glance.notifier module catches the original"},{"line_number":92,"context_line":"            # exception.Duplicate exception and raises a webob.exc.HTTPConflict"},{"line_number":93,"context_line":"            # exception instead."},{"line_number":94,"context_line":"            msg \u003d (_(\"Unable to upload duplicate image data for image: %s\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLl0%3D","line":91,"updated":"2013-11-29 15:44:51.000000000","message":"Hmm... It\u0027s hard to say if you\u0027re doing the right thing. I would say you\u0027re making up a hole but didn\u0027t touch the real problem. See https://bugs.launchpad.net/glance/+bug/1256220","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6861712575581760d3d08c0bc51a05f3b463fddb","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        except webob.exc.HTTPConflict:"},{"line_number":91,"context_line":"            # bugfix:1241379 - the glance.notifier module catches the original"},{"line_number":92,"context_line":"            # exception.Duplicate exception and raises a webob.exc.HTTPConflict"},{"line_number":93,"context_line":"            # exception instead."},{"line_number":94,"context_line":"            msg \u003d (_(\"Unable to upload duplicate image data for image: %s\") %"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F%2BLQ8%3D","line":91,"in_reply_to":"AAAATn%2F%2BLl0%3D","updated":"2013-11-29 17:48:35.000000000","message":"I agree it\u0027s a little long-winded but I didn\u0027t want to change too much of the code (and break everything) in my first attempt. So I thought it would be better I first try to solve this problem assuming the notifier will get cleaned in another set of patches.\n\nBut if you think it\u0027s better, I can try to fix the two bugs in one shot.","commit_id":"da8b91be21c939ffd5f14ae4cbbbefd5853b5059"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"35122ef1a27c4504d933e7ee4eb81a15d556b5e2","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F93Uc%3D","line":52,"updated":"2013-12-05 04:05:24.000000000","message":"I think we are actually trying to add a checking to guarantee an image can be transferred to \"saving\" status from \"queued\" status only. So I think it will be better to avoid put business logic into wsgi level, we can put this checking logic down to glance.domain.Image at [1] IMO.\n\n[1] https://github.com/openstack/glance/blob/master/glance/domain/__init__.py#L128","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"aef464f7b2068cf2883f37ba067cbc28abf0a8c1","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9x5o%3D","line":52,"in_reply_to":"AAAATn%2F90a0%3D","updated":"2013-12-06 00:31:17.000000000","message":"I tried this out at home last night but ran into a problem: I tried to validate /all/ state transitions in accordance to the developer docs (\"queued\" can go only to \"saving\" or \"active\", etc.) but found that the delete API first sets the state to \"delete\" and then further down, the storage layer changes it to \"pending_delete\" depending on the configuration. This makes enforcing state a little more complicated. I will try to see if I can solve fix this state machine without breaking anything else.\n\nWorst case, I can hard-code a check for \"queued\" -\u003e \"saving\" state there but I think that might look a bit dirty.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":5347,"name":"Eddie Sheffield","email":"eddie.sheffield@rackspace.com","username":"eddie-sheffield"},"change_message_id":"e5f930905abf4702eb7f78cbdb188be9ba0f49c7","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F90a0%3D","line":52,"in_reply_to":"AAAATn%2F928s%3D","updated":"2013-12-05 15:52:56.000000000","message":"Zhi has a good point and I agree. Keep in mind that you don\u0027t want to be raising HTTPXxxx exceptions from the business logic layer, so when you move this down raise something like exceptions.Duplicate (or maybe a new exception that\u0027s more descriptive like exceptions.InvalidState or something) and translate it to HTTPConflict in this layer.\n\nThanks for the work!","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"2f860b8a3a50bba4c6a02cd68de00b358defcc28","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F928s%3D","line":52,"in_reply_to":"AAAATn%2F93Uc%3D","updated":"2013-12-05 12:39:48.000000000","message":"Let me see if I have understood you correctly (since I\u0027m a newbie): What you\u0027re suggesting is that I move this check to glance.domain.Image.status(), am I right?\n\nI think that\u0027s a good idea, but since that method is called for all \"status\" updates, then I should check for valid status values as well as valid state transitions too.\n\nI was going to upload the patchset incorporating Eddie Sheffield\u0027s comments but I think I will try do do what you suggest and see if the tests need any modifications.\n\nUnless, of course, other reviewers have different opinions.\n\nThanks for your guidance.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"6e356f2995b19db092bb4dddadf73f54c25a6e54","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9xeU%3D","line":52,"in_reply_to":"AAAATn%2F9x5o%3D","updated":"2013-12-06 03:58:34.000000000","message":"hmm we are in the same page now. I\u0027m looking forward to see your \"validate all state transitions\" implementation.\n\nactually i think we need a global image state FSM mechanism finally, thoughts? https://blueprints.launchpad.net/glance/+spec/image-status-global-state-machine","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"90bed226d2ae074d7cf38f1895d0dab976fd1806","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9xag%3D","line":52,"in_reply_to":"AAAATn%2F9xcE%3D","updated":"2013-12-06 04:25:59.000000000","message":"Of cause, no worries, and please skip it, sorry.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"727c3870859b5a50b3cdea99947c7466aa684186","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9xcE%3D","line":52,"in_reply_to":"AAAATn%2F9xdU%3D","updated":"2013-12-06 04:14:11.000000000","message":"I just browsed the blueprint (and etherpad link) ... that is a MUCH bigger work than I can do currently! If I understand correctly, the blueprint aims at implementing a \"safe\" state machine that can also prevent race conditions and ensure DB consistency, right?\n\nThat\u0027s a little too big a bite for me now, though I sure would be interested in slowly working towards it. I\u0027ll set that as my goal but for this particular bug, can I just put in a simple state transition check?","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"ac4e3089969b6f4d03ed2ecfb5d3a6e4be8f3319","unresolved":false,"context_lines":[{"line_number":49,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":50,"context_line":"        try:"},{"line_number":51,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":52,"context_line":"            if image.status !\u003d \"queued\":"},{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F9xdU%3D","line":52,"in_reply_to":"AAAATn%2F9xeU%3D","updated":"2013-12-06 04:04:43.000000000","message":"The \"validation\" part is easy ... fixing the problems isn\u0027t :)!!\n\nI can catch invalid state transitions but I\u0027m still have to get more familiarity with Glance to see how to fix them.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":5347,"name":"Eddie Sheffield","email":"eddie.sheffield@rackspace.com","username":"eddie-sheffield"},"change_message_id":"4f0400929fd2241ee07fdbd370eff63fbae7201a","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"},{"line_number":56,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"},{"line_number":57,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":58,"context_line":"            try:"},{"line_number":59,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F94L4%3D","line":56,"updated":"2013-12-04 23:11:51.000000000","message":"Much nicer! One tiny quibble - this exception will get caught and re-raised by the general \u0027except Exception\u0027 clause below. Just causes an extra log line to be generated. If other reviewers are ok with that so am I, but you may need to add an \u0027except\u0027 clause to catch and silently re-raise this exception explicitly if you need to get rid of the extra log line.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6892364d97af6fbb70d63b879fd2b6735f313210","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                msg \u003d (_(\"An image file for image %s already exists\")"},{"line_number":54,"context_line":"                       % image_id)"},{"line_number":55,"context_line":"                LOG.debug(msg)"},{"line_number":56,"context_line":"                raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"},{"line_number":57,"context_line":"            image.status \u003d \u0027saving\u0027"},{"line_number":58,"context_line":"            try:"},{"line_number":59,"context_line":"                image_repo.save(image)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F93bw%3D","line":56,"in_reply_to":"AAAATn%2F94L4%3D","updated":"2013-12-05 03:11:32.000000000","message":"Aargh, I missed the catch-all \"except Exception\"! I\u0027ll catch and re-raise it.\n\nThanks.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        except exception.InvalidState:"},{"line_number":78,"context_line":"            msg \u003d _(\"An image file for image %s already exists\") % image_id"},{"line_number":79,"context_line":"            LOG.debug(msg)"},{"line_number":80,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wbI%3D","line":78,"updated":"2013-12-06 11:34:07.000000000","message":"Do you think this msg is little overkill? since you are assuming this exception has always be triggered by transferring from active/non-queued to saving (image file/content already exists), for example we have a checking in domain.Image to prevent image status change to saving/killed from saving, and if we meet that situation then end user will see this error message, but maybe finally image does not has its content at all.\n\nI\u0027m not sure what message is appropriate at here for end user, since it is based on its status changing context. Can we just use exception\u0027s message to HTTPConflict for end user directly? Any thoughts?\n\nthanks","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dunicode(e))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        except exception.InvalidState:"},{"line_number":78,"context_line":"            msg \u003d _(\"An image file for image %s already exists\") % image_id"},{"line_number":79,"context_line":"            LOG.debug(msg)"},{"line_number":80,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003dmsg, request\u003dreq)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9vxg%3D","line":78,"in_reply_to":"AAAATn%2F9wbI%3D","updated":"2013-12-06 13:17:09.000000000","message":"I\u0027ll need to think harder on this.\n\nThe problem is that a lot of context information is lost in the domain.Image status setter so I can\u0027t raise a more specific exception there. I need to find a way to pass context info lower down.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"950e64cef1849c649d30b3156fcd2e3cfab643e5","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        except exception.DuplicateImageFile as e:"},{"line_number":78,"context_line":"            LOG.debug(e)"},{"line_number":79,"context_line":"            raise webob.exc.HTTPConflict(explanation\u003de, request\u003dreq)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        except exception.Forbidden as e:"},{"line_number":82,"context_line":"            msg \u003d (_(\"Not allowed to upload image data for image %s\") %"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9M%2Bw%3D","line":79,"updated":"2013-12-14 07:47:43.000000000","message":"For avoid message coding error, I prefer set unicode error message to exception object. (\"explanation\u003dunicode(e)\")","commit_id":"898810ec1e80d59ddcfc245afe2d24f859b8303c"}],"glance/common/exception.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    message \u003d _(\"The location %(location)s already exists\")"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"class InvalidState(GlanceException):"},{"line_number":334,"context_line":"    message \u003d _(\"Transition from %(start_state)s to %(target_state)s \""},{"line_number":335,"context_line":"                \"state is not allowed.\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wcQ%3D","line":333,"updated":"2013-12-06 11:34:07.000000000","message":"I think following will be better if we keep consistent with existing code (Line #324-326):\n\nclass InvalidImageStatusTransition(GlanceException, Invalid):\n        message \u003d _(\"Status transition from %(cur_status)s to\"\n                            \" %(new_status)s is not allowed\")","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":330,"context_line":"    message \u003d _(\"The location %(location)s already exists\")"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"class InvalidState(GlanceException):"},{"line_number":334,"context_line":"    message \u003d _(\"Transition from %(start_state)s to %(target_state)s \""},{"line_number":335,"context_line":"                \"state is not allowed.\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9vys%3D","line":333,"in_reply_to":"AAAATn%2F9wcQ%3D","updated":"2013-12-06 13:17:09.000000000","message":"OK","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"}],"glance/domain/__init__.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":98,"context_line":"class Image(object):"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    valid_state_targets \u003d {"},{"line_number":101,"context_line":"        None: (\u0027queued\u0027, \u0027saving\u0027, \u0027active\u0027),"},{"line_number":102,"context_line":"        \u0027queued\u0027: (\u0027saving\u0027, \u0027active\u0027, \u0027deleted\u0027),"},{"line_number":103,"context_line":"        \u0027saving\u0027: (\u0027active\u0027, \u0027killed\u0027, \u0027deleted\u0027),"},{"line_number":104,"context_line":"        \u0027active\u0027: (\u0027queued\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wSs%3D","line":101,"updated":"2013-12-06 11:34:07.000000000","message":"on the code implementation level, should we allow target state can change to its original value? I think it can avoid some unnecessary exception raisings (if we have, now and future). \n\nor we can add a checking at line #144, if new status equal original status value, then return directly.\n\nThat means image status has no change.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":98,"context_line":"class Image(object):"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    valid_state_targets \u003d {"},{"line_number":101,"context_line":"        None: (\u0027queued\u0027, \u0027saving\u0027, \u0027active\u0027),"},{"line_number":102,"context_line":"        \u0027queued\u0027: (\u0027saving\u0027, \u0027active\u0027, \u0027deleted\u0027),"},{"line_number":103,"context_line":"        \u0027saving\u0027: (\u0027active\u0027, \u0027killed\u0027, \u0027deleted\u0027),"},{"line_number":104,"context_line":"        \u0027active\u0027: (\u0027queued\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9v00%3D","line":101,"in_reply_to":"AAAATn%2F9wSs%3D","updated":"2013-12-06 13:17:09.000000000","message":"I think I\u0027ll put it below - makes code browsing a little easier.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"db6ed6157b9b1457eecd7996391112ad5d92440a","unresolved":false,"context_lines":[{"line_number":98,"context_line":"class Image(object):"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    valid_state_targets \u003d {"},{"line_number":101,"context_line":"        None: (\u0027queued\u0027, \u0027saving\u0027, \u0027active\u0027),"},{"line_number":102,"context_line":"        \u0027queued\u0027: (\u0027saving\u0027, \u0027active\u0027, \u0027deleted\u0027),"},{"line_number":103,"context_line":"        \u0027saving\u0027: (\u0027active\u0027, \u0027killed\u0027, \u0027deleted\u0027),"},{"line_number":104,"context_line":"        \u0027active\u0027: (\u0027queued\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9lDQ%3D","line":101,"in_reply_to":"AAAATn%2F9wSs%3D","updated":"2013-12-10 01:23:23.000000000","message":"One thought struck me ... if we allow a target state to be set to the same value then we could have problems like permitting an image file to be uploaded while the upload is in progress (the status does not change and so the upload proceeds and the two concurrent uploads mess up the image file).\n\nSo I think, for most part, such transitions should also be checked for in a case by case basis.\n\nMy next patchset should will take this into consideration too - hopefully today.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    @status.setter"},{"line_number":138,"context_line":"    def status(self, status):"},{"line_number":139,"context_line":"        has_status \u003d hasattr(self, \u0027_status\u0027)"},{"line_number":140,"context_line":"        if not has_status:"},{"line_number":141,"context_line":"            curr_status \u003d None"},{"line_number":142,"context_line":"        else:"},{"line_number":143,"context_line":"            curr_status \u003d self._status"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wWA%3D","line":140,"updated":"2013-12-06 11:34:07.000000000","message":"nit: curr_status \u003d self._status if has_status else None\n\nobviously it\u0027s not a blocker.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":137,"context_line":"    @status.setter"},{"line_number":138,"context_line":"    def status(self, status):"},{"line_number":139,"context_line":"        has_status \u003d hasattr(self, \u0027_status\u0027)"},{"line_number":140,"context_line":"        if not has_status:"},{"line_number":141,"context_line":"            curr_status \u003d None"},{"line_number":142,"context_line":"        else:"},{"line_number":143,"context_line":"            curr_status \u003d self._status"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wJ0%3D","line":140,"in_reply_to":"AAAATn%2F9wWA%3D","updated":"2013-12-06 13:17:09.000000000","message":"I wanted to do that but I don\u0027t know if python 2.6 supports that syntax (I don\u0027t have python 2.6).\n\nOk - it seems to work even in 2.5.5 so I\u0027ll shorten it.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            curr_status \u003d self._status"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if status not in self.valid_state_targets[curr_status]:"},{"line_number":146,"context_line":"            LOG.debug(\"Invalid image state transition from \u0027%s\u0027 -\u003e \u0027%s\u0027\" %"},{"line_number":147,"context_line":"                      (curr_status, status))"},{"line_number":148,"context_line":"            raise exception.InvalidState(start_state\u003dcurr_status,"},{"line_number":149,"context_line":"                                         target_state\u003dstatus)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wOo%3D","line":146,"updated":"2013-12-06 11:34:07.000000000","message":"1. pls using _() to help support i18n.\n\n2. pls using dict do replacement for string formation since you have more then one placeholder.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            curr_status \u003d self._status"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        if status not in self.valid_state_targets[curr_status]:"},{"line_number":146,"context_line":"            LOG.debug(\"Invalid image state transition from \u0027%s\u0027 -\u003e \u0027%s\u0027\" %"},{"line_number":147,"context_line":"                      (curr_status, status))"},{"line_number":148,"context_line":"            raise exception.InvalidState(start_state\u003dcurr_status,"},{"line_number":149,"context_line":"                                         target_state\u003dstatus)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wJs%3D","line":146,"in_reply_to":"AAAATn%2F9wOo%3D","updated":"2013-12-06 13:17:09.000000000","message":"Regarding (1): Since it\u0027s only a debug message, I didn\u0027t use it ... I recall the wiki actually discouraging using _() for debug logs.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"dcc73edaf3db1488c31648ece6c96dec6336f167","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def delete(self):"},{"line_number":182,"context_line":"        if self.protected:"},{"line_number":183,"context_line":"            raise exception.ProtectedImageDelete(image_id\u003dself.image_id)"},{"line_number":184,"context_line":"        if CONF.delayed_delete and self.locations:"},{"line_number":185,"context_line":"            self.status \u003d \u0027pending_delete\u0027"},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            self.status \u003d \u0027deleted\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9wQc%3D","line":184,"updated":"2013-12-06 11:34:07.000000000","message":"we need import \"delayed_delete\" options from store before use it.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6addab812f9df75423f540a02c68a0a9f2220644","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def delete(self):"},{"line_number":182,"context_line":"        if self.protected:"},{"line_number":183,"context_line":"            raise exception.ProtectedImageDelete(image_id\u003dself.image_id)"},{"line_number":184,"context_line":"        if CONF.delayed_delete and self.locations:"},{"line_number":185,"context_line":"            self.status \u003d \u0027pending_delete\u0027"},{"line_number":186,"context_line":"        else:"},{"line_number":187,"context_line":"            self.status \u003d \u0027deleted\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F9vy4%3D","line":184,"in_reply_to":"AAAATn%2F9wQc%3D","updated":"2013-12-06 13:17:09.000000000","message":"Ok. But that also worries me ... how did the delayed_delete unit tests pass then?!\n\nI\u0027ll have to take a closer look.","commit_id":"06b4a21c8bd22b1262e0d36178ef346420508ac4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"950e64cef1849c649d30b3156fcd2e3cfab643e5","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            #   \"target_state\": { \"source_state\": exception_instance, ... }"},{"line_number":168,"context_line":"            #   ..."},{"line_number":169,"context_line":"            # }"},{"line_number":170,"context_line":"            # A special value of \"*\" for a \"source state\" matches any (invalid)"},{"line_number":171,"context_line":"            # source state."},{"line_number":172,"context_line":"            not_found \u003d exception.NotFound()"},{"line_number":173,"context_line":"            exc_table \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9M9I%3D","line":170,"updated":"2013-12-14 07:47:43.000000000","message":"tbh i think this stuff/approach is hard to make it, what we try to resolve, always correct and hard to understanding (and maintenance so) since imo we can\u0027t get correct error message for end user base on image status transform only (or hard to mark it, need much logic checking).\n\nSo do you think we can just raise one generic exception to end user directly instead a particular conjectural exception? in the exception we can tell user that invalid status transition (source and target), i think it\u0027s easy and enough for end user know what\u0027s wrong there (like https://github.com/openstack/glance/blob/master/glance/common/exception.py#L324 )\n\nthoughts?","commit_id":"898810ec1e80d59ddcfc245afe2d24f859b8303c"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"1645a966e133c70c4763275e10ec23cb9c74b63d","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            #   \"target_state\": { \"source_state\": exception_instance, ... }"},{"line_number":168,"context_line":"            #   ..."},{"line_number":169,"context_line":"            # }"},{"line_number":170,"context_line":"            # A special value of \"*\" for a \"source state\" matches any (invalid)"},{"line_number":171,"context_line":"            # source state."},{"line_number":172,"context_line":"            not_found \u003d exception.NotFound()"},{"line_number":173,"context_line":"            exc_table \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F9K%2BE%3D","line":170,"in_reply_to":"AAAATn%2F9M9I%3D","updated":"2013-12-16 00:25:39.000000000","message":"The reason I took this approach is that I didn\u0027t want the user to have to understand the image status state machine. Currently, it might not be too hard to understand but in future with more states and transitions I felt it would be best if we didn\u0027t have to make the user figure out what the problem is.\n\nBut on the other hand we already expose image status info to the user (though I don\u0027t remember the (user) docs mentioning the state diagram), so we might as well let the user figure it out for himself/herself.\n\nI hate returning just a generic error message though :(!","commit_id":"898810ec1e80d59ddcfc245afe2d24f859b8303c"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"684103de495c29ad0222fd579c349e2275b1bc94","unresolved":false,"context_lines":[{"line_number":52,"context_line":"_delayed_delete_imported \u003d False"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def _import_delayed_delete():"},{"line_number":56,"context_line":"    # glance.store (indirectly) imports glance.domain therefore we can\u0027t put"},{"line_number":57,"context_line":"    # the CONF.import_opt outside - we have to do it in a convoluted/indirect"},{"line_number":58,"context_line":"    # way!!"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2Frc8%3D","line":55,"updated":"2014-01-03 17:29:30.000000000","message":"so what about don\u0027t change these two parts at here and store.__init__.py? then we can prevent do this strange importing.","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"a9704ad688c56c5b80038bb28c820a2192510b6b","unresolved":false,"context_lines":[{"line_number":52,"context_line":"_delayed_delete_imported \u003d False"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def _import_delayed_delete():"},{"line_number":56,"context_line":"    # glance.store (indirectly) imports glance.domain therefore we can\u0027t put"},{"line_number":57,"context_line":"    # the CONF.import_opt outside - we have to do it in a convoluted/indirect"},{"line_number":58,"context_line":"    # way!!"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F%2BO4%3D","line":55,"in_reply_to":"AAAAT3%2F%2Frc8%3D","updated":"2014-01-06 01:32:47.000000000","message":"If we do that then we need to allow a \"deleted\" -\u003e \"pending delete\" transition (that\u0027s what the original code was like - the image is first deleted and then, when it gets to store/__init__() it changes the \"delete\" to \"pending_delete\"). But that messes up with the state transition check in the \"status\" setter function.\n\nI don\u0027t know if there\u0027s a big harm in adding a \"deleted\" -\u003e \"pending delete\" transition (because user can never access the \"pending_delete\" state directly anyway) but it just seemed neater (in terms of code) to do the check here and not have the store to worry about this (and keep the state machine clean).\n\nBesides, even though it is a little twisted, the import from inside this function happens only once anyway so I suppose it\u0027s not a big deal, is it?","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"591d9915d81fe0b7fae390be621f52112d3a5ff6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"_delayed_delete_imported \u003d False"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"def _import_delayed_delete():"},{"line_number":56,"context_line":"    # glance.store (indirectly) imports glance.domain therefore we can\u0027t put"},{"line_number":57,"context_line":"    # the CONF.import_opt outside - we have to do it in a convoluted/indirect"},{"line_number":58,"context_line":"    # way!!"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F9%2F4%3D","line":55,"in_reply_to":"AAAAUH%2F%2F%2BO4%3D","updated":"2014-01-06 03:50:22.000000000","message":"1. The reason why I propose to revert this is that this method is really ugly. (as we know because we have a loop importing between here an store)\n\n2. currently you are right, end user can not access \"pending_delete\" state directly but I\u0027m not sure we won\u0027t do that in future. ( you can take a look on [1], just a context sharing, but I believe \"pending_delete\" is a glance internal status for image bugt end user)\n\nI\u0027m not a blocker at here, and I think image status \"deleted\" -\u003e \"pending delete\" transition is not make sense base on current design.\n\n[1] https://blueprints.launchpad.net/glance/+spec/image-recover (an open question at Whiteboard section)\n\nthanks","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"591d9915d81fe0b7fae390be621f52112d3a5ff6","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    # glance.store (indirectly) imports glance.domain therefore we can\u0027t put"},{"line_number":57,"context_line":"    # the CONF.import_opt outside - we have to do it in a convoluted/indirect"},{"line_number":58,"context_line":"    # way!!"},{"line_number":59,"context_line":"    global CONF, _delayed_delete_imported"},{"line_number":60,"context_line":"    if not _delayed_delete_imported:"},{"line_number":61,"context_line":"        CONF.import_opt(\u0027delayed_delete\u0027, \u0027glance.store\u0027)"},{"line_number":62,"context_line":"        _delayed_delete_imported \u003d True"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F9%2Fc%3D","line":59,"updated":"2014-01-06 03:50:22.000000000","message":"we don\u0027t need do \"global CONF\" at here.","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"b3894aa34443f2453868736e02f315e1ae3650c4","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    # glance.store (indirectly) imports glance.domain therefore we can\u0027t put"},{"line_number":57,"context_line":"    # the CONF.import_opt outside - we have to do it in a convoluted/indirect"},{"line_number":58,"context_line":"    # way!!"},{"line_number":59,"context_line":"    global CONF, _delayed_delete_imported"},{"line_number":60,"context_line":"    if not _delayed_delete_imported:"},{"line_number":61,"context_line":"        CONF.import_opt(\u0027delayed_delete\u0027, \u0027glance.store\u0027)"},{"line_number":62,"context_line":"        _delayed_delete_imported \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUH%2F%2F7%2BQ%3D","line":59,"updated":"2014-01-06 15:40:20.000000000","message":"we don\u0027t need do \"global CONF\" at here.","commit_id":"aeaac90674914c5196fd3d0f7fa243870d61714e"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"b3894aa34443f2453868736e02f315e1ae3650c4","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @status.setter"},{"line_number":154,"context_line":"    def status(self, status):"},{"line_number":155,"context_line":"        if hasattr(self, \u0027_status\u0027):"},{"line_number":156,"context_line":"            cur_status \u003d self._status"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            if status not in self.valid_state_targets[cur_status]:"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUH%2F%2F77k%3D","line":155,"updated":"2014-01-06 15:40:20.000000000","message":"Under this new logic, we will miss the checking rules which on above line #117, so IMO original PS#7 logic is better.","commit_id":"aeaac90674914c5196fd3d0f7fa243870d61714e"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a5a8d27f61ec5676eaafaba153bcae922e0ddcad","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        # Each key denotes a \"current\" state for the image. Corresponding"},{"line_number":115,"context_line":"        # values list the valid states to which we can jump from that \"current\""},{"line_number":116,"context_line":"        # state."},{"line_number":117,"context_line":"        None: (\u0027queued\u0027, \u0027saving\u0027, \u0027active\u0027),"},{"line_number":118,"context_line":"        \u0027queued\u0027: (\u0027saving\u0027, \u0027active\u0027, \u0027deleted\u0027),"},{"line_number":119,"context_line":"        \u0027saving\u0027: (\u0027active\u0027, \u0027killed\u0027, \u0027deleted\u0027),"},{"line_number":120,"context_line":"        \u0027active\u0027: (\u0027queued\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2Fgy8%3D","line":117,"updated":"2014-01-13 05:10:44.000000000","message":"The state machine is straighforward, but I\u0027d like confirm the status transitions: how an image transit from \u0027None\u0027 to \u0027saving\u0027 or \u0027active\u0027?, so as to update this chart http://docs.openstack.org/developer/glance/statuses.html Thanks.","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"f1a6226223e1d3a0c929de6f5e1cb25220e55bf0","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        # Each key denotes a \"current\" state for the image. Corresponding"},{"line_number":115,"context_line":"        # values list the valid states to which we can jump from that \"current\""},{"line_number":116,"context_line":"        # state."},{"line_number":117,"context_line":"        None: (\u0027queued\u0027, \u0027saving\u0027, \u0027active\u0027),"},{"line_number":118,"context_line":"        \u0027queued\u0027: (\u0027saving\u0027, \u0027active\u0027, \u0027deleted\u0027),"},{"line_number":119,"context_line":"        \u0027saving\u0027: (\u0027active\u0027, \u0027killed\u0027, \u0027deleted\u0027),"},{"line_number":120,"context_line":"        \u0027active\u0027: (\u0027queued\u0027, \u0027pending_delete\u0027, \u0027deleted\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2Ferc%3D","line":117,"in_reply_to":"AAAAUH%2F%2Fgy8%3D","updated":"2014-01-13 13:50:17.000000000","message":"Take a look at Zhi Yan\u0027s comments - Zhi prefers I remove this transition (part of removing the \u0027init_from_db\u0027 below in line 130).\n\nI\u0027ll submit a new patchset with your (and Zhi\u0027s) comments first (as soon as I can) and then we\u0027ll see whether or not the state diagram needs modifications, OK?","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a5a8d27f61ec5676eaafaba153bcae922e0ddcad","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        init_from_db \u003d kwargs.pop(\u0027from_db\u0027, False)"},{"line_number":130,"context_line":"        if init_from_db:"},{"line_number":131,"context_line":"            # Sinc the DB contains saved images, when it \"get()\"s images the"},{"line_number":132,"context_line":"            # initial state of the image may not conform to the state"},{"line_number":133,"context_line":"            # transition rules specified by \u0027valid_state_targets\u0027 above (e.g."},{"line_number":134,"context_line":"            # get a \"killed\" image). So when reconstructing Image domain"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2FgzQ%3D","line":131,"updated":"2014-01-13 05:10:44.000000000","message":"typo \u0027Since\u0027? BTW, how about using NOTE(koofoss): to indicate it\u0027s added by you :) ?\n\nBesides, I\u0027m not really conformatable for the design to add this weird \u0027from_db\u0027 parameter into Image __init__ method. I\u0027d like to see the other reviewers\u0027 comments.","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"f1a6226223e1d3a0c929de6f5e1cb25220e55bf0","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        init_from_db \u003d kwargs.pop(\u0027from_db\u0027, False)"},{"line_number":130,"context_line":"        if init_from_db:"},{"line_number":131,"context_line":"            # Sinc the DB contains saved images, when it \"get()\"s images the"},{"line_number":132,"context_line":"            # initial state of the image may not conform to the state"},{"line_number":133,"context_line":"            # transition rules specified by \u0027valid_state_targets\u0027 above (e.g."},{"line_number":134,"context_line":"            # get a \"killed\" image). So when reconstructing Image domain"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2FeuE%3D","line":131,"in_reply_to":"AAAAUH%2F%2FgzQ%3D","updated":"2014-01-13 13:50:17.000000000","message":"Yes, it\u0027s a typo. Thanks.\n\nI don\u0027t mind leaving behind my mark in the code base if others don\u0027t mind it ... I just thought others might mind it ;).\n\nRegarding the \u0027from_db\u0027 parameter - Zhi and I had a chat some days back - I will be removing this (haven\u0027t had time for the next patchset yet, hopefully soon), so it should address your concern.","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"a5a8d27f61ec5676eaafaba153bcae922e0ddcad","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            kw \u003d {\u0027cur_status\u0027: cur_status, \u0027new_status\u0027: status}"},{"line_number":172,"context_line":"            e \u003d exception.InvalidImageStatusTransition(**kw)"},{"line_number":173,"context_line":"            LOG.debug(e)"},{"line_number":174,"context_line":"            raise e"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if (has_status and self._status \u003d\u003d \u0027queued\u0027 and"},{"line_number":177,"context_line":"                status in (\u0027saving\u0027, \u0027active\u0027)):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2Fgy0%3D","line":174,"updated":"2014-01-13 05:10:44.000000000","message":"How about using save_and_reraise_exception?","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"f1a6226223e1d3a0c929de6f5e1cb25220e55bf0","unresolved":false,"context_lines":[{"line_number":171,"context_line":"            kw \u003d {\u0027cur_status\u0027: cur_status, \u0027new_status\u0027: status}"},{"line_number":172,"context_line":"            e \u003d exception.InvalidImageStatusTransition(**kw)"},{"line_number":173,"context_line":"            LOG.debug(e)"},{"line_number":174,"context_line":"            raise e"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"        if (has_status and self._status \u003d\u003d \u0027queued\u0027 and"},{"line_number":177,"context_line":"                status in (\u0027saving\u0027, \u0027active\u0027)):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2Fetw%3D","line":174,"in_reply_to":"AAAAUH%2F%2Fgy0%3D","updated":"2014-01-13 13:50:17.000000000","message":"But we\u0027re not *reraising* an exception here - we\u0027re raising it for the first time, so I don\u0027t think we can use save_and_reraise_exception, right?","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"}],"glance/store/__init__.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"f2ed2df79fd0e2d01264deb7ad66ebfca3668748","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            image, member_repo_proxy_class\u003dImageMemberRepoProxy,"},{"line_number":650,"context_line":"            member_repo_proxy_kwargs\u003dproxy_kwargs)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def delete(self, _ignored\u003dFalse):"},{"line_number":653,"context_line":"        self.image.delete(CONF.delayed_delete)"},{"line_number":654,"context_line":"        if self.image.locations:"},{"line_number":655,"context_line":"            for location in self.image.locations:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUH%2F%2B%2BHE%3D","line":652,"updated":"2014-01-20 09:14:17.000000000","message":"So tbh, i think here is a little ugly/strange sorry, why we not just go original way: check CONF.delayed_delete in domain. And I think that is better than this even it need a flag to prevent loop importing.\n\nFurthermore, \"_ignored\" is wrong IMO it should be named to \"delayed_delete\", since actually they are two different parameters for \"delete()\". (seems you are assuming calling side code always not use named argument pass-in.)\n\nWhat you think?\n\nthanks.","commit_id":"52ce3d5c9d033b4b301c9fb4edb539d4b18113a6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"7e4eb5e3d592efc1b4975352917ad8b76a2ed174","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            image, member_repo_proxy_class\u003dImageMemberRepoProxy,"},{"line_number":650,"context_line":"            member_repo_proxy_kwargs\u003dproxy_kwargs)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def delete(self, _ignored\u003dFalse):"},{"line_number":653,"context_line":"        self.image.delete(CONF.delayed_delete)"},{"line_number":654,"context_line":"        if self.image.locations:"},{"line_number":655,"context_line":"            for location in self.image.locations:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUH%2F%2B88U%3D","line":652,"in_reply_to":"AAAAUH%2F%2B%2BHE%3D","updated":"2014-01-20 13:41:19.000000000","message":"The newest version of domain/__init__.py (I rebased the patchset) has deleted CONF and all configuration items so I tried to be as \"compatible\" with that change as I could.\n\nRegarding the parameter name ... yeah you\u0027re right - I forgot about keyword arguments. I was thinking about somebody commenting the parameter is not used so I deliberately named it so. I\u0027ll change it back (if this is the way to go).\n\nSo do you think I should re-introduce CONF back into domain/__init__.py?","commit_id":"52ce3d5c9d033b4b301c9fb4edb539d4b18113a6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"d39e7fa5d87e0dda0e7f4d22734f47a50376827b","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            image, member_repo_proxy_class\u003dImageMemberRepoProxy,"},{"line_number":650,"context_line":"            member_repo_proxy_kwargs\u003dproxy_kwargs)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def delete(self, _ignored\u003dFalse):"},{"line_number":653,"context_line":"        self.image.delete(CONF.delayed_delete)"},{"line_number":654,"context_line":"        if self.image.locations:"},{"line_number":655,"context_line":"            for location in self.image.locations:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUH%2F%2B3rg%3D","line":652,"in_reply_to":"AAAAUH%2F%2B3sM%3D","updated":"2014-01-21 09:07:44.000000000","message":"OK, then I will pull the CONF back into domain.","commit_id":"52ce3d5c9d033b4b301c9fb4edb539d4b18113a6"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"d5810a5fa809608e5781ae23290e4e3ddbd20510","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            image, member_repo_proxy_class\u003dImageMemberRepoProxy,"},{"line_number":650,"context_line":"            member_repo_proxy_kwargs\u003dproxy_kwargs)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def delete(self, _ignored\u003dFalse):"},{"line_number":653,"context_line":"        self.image.delete(CONF.delayed_delete)"},{"line_number":654,"context_line":"        if self.image.locations:"},{"line_number":655,"context_line":"            for location in self.image.locations:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAUH%2F%2B3sM%3D","line":652,"in_reply_to":"AAAAUH%2F%2B88U%3D","updated":"2014-01-21 09:04:23.000000000","message":"Yes, I personally like go that way but change domain.delete() interface signature. Because I think former is more self-contains.","commit_id":"52ce3d5c9d033b4b301c9fb4edb539d4b18113a6"}],"glance/tests/functional/v2/test_images.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"684103de495c29ad0222fd579c349e2275b1bc94","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            self.assertEqual(expected_checksum, image[\u0027checksum\u0027])"},{"line_number":322,"context_line":"            self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        verify_image_data()"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"        # Try to download the data that was just uploaded"},{"line_number":327,"context_line":"        path \u003d self._url(\u0027/v2/images/%s/file\u0027 % image_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2FrSU%3D","line":324,"updated":"2014-01-03 17:29:30.000000000","message":"see below comments first, and I have same idea at here also, says move this internal helper verify_image_data() function to line #308 where on the top of this (relatively) complete testing block,\n\nnit: btw, what about rename and do a little change on this helper: I prefer to adding a \"_\" prefix and rename it to \"_verify_image_checksum_status(checksum, status)? since IMO it\u0027s an internal helper (so \"_\" prefix), and I think the new name is better in the context.","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"a9704ad688c56c5b80038bb28c820a2192510b6b","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            self.assertEqual(expected_checksum, image[\u0027checksum\u0027])"},{"line_number":322,"context_line":"            self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        verify_image_data()"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"        # Try to download the data that was just uploaded"},{"line_number":327,"context_line":"        path \u003d self._url(\u0027/v2/images/%s/file\u0027 % image_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F%2BOA%3D","line":324,"in_reply_to":"AAAAT3%2F%2FrSU%3D","updated":"2014-01-06 01:32:47.000000000","message":"I\u0027ll do that.\n\nThis was the first patchset I submitted so I was being very careful to only touch those things that affected the commit ... I can be a little braver now :).","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"591d9915d81fe0b7fae390be621f52112d3a5ff6","unresolved":false,"context_lines":[{"line_number":321,"context_line":"            self.assertEqual(expected_checksum, image[\u0027checksum\u0027])"},{"line_number":322,"context_line":"            self.assertEqual(\u0027active\u0027, image[\u0027status\u0027])"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        verify_image_data()"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"        # Try to download the data that was just uploaded"},{"line_number":327,"context_line":"        path \u003d self._url(\u0027/v2/images/%s/file\u0027 % image_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F9%2B8%3D","line":324,"in_reply_to":"AAAAUH%2F%2F%2BOA%3D","updated":"2014-01-06 03:50:22.000000000","message":"cool, thanks.","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"684103de495c29ad0222fd579c349e2275b1bc94","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        headers \u003d self._headers({\u0027Content-Type\u0027: \u0027application/octet-stream\u0027})"},{"line_number":338,"context_line":"        response \u003d requests.put(path, headers\u003dheaders, data\u003d\u0027XXX\u0027)"},{"line_number":339,"context_line":"        self.assertEqual(409, response.status_code)"},{"line_number":340,"context_line":"        # ... and check that the original data has remained untouched"},{"line_number":341,"context_line":"        verify_image_data()"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        # Ensure the size is updated to reflect the data uploaded"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2FrWM%3D","line":340,"updated":"2014-01-03 17:29:30.000000000","message":"could you pls reword and merge this sentence to above comments at #335 which belongs with this testing block?","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"a9704ad688c56c5b80038bb28c820a2192510b6b","unresolved":false,"context_lines":[{"line_number":337,"context_line":"        headers \u003d self._headers({\u0027Content-Type\u0027: \u0027application/octet-stream\u0027})"},{"line_number":338,"context_line":"        response \u003d requests.put(path, headers\u003dheaders, data\u003d\u0027XXX\u0027)"},{"line_number":339,"context_line":"        self.assertEqual(409, response.status_code)"},{"line_number":340,"context_line":"        # ... and check that the original data has remained untouched"},{"line_number":341,"context_line":"        verify_image_data()"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        # Ensure the size is updated to reflect the data uploaded"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F%2BN4%3D","line":340,"in_reply_to":"AAAAT3%2F%2FrWM%3D","updated":"2014-01-06 01:32:47.000000000","message":"Done","commit_id":"1579c65d5951b581720618b7662f26dd4cf3cd44"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"b3894aa34443f2453868736e02f315e1ae3650c4","unresolved":false,"context_lines":[{"line_number":349,"context_line":"        response \u003d requests.get(path, headers\u003dheaders)"},{"line_number":350,"context_line":"        self.assertEqual(204, response.status_code)"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        self._data_upload_subtest(image_id)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"        # `disk_format` and `container_format` cannot"},{"line_number":355,"context_line":"        # be replaced when the image is active."}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAAUH%2F%2F76Q%3D","line":352,"updated":"2014-01-06 15:40:20.000000000","message":"IMO this change is a little overkill, I prefer we organize this test progress in the case as flat and real as possible. To extract \"_verify_image_checksum_and_status()\" helper out is good to me, but move above four sub-steps (line #68-91) out from this \"test_image_lifecycle()\" case is useless, on one hand \"_data_upload_subtest()\" code snippets does not be reused in any other place and it break this image lifecycle test case at here on the other.","commit_id":"aeaac90674914c5196fd3d0f7fa243870d61714e"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"1aa8996bb927108553b629d9826f93aa874806ea","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        _verify_image_checksum_and_status(expected_checksum, \u0027active\u0027)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        # Ensure the size is updated to reflect the data uploaded"},{"line_number":361,"context_line":"        path \u003d self._url(image_path)"},{"line_number":362,"context_line":"        response \u003d requests.get(path, headers\u003dself._headers())"},{"line_number":363,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":364,"context_line":"        self.assertEqual(5, json.loads(response.text)[\u0027size\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2F4j4%3D","line":361,"updated":"2014-01-07 09:53:42.000000000","message":"nit: IMO it\u0027s an unrelated change, just like line #337, #367 and below lines, not a big issue, but I advise to revert it.","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"f1a6226223e1d3a0c929de6f5e1cb25220e55bf0","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        _verify_image_checksum_and_status(expected_checksum, \u0027active\u0027)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        # Ensure the size is updated to reflect the data uploaded"},{"line_number":361,"context_line":"        path \u003d self._url(image_path)"},{"line_number":362,"context_line":"        response \u003d requests.get(path, headers\u003dself._headers())"},{"line_number":363,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":364,"context_line":"        self.assertEqual(5, json.loads(response.text)[\u0027size\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAUH%2F%2FerM%3D","line":361,"in_reply_to":"AAAAUH%2F%2F4j4%3D","updated":"2014-01-13 13:50:17.000000000","message":"Ok.","commit_id":"143a8107c786dba3b3e903a00c3e1f4011685be6"}],"glance/tests/unit/test_store_image.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"684103de495c29ad0222fd579c349e2275b1bc94","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":96,"context_line":"                          self.store_api.get_from_backend, {}, location[\u0027url\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def test_image_delayed_delete(self):"},{"line_number":99,"context_line":"        self.config(delayed_delete\u003dTrue)"},{"line_number":100,"context_line":"        image \u003d glance.store.ImageProxy(self.image_stub, {}, self.store_api)"},{"line_number":101,"context_line":"        self.assertEqual(image.status, \u0027active\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2FrOw%3D","side":"PARENT","line":98,"updated":"2014-01-03 17:29:30.000000000","message":"Can you give a short explaining why you need remove this stuff? since this is test case is make sense to me, thanks.","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"a9704ad688c56c5b80038bb28c820a2192510b6b","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":96,"context_line":"                          self.store_api.get_from_backend, {}, location[\u0027url\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def test_image_delayed_delete(self):"},{"line_number":99,"context_line":"        self.config(delayed_delete\u003dTrue)"},{"line_number":100,"context_line":"        image \u003d glance.store.ImageProxy(self.image_stub, {}, self.store_api)"},{"line_number":101,"context_line":"        self.assertEqual(image.status, \u0027active\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F%2BNI%3D","side":"PARENT","line":98,"in_reply_to":"AAAAT3%2F%2FrOw%3D","updated":"2014-01-06 01:32:47.000000000","message":"Because the store doesn\u0027t set the \u0027delayed_delete\u0027 flag any more - it\u0027s set directly in the \"status\" setter function.\n\nOn hindsight I should move this out of the the \"store\" test suite and into another \"domain\" test suite (I\u0027ll find the right file and move it there).","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"591d9915d81fe0b7fae390be621f52112d3a5ff6","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":96,"context_line":"                          self.store_api.get_from_backend, {}, location[\u0027url\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def test_image_delayed_delete(self):"},{"line_number":99,"context_line":"        self.config(delayed_delete\u003dTrue)"},{"line_number":100,"context_line":"        image \u003d glance.store.ImageProxy(self.image_stub, {}, self.store_api)"},{"line_number":101,"context_line":"        self.assertEqual(image.status, \u0027active\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F9%2Bs%3D","side":"PARENT","line":98,"in_reply_to":"AAAAUH%2F%2F%2BNI%3D","updated":"2014-01-06 03:50:22.000000000","message":"Yes, it will be necessary.","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"}],"glance/tests/unit/v2/test_image_data_resource.py":[{"author":{"_account_id":5347,"name":"Eddie Sheffield","email":"eddie.sheffield@rackspace.com","username":"eddie-sheffield"},"change_message_id":"4f0400929fd2241ee07fdbd370eff63fbae7201a","unresolved":false,"context_lines":[{"line_number":192,"context_line":"    def test_upload_data_exists(self):"},{"line_number":193,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":194,"context_line":"        image \u003d FakeImage()"},{"line_number":195,"context_line":"        image.set_data \u003d Raise(webob.exc.HTTPConflict)"},{"line_number":196,"context_line":"        self.image_repo.result \u003d image"},{"line_number":197,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":198,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F94NI%3D","line":195,"updated":"2013-12-04 23:11:51.000000000","message":"This line isn\u0027t required now since the exception is raised by your new code, not the set_data method call.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"6892364d97af6fbb70d63b879fd2b6735f313210","unresolved":false,"context_lines":[{"line_number":192,"context_line":"    def test_upload_data_exists(self):"},{"line_number":193,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":194,"context_line":"        image \u003d FakeImage()"},{"line_number":195,"context_line":"        image.set_data \u003d Raise(webob.exc.HTTPConflict)"},{"line_number":196,"context_line":"        self.image_repo.result \u003d image"},{"line_number":197,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":198,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F93bQ%3D","line":195,"in_reply_to":"AAAATn%2F94NI%3D","updated":"2013-12-05 03:11:32.000000000","message":"Aah, good catch. Thanks.","commit_id":"f817acc716745fb1559608d4e602f442d65c14b3"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"684103de495c29ad0222fd579c349e2275b1bc94","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    def test_upload_data_exists(self):"},{"line_number":200,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":201,"context_line":"        image \u003d FakeImage()"},{"line_number":202,"context_line":"        image.set_data \u003d Raise(exception.Duplicate)"},{"line_number":203,"context_line":"        self.image_repo.result \u003d image"},{"line_number":204,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":205,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAT3%2F%2FrLM%3D","side":"PARENT","line":202,"updated":"2014-01-03 17:29:30.000000000","message":"TBH I have to say the new InvalidImageStatusTransition exception is not much intuitive then original Duplicate exception for this duplicated upload issue for end user, but seems we have not a good way to tell end user an intuitive exception from his perspective base on a wrong image status transition issue only, it\u0027s really hard to conjecture a correct exception type/message which would be intuitive enough and easy to understand for end user, you know image status transition is too low to hard to get a high level recapitulative exception from end user\u0027s perspective.\n\nany thoughts from other reviewers?","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"a9704ad688c56c5b80038bb28c820a2192510b6b","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    def test_upload_data_exists(self):"},{"line_number":200,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":201,"context_line":"        image \u003d FakeImage()"},{"line_number":202,"context_line":"        image.set_data \u003d Raise(exception.Duplicate)"},{"line_number":203,"context_line":"        self.image_repo.result \u003d image"},{"line_number":204,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":205,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F%2BMs%3D","side":"PARENT","line":202,"in_reply_to":"AAAAT3%2F%2FrLM%3D","updated":"2014-01-06 01:32:47.000000000","message":"Yes - that was the problem I faced when moving the status check into the status setter function :).\n\nBut I\u0027m thinking ... perhaps there\u0027s another way out (I mentioned this in my mail to the mailing list a few days back) - what if we enhance the GlanceException class to not just have the error message but also to preserve the error data (i.e. keep the \"args\" and \"kwargs\" arguments). Then, in the api layer we can analyze the args and decide what HTTP status to return to the end user.\n\nThat (IMHO) should be a neat approach - keep the state transition checks in domain layer and the API layer only checks the exception data to determine what HTTP status to return to the end user (no domain operations).\n\nWhat do you think?","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"591d9915d81fe0b7fae390be621f52112d3a5ff6","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    def test_upload_data_exists(self):"},{"line_number":200,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":201,"context_line":"        image \u003d FakeImage()"},{"line_number":202,"context_line":"        image.set_data \u003d Raise(exception.Duplicate)"},{"line_number":203,"context_line":"        self.image_repo.result \u003d image"},{"line_number":204,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":205,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F99k%3D","side":"PARENT","line":202,"in_reply_to":"AAAAUH%2F%2F%2BMs%3D","updated":"2014-01-06 03:50:22.000000000","message":"Do you think we can get a correct exception message/type for end use only via a \"args\" and \"kwargs\"? or say the key for your idea, IMO if I understand correct, is that what \"error data\" should be added to  GlanceException, and how to make that \"error data\" has a general structure (since IMO api layer need a general parsing logic)","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"},{"author":{"_account_id":9520,"name":"David Koo (koofoss)","email":"david.koo@huawei.com","username":"koofoss"},"change_message_id":"0c2417830d9b6886c556cc39e898c89953af399a","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    def test_upload_data_exists(self):"},{"line_number":200,"context_line":"        request \u003d unit_test_utils.get_fake_request()"},{"line_number":201,"context_line":"        image \u003d FakeImage()"},{"line_number":202,"context_line":"        image.set_data \u003d Raise(exception.Duplicate)"},{"line_number":203,"context_line":"        self.image_repo.result \u003d image"},{"line_number":204,"context_line":"        self.assertRaises(webob.exc.HTTPConflict, self.controller.upload,"},{"line_number":205,"context_line":"                          request, unit_test_utils.UUID1, \u0027YYYY\u0027, 4)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2F9j8%3D","side":"PARENT","line":202,"in_reply_to":"AAAAUH%2F%2F99k%3D","updated":"2014-01-06 06:45:36.000000000","message":"Hmm, you\u0027re right - the API layer can\u0027t do it without knowledge of how to handle the state machine (which it is not allowed to have).\n\nAnd the domain layer doesn\u0027t know what operation is being executed and so can\u0027t throw a more specific error.\n\nI had thought of adding another \u0027set_status()\u0027 method to domain.Image and pass the operation as a parameter to this method (e.g. image.set_status(\u0027saving\u0027, \u0027upload\u0027)) and enforce the state transitions inside this set_status method.\n\nBut I think it looks so ugly that I\u0027m sure it will be -1ed instantly, no? :).","commit_id":"f54de77ee5cee25e9626f668332f0f852f5308f4"}]}