)]}'
{"glance/store/rbd.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"bba843b575197e95d8a9979fab254ca953a1b471","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            with conn.open_ioctx(self.pool) as ioctx:"},{"line_number":264,"context_line":"                try:"},{"line_number":265,"context_line":"                    # First remove snapshot."},{"line_number":266,"context_line":"                    if snapshot_name is not None:"},{"line_number":267,"context_line":"                        with rbd.Image(ioctx, image_name) as image:"},{"line_number":268,"context_line":"                            try:"},{"line_number":269,"context_line":"                                image.unprotect_snap(snapshot_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTrE%3D","line":266,"updated":"2013-09-30 14:49:43.000000000","message":"Why we need change this, IMO empty snapshot name should skip also, if caller pass it in here.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"e29f936deda57338fa39ebfc4de7c7a2379e8b80","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            with conn.open_ioctx(self.pool) as ioctx:"},{"line_number":264,"context_line":"                try:"},{"line_number":265,"context_line":"                    # First remove snapshot."},{"line_number":266,"context_line":"                    if snapshot_name is not None:"},{"line_number":267,"context_line":"                        with rbd.Image(ioctx, image_name) as image:"},{"line_number":268,"context_line":"                            try:"},{"line_number":269,"context_line":"                                image.unprotect_snap(snapshot_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTD0%3D","line":266,"in_reply_to":"AAAAS3%2F%2FTE4%3D","updated":"2013-09-30 16:51:40.000000000","message":"OK, i\u0027m fine. I just want to go ignore way to allow this function support empty snapshot name pass-in.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"3586e8d2fd8668bfd2c5385544b18ac43fdfa344","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            with conn.open_ioctx(self.pool) as ioctx:"},{"line_number":264,"context_line":"                try:"},{"line_number":265,"context_line":"                    # First remove snapshot."},{"line_number":266,"context_line":"                    if snapshot_name is not None:"},{"line_number":267,"context_line":"                        with rbd.Image(ioctx, image_name) as image:"},{"line_number":268,"context_line":"                            try:"},{"line_number":269,"context_line":"                                image.unprotect_snap(snapshot_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTE4%3D","line":266,"in_reply_to":"AAAAS3%2F%2FTJQ%3D","updated":"2013-09-30 16:48:34.000000000","message":"Zhi, apologies but it is not clear what you are trying to say here. We should not be allowing empty string names here. If one is passed in, this should raise an error. Right now if one is passed in it just gets ignored which I think is incorrect. If you want to allow empty string names then don\u0027t ignore them. If you don\u0027t want to allow them then raise an error.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"2b655ced019a2c26a808268dd81929329f90583c","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            with conn.open_ioctx(self.pool) as ioctx:"},{"line_number":264,"context_line":"                try:"},{"line_number":265,"context_line":"                    # First remove snapshot."},{"line_number":266,"context_line":"                    if snapshot_name is not None:"},{"line_number":267,"context_line":"                        with rbd.Image(ioctx, image_name) as image:"},{"line_number":268,"context_line":"                            try:"},{"line_number":269,"context_line":"                                image.unprotect_snap(snapshot_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTJQ%3D","line":266,"in_reply_to":"AAAAS3%2F%2FTkA%3D","updated":"2013-09-30 16:35:50.000000000","message":"I can\u0027t but keep function work is the responsibility of this code context. so take care empty string for snapshot name is reasonable to me.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"0013ee760d10dd1d227e937403ae87b82873cdb7","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            with conn.open_ioctx(self.pool) as ioctx:"},{"line_number":264,"context_line":"                try:"},{"line_number":265,"context_line":"                    # First remove snapshot."},{"line_number":266,"context_line":"                    if snapshot_name is not None:"},{"line_number":267,"context_line":"                        with rbd.Image(ioctx, image_name) as image:"},{"line_number":268,"context_line":"                            try:"},{"line_number":269,"context_line":"                                image.unprotect_snap(snapshot_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTkA%3D","line":266,"in_reply_to":"AAAAS3%2F%2FTrE%3D","updated":"2013-09-30 15:09:27.000000000","message":"I think if an empty snapshot name is passed in that would be an error case. Can you think of a valid reason why an empty string would be passed in?","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"bba843b575197e95d8a9979fab254ca953a1b471","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                    try:"},{"line_number":335,"context_line":"                        self._delete_image(loc.image, loc.snapshot)"},{"line_number":336,"context_line":"                    except exception.NotFound:"},{"line_number":337,"context_line":"                        pass"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"                    raise exc"},{"line_number":340,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTr8%3D","line":337,"updated":"2013-09-30 14:49:43.000000000","message":"Humm, this try-catch style just like I did in https://review.openstack.org/#/c/42896/4/glance/store/rbd.py, but markwash didn\u0027t like it. For now it seems more reasonable.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"2b655ced019a2c26a808268dd81929329f90583c","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                    try:"},{"line_number":335,"context_line":"                        self._delete_image(loc.image, loc.snapshot)"},{"line_number":336,"context_line":"                    except exception.NotFound:"},{"line_number":337,"context_line":"                        pass"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"                    raise exc"},{"line_number":340,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTIc%3D","line":337,"in_reply_to":"AAAAS3%2F%2FTjU%3D","updated":"2013-09-30 16:35:50.000000000","message":"I just put my recent work record here, and think your change is reasonable. But btw I don\u0027t think save_and_reraise() is just such workaround for something, it\u0027s worth util function to me.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"0013ee760d10dd1d227e937403ae87b82873cdb7","unresolved":false,"context_lines":[{"line_number":334,"context_line":"                    try:"},{"line_number":335,"context_line":"                        self._delete_image(loc.image, loc.snapshot)"},{"line_number":336,"context_line":"                    except exception.NotFound:"},{"line_number":337,"context_line":"                        pass"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"                    raise exc"},{"line_number":340,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAS3%2F%2FTjU%3D","line":337,"in_reply_to":"AAAAS3%2F%2FTr8%3D","updated":"2013-09-30 15:09:27.000000000","message":"AFAIK the save_and_reraise calls were a work around for an issue in python-eventlet - https://bitbucket.org/eventlet/eventlet/issue/149/yield-in-except-clause-with-wilcard-raise\n\n(1) that issue has been fixed\n(2) python-eventlet is not actually being used here\n\nI have tested this patch and it works fine.","commit_id":"8679e2ac5661ab43acb8fda6864e0d4601c886ef"}],"glance/tests/unit/test_rbd_store.py":[{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"9ef8310dd95409638ccb59324cd9830fb9e95f19","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        def _fake_image_write(*args, **kwargs):"},{"line_number":56,"context_line":"            called_commands.append(\u0027write\u0027)"},{"line_number":57,"context_line":"            return FAKE_CHUNKSIZE"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        def _fake_enter(*args, **kwargs):"},{"line_number":60,"context_line":"            raise exception.NotFound(\"\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FRuU%3D","line":57,"updated":"2013-10-01 02:49:10.000000000","message":"undefined FAKE_CHUNKSIZE","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"99a2f517e367e4d1ef736880ac34c2abae7a0885","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        def _fake_image_write(*args, **kwargs):"},{"line_number":56,"context_line":"            called_commands.append(\u0027write\u0027)"},{"line_number":57,"context_line":"            return FAKE_CHUNKSIZE"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        def _fake_enter(*args, **kwargs):"},{"line_number":60,"context_line":"            raise exception.NotFound(\"\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FQo8%3D","line":57,"in_reply_to":"AAAAS3%2F%2FRuU%3D","updated":"2013-10-01 11:49:12.000000000","message":"Done","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"9ef8310dd95409638ccb59324cd9830fb9e95f19","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        image_file \u003d utils.LimitingReader(StringIO.StringIO(\u0027xx\u0027), 1)"},{"line_number":68,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":69,"context_line":"                          \u0027fake_image_id\u0027, image_file, 2)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        self.assertEqual(called_commands, [\u0027create\u0027, \u0027delete\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FRtE%3D","line":68,"updated":"2013-10-01 02:49:10.000000000","message":"The refactoring for this function is ok to me, but could you add a new test case to cover NotFound exception? since as in my old code I want this test case cover store cleanup logic for ImageSizeLimitExceeded exception (LimitingReader railed).\n\nIf you still like change this test case directly, then I don\u0027t think image_file need wrapped by LimitingReader, it can be a StringIO object, since the (NotFound) exception raise by Image.__entrer__(), and it will not trigger ImageSizeLimitExceeded exception before reach your _fake_enter() function.","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"99a2f517e367e4d1ef736880ac34c2abae7a0885","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        image_file \u003d utils.LimitingReader(StringIO.StringIO(\u0027xx\u0027), 1)"},{"line_number":68,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":69,"context_line":"                          \u0027fake_image_id\u0027, image_file, 2)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        self.assertEqual(called_commands, [\u0027create\u0027, \u0027delete\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FQo0%3D","line":68,"in_reply_to":"AAAAS3%2F%2FRtE%3D","updated":"2013-10-01 11:49:12.000000000","message":"Done","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"9ef8310dd95409638ccb59324cd9830fb9e95f19","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        self.assertEqual(called_commands, [\u0027create\u0027])"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def tearDown(self):"},{"line_number":88,"context_line":"        self.stubs.UnsetAll"},{"line_number":89,"context_line":"        super(TestStore, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FRso%3D","line":87,"updated":"2013-10-01 02:49:10.000000000","message":"IMO we don\u0027t need such cleanup code explicitly here. https://github.com/openstack/glance/blob/master/glance/tests/utils.py#L60","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"99a2f517e367e4d1ef736880ac34c2abae7a0885","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        self.assertEqual(called_commands, [\u0027create\u0027])"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def tearDown(self):"},{"line_number":88,"context_line":"        self.stubs.UnsetAll"},{"line_number":89,"context_line":"        super(TestStore, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAS3%2F%2FQow%3D","line":87,"in_reply_to":"AAAAS3%2F%2FRso%3D","updated":"2013-10-01 11:49:12.000000000","message":"Done","commit_id":"f8cee386b2e056531e05368b6904ab6b6110c88f"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"6254449b5f53226693fcca24cac1a1511faa7575","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.stubs.Set(self.store, \u0027_delete_image\u0027, _fake_delete_image)"},{"line_number":54,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":57,"context_line":"                          \u0027fake_image_id\u0027, StringIO.StringIO(\u0027xx\u0027), 2)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        self.called_commands_expected \u003d [\u0027create\u0027, \u0027delete\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAS3%2F%2FODc%3D","line":56,"updated":"2013-10-02 02:25:15.000000000","message":"Thanks for adding those test cases for your change!\n\nOne suggestion (not a blocker), do you think it will be better if we add a test case for \u0027ImageSizeLimitExceeded\u0027 exception? to verify RBD store can delete partial uploaded image when uploading progress meetImageSizeLimitExceeded exception (but not only check NotFound).","commit_id":"d72cb94ded0e76528cbe262f768a6113ad67ce80"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"0bdd55316707fdf4abe00c3f87ede91ef97dcd03","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.stubs.Set(self.store, \u0027_delete_image\u0027, _fake_delete_image)"},{"line_number":54,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":57,"context_line":"                          \u0027fake_image_id\u0027, StringIO.StringIO(\u0027xx\u0027), 2)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        self.called_commands_expected \u003d [\u0027create\u0027, \u0027delete\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2F7e8%3D","line":56,"in_reply_to":"AAAAS3%2F%2FM7c%3D","updated":"2013-10-07 22:19:18.000000000","message":"Zhiyan,\n\nTurns out your original test was probably never getting run since it was Skipping if rbd was not installed. Also, the rbd store add() cannot result in a SizeLimitExceeded exception since it uses utils.chunkreadable which only returns the exact number of bytes extant. I have added in a mock that generates a sizeLimitExceeded exception just to prove it does get handled in the expected way.","commit_id":"d72cb94ded0e76528cbe262f768a6113ad67ce80"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"debd34df7485cbf44eb9dac38ff8269fe79c9372","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.stubs.Set(self.store, \u0027_delete_image\u0027, _fake_delete_image)"},{"line_number":54,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":57,"context_line":"                          \u0027fake_image_id\u0027, StringIO.StringIO(\u0027xx\u0027), 2)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        self.called_commands_expected \u003d [\u0027create\u0027, \u0027delete\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAS3%2F%2FM7c%3D","line":56,"in_reply_to":"AAAAS3%2F%2FODc%3D","updated":"2013-10-02 13:11:44.000000000","message":"I\u0027ll see if I can get that in.","commit_id":"d72cb94ded0e76528cbe262f768a6113ad67ce80"},{"author":{"_account_id":6549,"name":"Zhi Yan Liu","email":"lzy.dev@gmail.com","username":"lzy-dev"},"change_message_id":"c28257a0f18ab83080084d124371cc837167b433","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.stubs.Set(self.store, \u0027_delete_image\u0027, _fake_delete_image)"},{"line_number":54,"context_line":"        self.stubs.Set(mock_rbd.Image, \u0027__enter__\u0027, _fake_enter)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        self.assertRaises(exception.NotFound, self.store.add,"},{"line_number":57,"context_line":"                          \u0027fake_image_id\u0027, StringIO.StringIO(\u0027xx\u0027), 2)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"        self.called_commands_expected \u003d [\u0027create\u0027, \u0027delete\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2F5ls%3D","line":56,"in_reply_to":"AAAATH%2F%2F7e8%3D","updated":"2013-10-08 10:38:31.000000000","message":"Cool, thanks.","commit_id":"d72cb94ded0e76528cbe262f768a6113ad67ce80"}]}