)]}'
{"cinder/objects/volume.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"be7acc4e655058c74903f6b01afe88bda43f34eb","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            if metadata:"},{"line_number":189,"context_line":"                volume.glance_metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":190,"context_line":"                                          for item in metadata}"},{"line_number":191,"context_line":"        if \u0027volume_type\u0027 in expected_attrs:"},{"line_number":192,"context_line":"            db_volume_type \u003d db_volume.get(\u0027volume_type\u0027)"},{"line_number":193,"context_line":"            if db_volume_type:"},{"line_number":194,"context_line":"                volume.volume_type \u003d objects.VolumeType._from_db_object("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_ad6c2037","line":191,"updated":"2015-12-09 20:27:19.000000000","message":"If we\u0027re changing obj_load_attr then we should also make similar changes here.","commit_id":"4a31668eb0c82e06e693398441056672eb4df2cc"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"13445fb448055712060cbc6519354baa97cb63a5","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            if metadata:"},{"line_number":189,"context_line":"                volume.glance_metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":190,"context_line":"                                          for item in metadata}"},{"line_number":191,"context_line":"        if \u0027volume_type\u0027 in expected_attrs:"},{"line_number":192,"context_line":"            db_volume_type \u003d db_volume.get(\u0027volume_type\u0027)"},{"line_number":193,"context_line":"            if db_volume_type:"},{"line_number":194,"context_line":"                volume.volume_type \u003d objects.VolumeType._from_db_object("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_729a94c7","line":191,"in_reply_to":"7a740942_48780a90","updated":"2015-12-09 20:51:27.000000000","message":"Hm, you\u0027re right… Even if this will trigger SQLAlchemy lazy-load, it won\u0027t raise an exception.","commit_id":"4a31668eb0c82e06e693398441056672eb4df2cc"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"a41e722f51998003412b2c661261ec49dca66430","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            if metadata:"},{"line_number":189,"context_line":"                volume.glance_metadata \u003d {item[\u0027key\u0027]: item[\u0027value\u0027]"},{"line_number":190,"context_line":"                                          for item in metadata}"},{"line_number":191,"context_line":"        if \u0027volume_type\u0027 in expected_attrs:"},{"line_number":192,"context_line":"            db_volume_type \u003d db_volume.get(\u0027volume_type\u0027)"},{"line_number":193,"context_line":"            if db_volume_type:"},{"line_number":194,"context_line":"                volume.volume_type \u003d objects.VolumeType._from_db_object("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a740942_48780a90","line":191,"in_reply_to":"7a740942_ad6c2037","updated":"2015-12-09 20:35:50.000000000","message":"Do we actually need a similar check here?  It looks like for this path we\u0027re just checking if the volume_type is already in the db_volume that is returned and we\u0027re not actually querying based on volume_type_id here?","commit_id":"4a31668eb0c82e06e693398441056672eb4df2cc"}],"cinder/tests/unit/api/contrib/test_volume_actions.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5b8080e8df73b5e98247c402855383bd571fc34","unresolved":false,"context_lines":[{"line_number":983,"context_line":"        def stub_volume_get(self, context, volume_id):"},{"line_number":984,"context_line":"            volume \u003d stubs.stub_volume(volume_id)"},{"line_number":985,"context_line":"            volume[\u0027status\u0027] \u003d \u0027available\u0027"},{"line_number":986,"context_line":"            volume[\u0027volume_type\u0027] \u003d None"},{"line_number":987,"context_line":"            return volume"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        self.stubs.Set(volume_api.API, \u0027get\u0027, stub_volume_get)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_3a7745a1","line":986,"updated":"2015-12-08 20:39:58.000000000","message":"To have a correct regression test I think calling volume[\u0027volume_type\u0027] should raise an exception.","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"84e3015c2c366edc0c55d4586bc971c5a5282f90","unresolved":false,"context_lines":[{"line_number":983,"context_line":"        def stub_volume_get(self, context, volume_id):"},{"line_number":984,"context_line":"            volume \u003d stubs.stub_volume(volume_id)"},{"line_number":985,"context_line":"            volume[\u0027status\u0027] \u003d \u0027available\u0027"},{"line_number":986,"context_line":"            volume[\u0027volume_type\u0027] \u003d None"},{"line_number":987,"context_line":"            return volume"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        self.stubs.Set(volume_api.API, \u0027get\u0027, stub_volume_get)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_836b7084","line":986,"in_reply_to":"7a740942_3a7745a1","updated":"2015-12-08 21:57:14.000000000","message":"Do you have an idea to test this? Since the volume is fake dictionary volume instead of object, I didn\u0027t conceive an idea to raise an exception when volume[\u0027volume_type\u0027] was referred.","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5b8080e8df73b5e98247c402855383bd571fc34","unresolved":false,"context_lines":[{"line_number":986,"context_line":"            volume[\u0027volume_type\u0027] \u003d None"},{"line_number":987,"context_line":"            return volume"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        self.stubs.Set(volume_api.API, \u0027get\u0027, stub_volume_get)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        # Need to mock get_volume_image_metadata, create,"},{"line_number":992,"context_line":"        # update and copy_volume_to_image"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_f028ff9b","line":989,"updated":"2015-12-08 20:39:58.000000000","message":"Can\u0027t we just use mock instead of mox stubs? I don\u0027t think we should add tests relying on mox anymore.","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"84e3015c2c366edc0c55d4586bc971c5a5282f90","unresolved":false,"context_lines":[{"line_number":986,"context_line":"            volume[\u0027volume_type\u0027] \u003d None"},{"line_number":987,"context_line":"            return volume"},{"line_number":988,"context_line":""},{"line_number":989,"context_line":"        self.stubs.Set(volume_api.API, \u0027get\u0027, stub_volume_get)"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        # Need to mock get_volume_image_metadata, create,"},{"line_number":992,"context_line":"        # update and copy_volume_to_image"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_23f5dceb","line":989,"in_reply_to":"7a740942_f028ff9b","updated":"2015-12-08 21:57:14.000000000","message":"Forgive my copy and paste... Done","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c5b8080e8df73b5e98247c402855383bd571fc34","unresolved":false,"context_lines":[{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        # Need to mock get_volume_image_metadata, create,"},{"line_number":992,"context_line":"        # update and copy_volume_to_image"},{"line_number":993,"context_line":"        with mock.patch.object(volume_api.API, \"get_volume_image_metadata\") \\"},{"line_number":994,"context_line":"                as mock_get_volume_image_metadata:"},{"line_number":995,"context_line":"            mock_get_volume_image_metadata.side_effect \u003d \\"},{"line_number":996,"context_line":"                fake_get_volume_image_metadata"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_7a614dd2","line":993,"updated":"2015-12-08 20:39:58.000000000","message":"Wouldn\u0027t it be more readable with decorators instead of 4 context managers?","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"84e3015c2c366edc0c55d4586bc971c5a5282f90","unresolved":false,"context_lines":[{"line_number":990,"context_line":""},{"line_number":991,"context_line":"        # Need to mock get_volume_image_metadata, create,"},{"line_number":992,"context_line":"        # update and copy_volume_to_image"},{"line_number":993,"context_line":"        with mock.patch.object(volume_api.API, \"get_volume_image_metadata\") \\"},{"line_number":994,"context_line":"                as mock_get_volume_image_metadata:"},{"line_number":995,"context_line":"            mock_get_volume_image_metadata.side_effect \u003d \\"},{"line_number":996,"context_line":"                fake_get_volume_image_metadata"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a740942_63ef641e","line":993,"in_reply_to":"7a740942_7a614dd2","updated":"2015-12-08 21:57:14.000000000","message":"Done","commit_id":"d2ed84a06fef9d1f17f155eab9a36f3986eab8b5"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"749f6375f8a34d46621dc9afcb6271b1cca757b6","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                \"value\": \"246254365\"}"},{"line_number":991,"context_line":"            return meta_dict"},{"line_number":992,"context_line":""},{"line_number":993,"context_line":"        def volume_type_not_found_raise_exc(volume_type_id):"},{"line_number":994,"context_line":"            raise exception.VolumeTypeNotFound(volume_type_id\u003dvolume_type_id)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        # Set \u0027volume_type_id \u003d None\u0027 to get \u0027volume_type \u003d None\u0027."}],"source_content_type":"text/x-python","patch_set":4,"id":"7a740942_25c70b65","line":993,"updated":"2015-12-09 09:24:25.000000000","message":"Is this function used anywhere? You should probably mock DB API method for getting volume type with it (or even better to mock it and use side_effect).","commit_id":"9d305e0cf85fb3400bb09a76e65e95ff40323258"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"af8b1d8861c7057e4b5ab3caf3dd2baa1ff04708","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                \"value\": \"246254365\"}"},{"line_number":991,"context_line":"            return meta_dict"},{"line_number":992,"context_line":""},{"line_number":993,"context_line":"        def volume_type_not_found_raise_exc(volume_type_id):"},{"line_number":994,"context_line":"            raise exception.VolumeTypeNotFound(volume_type_id\u003dvolume_type_id)"},{"line_number":995,"context_line":""},{"line_number":996,"context_line":"        # Set \u0027volume_type_id \u003d None\u0027 to get \u0027volume_type \u003d None\u0027."}],"source_content_type":"text/x-python","patch_set":4,"id":"7a740942_2b3b439e","line":993,"in_reply_to":"7a740942_25c70b65","updated":"2015-12-09 17:19:24.000000000","message":"This was unnecessary code :(. \n\nI proposed a new patch which uses volume object instead of stub dict volume. \n\nUsing volume object, I confirmed if copy_volume_to_image_volume() touched volume_type when it was None, VolumeTypeNotFound exception was raised before the fix with this test case. And after applying the fix, VolumeTypeNotFound does not happen.","commit_id":"9d305e0cf85fb3400bb09a76e65e95ff40323258"}],"cinder/volume/api.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b88611061e4738ce7450317bd39ff7f028c32d40","unresolved":false,"context_lines":[{"line_number":1212,"context_line":"                    \"status\": \u0027uploading\u0027,"},{"line_number":1213,"context_line":"                    \"display_description\": volume[\u0027display_description\u0027],"},{"line_number":1214,"context_line":"                    \"size\": volume[\u0027size\u0027],"},{"line_number":1215,"context_line":"                    \"volume_type_id\": volume[\u0027volume_type_id\u0027],"},{"line_number":1216,"context_line":"                    \"image_id\": recv_metadata[\u0027id\u0027],"},{"line_number":1217,"context_line":"                    \"container_format\": recv_metadata[\u0027container_format\u0027],"},{"line_number":1218,"context_line":"                    \"disk_format\": recv_metadata[\u0027disk_format\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_91aaf383","line":1215,"updated":"2015-12-08 14:56:38.000000000","message":"We shouldn\u0027t probably change the key itself… I\u0027ve checked that in Liberty *and* Kilo we\u0027ve returned the dict as volume_type. In cinder show we\u0027re returning volume type name here…\n\nI would say we should stick to the old behavior. You should be able to check here for existence of volume_type parameter on versioned object somehow.","commit_id":"a848681b80a9198eeba670b9c946b9984658c932"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"b141ec88006f2e7d8337f3e3e59b66d3b45ec8da","unresolved":false,"context_lines":[{"line_number":1212,"context_line":"                    \"status\": \u0027uploading\u0027,"},{"line_number":1213,"context_line":"                    \"display_description\": volume[\u0027display_description\u0027],"},{"line_number":1214,"context_line":"                    \"size\": volume[\u0027size\u0027],"},{"line_number":1215,"context_line":"                    \"volume_type_id\": volume[\u0027volume_type_id\u0027],"},{"line_number":1216,"context_line":"                    \"image_id\": recv_metadata[\u0027id\u0027],"},{"line_number":1217,"context_line":"                    \"container_format\": recv_metadata[\u0027container_format\u0027],"},{"line_number":1218,"context_line":"                    \"disk_format\": recv_metadata[\u0027disk_format\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_22440bb5","line":1215,"in_reply_to":"7a740942_0c902894","updated":"2015-12-08 15:40:41.000000000","message":"Hi Michal, Thank you for your investigation. You are correct. I thought this couldn\u0027t return correct volume[\u0027volume_type\u0027] type in the past, but it was my misunderstanding. I agree that we should stick to the old behavior to keep API compatibility.\n\nHi Peter, I know touching volume[\u0027volume_type\u0027] raises VolumeTypeNotFound exception from DB when the volume doesn\u0027t have volume_type, but in this case we should check volume_type_id at first and then if volume_type_id is None, we can return volume_type is None too in the API response.\n\nI will double check whether this keeps API compatibility.","commit_id":"a848681b80a9198eeba670b9c946b9984658c932"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"89b22b8188a39391033e6803dd68375654dd4eed","unresolved":false,"context_lines":[{"line_number":1212,"context_line":"                    \"status\": \u0027uploading\u0027,"},{"line_number":1213,"context_line":"                    \"display_description\": volume[\u0027display_description\u0027],"},{"line_number":1214,"context_line":"                    \"size\": volume[\u0027size\u0027],"},{"line_number":1215,"context_line":"                    \"volume_type_id\": volume[\u0027volume_type_id\u0027],"},{"line_number":1216,"context_line":"                    \"image_id\": recv_metadata[\u0027id\u0027],"},{"line_number":1217,"context_line":"                    \"container_format\": recv_metadata[\u0027container_format\u0027],"},{"line_number":1218,"context_line":"                    \"disk_format\": recv_metadata[\u0027disk_format\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_d05ce31a","line":1215,"in_reply_to":"7a740942_22440bb5","updated":"2015-12-08 20:18:07.000000000","message":"Mitsuhiro: I totally agree. :)","commit_id":"a848681b80a9198eeba670b9c946b9984658c932"},{"author":{"_account_id":10628,"name":"Peter Wang","email":"peter.wang13@emc.com","username":"peter_wang"},"change_message_id":"eb788d76dbc3fdf70728bf1da8cab602661af125","unresolved":false,"context_lines":[{"line_number":1212,"context_line":"                    \"status\": \u0027uploading\u0027,"},{"line_number":1213,"context_line":"                    \"display_description\": volume[\u0027display_description\u0027],"},{"line_number":1214,"context_line":"                    \"size\": volume[\u0027size\u0027],"},{"line_number":1215,"context_line":"                    \"volume_type_id\": volume[\u0027volume_type_id\u0027],"},{"line_number":1216,"context_line":"                    \"image_id\": recv_metadata[\u0027id\u0027],"},{"line_number":1217,"context_line":"                    \"container_format\": recv_metadata[\u0027container_format\u0027],"},{"line_number":1218,"context_line":"                    \"disk_format\": recv_metadata[\u0027disk_format\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a740942_0c902894","line":1215,"in_reply_to":"7a740942_91aaf383","updated":"2015-12-08 15:06:56.000000000","message":"Hi all\nI\u0027d like to provide more info behind the bug filed. \n\nvolume[\u0027volume_type\u0027] would trigger a db call which raise an 404 not found error in my case (no volume type associated with the volume). so cinder client got 400 when using \u0027cinder upload-to-image\u0027\n\nvolume[\u0027volume_type_id\u0027] would not raise exception.\n\n\nThanks\nPeter","commit_id":"a848681b80a9198eeba670b9c946b9984658c932"},{"author":{"_account_id":8912,"name":"Ryan McNair","email":"rdmcnair@us.ibm.com","username":"rdmcnair"},"change_message_id":"3420cfc7e88fc0843af36f03d190a42196ce2672","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"        self.volume_rpcapi.copy_volume_to_image(context,"},{"line_number":1207,"context_line":"                                                volume,"},{"line_number":1208,"context_line":"                                                recv_metadata)"},{"line_number":1209,"context_line":""},{"line_number":1210,"context_line":"        # If the volume doesn\u0027t have volume_type, volume[\u0027volume_type\u0027]"},{"line_number":1211,"context_line":"        # would trigger a db call which raise VolumeTypeNotFound exception."},{"line_number":1212,"context_line":"        volume_type \u003d (volume[\u0027volume_type\u0027] if volume[\u0027volume_type_id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a740942_a64a05ab","line":1209,"updated":"2015-12-09 18:35:17.000000000","message":"Shouldn\u0027t we really be moving this logic into Volume VO at here - https://github.com/openstack/cinder/blob/master/cinder/objects/volume.py#L293?\n\nOtherwise we\u0027ll just end up with the same issue when some other piece of code does volume.volume_type when volume.volume_type_id is not set","commit_id":"0aa9918ce3105636c6c5b85d5b230a29b9ae7731"},{"author":{"_account_id":10115,"name":"Mitsuhiro Tanino","email":"mitsuhiro.tanino@lycorp.co.jp","username":"mtanino"},"change_message_id":"35e3f4dbbe87a810553541eedc17f04bab88a195","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"        self.volume_rpcapi.copy_volume_to_image(context,"},{"line_number":1207,"context_line":"                                                volume,"},{"line_number":1208,"context_line":"                                                recv_metadata)"},{"line_number":1209,"context_line":""},{"line_number":1210,"context_line":"        # If the volume doesn\u0027t have volume_type, volume[\u0027volume_type\u0027]"},{"line_number":1211,"context_line":"        # would trigger a db call which raise VolumeTypeNotFound exception."},{"line_number":1212,"context_line":"        volume_type \u003d (volume[\u0027volume_type\u0027] if volume[\u0027volume_type_id\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a740942_f2c8c7aa","line":1209,"in_reply_to":"7a740942_a64a05ab","updated":"2015-12-09 19:59:39.000000000","message":"Seems good idea. Thanks!","commit_id":"0aa9918ce3105636c6c5b85d5b230a29b9ae7731"}]}