)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"75072f8afd627f0d94c2ba44c64e5e6ba0ee100c","unresolved":false,"context_lines":[{"line_number":12,"context_line":"selection logic in cinder.volume.flows.api.create_volume."},{"line_number":13,"context_line":"ExtractVolumeRequestTask from being able to infer the appropriate"},{"line_number":14,"context_line":"volume_type from the source volume, snapshot, or image metadata, and"},{"line_number":15,"context_line":"has caused a regression where the created volume is of the default"},{"line_number":16,"context_line":"type instead of the inferred type."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Additionally, the volume and snapshot OVOs currently set the"},{"line_number":19,"context_line":"volume_type_id to the default type on object creation if a type"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff570b3c_cfdafd77","line":16,"range":{"start_line":15,"start_character":4,"end_line":16,"end_character":34},"updated":"2020-05-22 08:21:11.000000000","message":"thanks for catching this Brian","commit_id":"868c27ea9bfcf30dd39d4127e72b90e557025e1f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"75072f8afd627f0d94c2ba44c64e5e6ba0ee100c","unresolved":false,"context_lines":[{"line_number":25,"context_line":"cannot be saved unless it has a volume_type."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"So we either have to do some database changes or move the volume_type"},{"line_number":28,"context_line":"selection logic into the REST API layer (which I don\u0027t like because"},{"line_number":29,"context_line":"the ExtractVolumeRequestTask is the appropriate place for it)."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"This patch takes the latter approach, but it doesn\u0027t contain any"},{"line_number":32,"context_line":"database changes yet.  I wanted to get some feedback first."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff570b3c_6fb931e8","line":29,"range":{"start_line":28,"start_character":41,"end_line":29,"end_character":60},"updated":"2020-05-22 08:21:11.000000000","message":"we\u0027ve 2 checks[1][2] regarding the source_volume and snapshots in the API layer which can be easily modified to associate the appropriate volume type.\n\nCould you expand on why ExtractVolumeRequestTask is the appropriate place for it? \n\n[1] https://github.com/openstack/cinder/blob/master/cinder/api/v3/volumes.py#L333-L341\n[2] https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L272-L283","commit_id":"868c27ea9bfcf30dd39d4127e72b90e557025e1f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b0b28d39c4d3f167eb594e6bf36c3e7bf092a551","unresolved":false,"context_lines":[{"line_number":25,"context_line":"cannot be saved unless it has a volume_type."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"So we either have to do some database changes or move the volume_type"},{"line_number":28,"context_line":"selection logic into the REST API layer (which I don\u0027t like because"},{"line_number":29,"context_line":"the ExtractVolumeRequestTask is the appropriate place for it)."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"This patch takes the latter approach, but it doesn\u0027t contain any"},{"line_number":32,"context_line":"database changes yet.  I wanted to get some feedback first."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff570b3c_55631bb2","line":29,"range":{"start_line":28,"start_character":41,"end_line":29,"end_character":60},"in_reply_to":"ff570b3c_6fb931e8","updated":"2020-05-22 13:18:52.000000000","message":"Well, other checks like this are being done in the ExtractVolumeRequestTask, so it would keep all the validation in one place.  The major reason, though, is that we need to have access to the image metadata if an imageRef has been passed in the API call, and that requires a call out to the Image service API.","commit_id":"868c27ea9bfcf30dd39d4127e72b90e557025e1f"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"75072f8afd627f0d94c2ba44c64e5e6ba0ee100c","unresolved":false,"context_lines":[{"line_number":28,"context_line":"selection logic into the REST API layer (which I don\u0027t like because"},{"line_number":29,"context_line":"the ExtractVolumeRequestTask is the appropriate place for it)."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"This patch takes the latter approach, but it doesn\u0027t contain any"},{"line_number":32,"context_line":"database changes yet.  I wanted to get some feedback first."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: I05915f2e32b1229ad320cd1c5748de3d63183b91"},{"line_number":35,"context_line":"Closes-bug: #1879578"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff570b3c_4f88ad21","line":32,"range":{"start_line":31,"start_character":0,"end_line":32,"end_character":21},"updated":"2020-05-22 08:21:11.000000000","message":"I feel the DB changes are important part of this feature as to mandating untyped volumes in the code doesn\u0027t seem of much use if the database can contain nullable values for volume type.","commit_id":"868c27ea9bfcf30dd39d4127e72b90e557025e1f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b0b28d39c4d3f167eb594e6bf36c3e7bf092a551","unresolved":false,"context_lines":[{"line_number":28,"context_line":"selection logic into the REST API layer (which I don\u0027t like because"},{"line_number":29,"context_line":"the ExtractVolumeRequestTask is the appropriate place for it)."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"This patch takes the latter approach, but it doesn\u0027t contain any"},{"line_number":32,"context_line":"database changes yet.  I wanted to get some feedback first."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: I05915f2e32b1229ad320cd1c5748de3d63183b91"},{"line_number":35,"context_line":"Closes-bug: #1879578"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff570b3c_75d1ff2f","line":32,"range":{"start_line":31,"start_character":0,"end_line":32,"end_character":21},"in_reply_to":"ff570b3c_4f88ad21","updated":"2020-05-22 13:18:52.000000000","message":"What we really want is that the volume_type_id be populated at the correct time, which we can tell by the volume status.  So we could move this out of the DB and into the object code, which could check the status field and raise an exception if there\u0027s no volume type.  Or do something like put a boolean column for \"must have volume type\" that gets set True when a volume first goes active, and then the OVO could raise if that column is True but volume_type_id is null.","commit_id":"868c27ea9bfcf30dd39d4127e72b90e557025e1f"}],"cinder/tests/functional/functional_helpers.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba4b848ddf82fabd2bccb1bde1fd908f0a7c3395","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                                         VOLUME, expected_end_status,"},{"line_number":198,"context_line":"                                         max_retries, status_field)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def _poll_snapshot_while(self, volume_id, continue_states,"},{"line_number":201,"context_line":"                             expected_end_status\u003dNone, max_retries\u003d5,"},{"line_number":202,"context_line":"                             status_field\u003d\u0027status\u0027):"},{"line_number":203,"context_line":"        return self._poll_resource_while(volume_id, continue_states,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_c267bde8","line":200,"range":{"start_line":200,"start_character":35,"end_line":200,"end_character":44},"updated":"2020-06-24 17:43:35.000000000","message":"snapshot_id","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7f40819eba65a7ced4339f2b399e9e36b29192bc","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                                         VOLUME, expected_end_status,"},{"line_number":198,"context_line":"                                         max_retries, status_field)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def _poll_snapshot_while(self, volume_id, continue_states,"},{"line_number":201,"context_line":"                             expected_end_status\u003dNone, max_retries\u003d5,"},{"line_number":202,"context_line":"                             status_field\u003d\u0027status\u0027):"},{"line_number":203,"context_line":"        return self._poll_resource_while(volume_id, continue_states,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_193da7dc","line":200,"range":{"start_line":200,"start_character":35,"end_line":200,"end_character":44},"in_reply_to":"bf51134e_c267bde8","updated":"2020-06-24 21:02:04.000000000","message":"Good catch on these.  Since we want to backport this, I think it will be faster in the long run if I change them now.","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba4b848ddf82fabd2bccb1bde1fd908f0a7c3395","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    def _poll_snapshot_while(self, volume_id, continue_states,"},{"line_number":201,"context_line":"                             expected_end_status\u003dNone, max_retries\u003d5,"},{"line_number":202,"context_line":"                             status_field\u003d\u0027status\u0027):"},{"line_number":203,"context_line":"        return self._poll_resource_while(volume_id, continue_states,"},{"line_number":204,"context_line":"                                         SNAPSHOT, expected_end_status,"},{"line_number":205,"context_line":"                                         max_retries, status_field)"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_025ed510","line":203,"range":{"start_line":203,"start_character":41,"end_line":203,"end_character":50},"updated":"2020-06-24 17:43:35.000000000","message":"snapshot_id","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"}],"cinder/tests/unit/volume/flows/api/test_create_volume.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"082e73e4ffed956c22b990f16bedf48b5d541a4d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    fake_config_value \u003d \u0027vt-from-config-value\u0027"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    big_ass_data_tuple \u003d ("},{"line_number":54,"context_line":"        # case 0: null params and no configured default should"},{"line_number":55,"context_line":"        # result in the system default volume type"},{"line_number":56,"context_line":"        {\u0027param_vol_type\u0027: None,"},{"line_number":57,"context_line":"         \u0027param_source_vol\u0027: None,"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_d25ec810","line":54,"updated":"2020-05-28 10:23:45.000000000","message":"Thanks a lot for test data description","commit_id":"61d4ab454a605f1c0f316a12047dabfb0d3923d2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba4b848ddf82fabd2bccb1bde1fd908f0a7c3395","unresolved":false,"context_lines":[{"line_number":61,"context_line":"         \u0027param_snap\u0027: None,"},{"line_number":62,"context_line":"         \u0027param_img_vol_type_id\u0027: None,"},{"line_number":63,"context_line":"         \u0027config_value\u0027: None,"},{"line_number":64,"context_line":"         \u0027expected_vol_type\u0027: \u0027vt-from-volume_type\u0027},"},{"line_number":65,"context_line":"        {\u0027param_vol_type\u0027: fake_vol_type,"},{"line_number":66,"context_line":"         \u0027param_source_vol\u0027: fake_source_vol,"},{"line_number":67,"context_line":"         \u0027param_snap\u0027: fake_snapshot,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_62635143","line":64,"range":{"start_line":64,"start_character":30,"end_line":64,"end_character":51},"updated":"2020-06-24 17:43:35.000000000","message":"we can use the variable here as well but this also works for me","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba4b848ddf82fabd2bccb1bde1fd908f0a7c3395","unresolved":false,"context_lines":[{"line_number":62,"context_line":"         \u0027param_img_vol_type_id\u0027: None,"},{"line_number":63,"context_line":"         \u0027config_value\u0027: None,"},{"line_number":64,"context_line":"         \u0027expected_vol_type\u0027: \u0027vt-from-volume_type\u0027},"},{"line_number":65,"context_line":"        {\u0027param_vol_type\u0027: fake_vol_type,"},{"line_number":66,"context_line":"         \u0027param_source_vol\u0027: fake_source_vol,"},{"line_number":67,"context_line":"         \u0027param_snap\u0027: fake_snapshot,"},{"line_number":68,"context_line":"         \u0027param_img_vol_type_id\u0027: fake_img_vol_type_id,"},{"line_number":69,"context_line":"         \u0027config_value\u0027: fake_config_value,"},{"line_number":70,"context_line":"         \u0027expected_vol_type\u0027: \u0027vt-from-volume_type\u0027},"},{"line_number":71,"context_line":"        # case set 2: if no volume_type is passed, the vt from the"},{"line_number":72,"context_line":"        # source_volume should be selected"},{"line_number":73,"context_line":"        {\u0027param_vol_type\u0027: None,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_e2e76111","line":70,"range":{"start_line":65,"start_character":0,"end_line":70,"end_character":53},"updated":"2020-06-24 17:43:35.000000000","message":"we won\u0027t be facing these type of scenario in real volume create flows, do we still need it?","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7f40819eba65a7ced4339f2b399e9e36b29192bc","unresolved":false,"context_lines":[{"line_number":62,"context_line":"         \u0027param_img_vol_type_id\u0027: None,"},{"line_number":63,"context_line":"         \u0027config_value\u0027: None,"},{"line_number":64,"context_line":"         \u0027expected_vol_type\u0027: \u0027vt-from-volume_type\u0027},"},{"line_number":65,"context_line":"        {\u0027param_vol_type\u0027: fake_vol_type,"},{"line_number":66,"context_line":"         \u0027param_source_vol\u0027: fake_source_vol,"},{"line_number":67,"context_line":"         \u0027param_snap\u0027: fake_snapshot,"},{"line_number":68,"context_line":"         \u0027param_img_vol_type_id\u0027: fake_img_vol_type_id,"},{"line_number":69,"context_line":"         \u0027config_value\u0027: fake_config_value,"},{"line_number":70,"context_line":"         \u0027expected_vol_type\u0027: \u0027vt-from-volume_type\u0027},"},{"line_number":71,"context_line":"        # case set 2: if no volume_type is passed, the vt from the"},{"line_number":72,"context_line":"        # source_volume should be selected"},{"line_number":73,"context_line":"        {\u0027param_vol_type\u0027: None,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_593effb7","line":70,"range":{"start_line":65,"start_character":0,"end_line":70,"end_character":53},"in_reply_to":"bf51134e_e2e76111","updated":"2020-06-24 21:02:04.000000000","message":"I think so.  The aim of this test is to define the behavior of the selection function, so that if it gets refactored and something here breaks, we\u0027ll know to look more closely.  The advantage of using ddt is that we only need to write one test for all the input combinations, so we might as well give it a full workout.  The other thing is that I want to be able to assert that the selection function will *never* return None, so I kind of went overboard in considering non-actual input sets ... but now I am confident that the selection function will never return None.","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ba4b848ddf82fabd2bccb1bde1fd908f0a7c3395","unresolved":false,"context_lines":[{"line_number":171,"context_line":"         \u0027config_value\u0027: fake_config_value})"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    @ddt.data(*smaller_data_tuple)"},{"line_number":174,"context_line":"    @mock.patch(\u0027cinder.objects.VolumeType.get_by_name_or_id\u0027,"},{"line_number":175,"context_line":"                side_effect \u003d exception.VolumeTypeNotFoundByName("},{"line_number":176,"context_line":"                    volume_type_name\u003d\"get_by_name_or_id\"))"},{"line_number":177,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_by_name\u0027,"},{"line_number":178,"context_line":"                side_effect \u003d exception.VolumeTypeNotFoundByName("},{"line_number":179,"context_line":"                    volume_type_name\u003d\"get_by_name\"))"},{"line_number":180,"context_line":"    @ddt.unpack"},{"line_number":181,"context_line":"    def test_neg_get_volume_type(self,"},{"line_number":182,"context_line":"                                 mock_get_volume_type_by_name,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_d01f8469","line":179,"range":{"start_line":174,"start_character":0,"end_line":179,"end_character":52},"updated":"2020-06-24 17:43:35.000000000","message":"these tests work without the mocking as well (as the types don\u0027t exist anyway)\nmaybe we can remove this in a followup patch","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"7f40819eba65a7ced4339f2b399e9e36b29192bc","unresolved":false,"context_lines":[{"line_number":171,"context_line":"         \u0027config_value\u0027: fake_config_value})"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    @ddt.data(*smaller_data_tuple)"},{"line_number":174,"context_line":"    @mock.patch(\u0027cinder.objects.VolumeType.get_by_name_or_id\u0027,"},{"line_number":175,"context_line":"                side_effect \u003d exception.VolumeTypeNotFoundByName("},{"line_number":176,"context_line":"                    volume_type_name\u003d\"get_by_name_or_id\"))"},{"line_number":177,"context_line":"    @mock.patch(\u0027cinder.volume.volume_types.get_volume_type_by_name\u0027,"},{"line_number":178,"context_line":"                side_effect \u003d exception.VolumeTypeNotFoundByName("},{"line_number":179,"context_line":"                    volume_type_name\u003d\"get_by_name\"))"},{"line_number":180,"context_line":"    @ddt.unpack"},{"line_number":181,"context_line":"    def test_neg_get_volume_type(self,"},{"line_number":182,"context_line":"                                 mock_get_volume_type_by_name,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_b99d9b67","line":179,"range":{"start_line":174,"start_character":0,"end_line":179,"end_character":52},"in_reply_to":"bf51134e_d01f8469","updated":"2020-06-24 21:02:04.000000000","message":"I usually dislike unneeded mocking, but it\u0027s a contingent matter of fact that the fake type names don\u0027t map to anything, and these mocks remove the contingency so that you\u0027ll get an exception whether the type exists or not, which is the behavior we want for this test.  So I think using the mocks keeps this unit test isolated.  But I could very well just be being stubborn here.","commit_id":"594f2e61f0c02e1a4a27f969862e8ed08bafd33b"}],"cinder/volume/flows/api/create_volume.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"082e73e4ffed956c22b990f16bedf48b5d541a4d","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        \"\"\"Returns a volume_type object or raises.  Never returns None.\"\"\""},{"line_number":356,"context_line":"        if volume_type:"},{"line_number":357,"context_line":"            return volume_type"},{"line_number":358,"context_line":"        identifier \u003d {}"},{"line_number":359,"context_line":"        if source_volume:"},{"line_number":360,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027volume\u0027,"},{"line_number":361,"context_line":"                          \u0027id\u0027: source_volume[\u0027volume_type_id\u0027]}"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_92a9f0de","line":358,"updated":"2020-05-28 10:23:45.000000000","message":"nit: IMO,  \u0027identifier \u003d None\u0027 will be more clear","commit_id":"61d4ab454a605f1c0f316a12047dabfb0d3923d2"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"11438ad8ecd544fe580e6e4ae3aedefa4fa61627","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        \"\"\"Returns a volume_type object or raises.  Never returns None.\"\"\""},{"line_number":356,"context_line":"        if volume_type:"},{"line_number":357,"context_line":"            return volume_type"},{"line_number":358,"context_line":"        identifier \u003d {}"},{"line_number":359,"context_line":"        if source_volume:"},{"line_number":360,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027volume\u0027,"},{"line_number":361,"context_line":"                          \u0027id\u0027: source_volume[\u0027volume_type_id\u0027]}"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff570b3c_93fe1236","line":358,"in_reply_to":"ff570b3c_92a9f0de","updated":"2020-05-28 13:39:40.000000000","message":"I thought I was making a big improvement by not using a defaultdict, but you are right, None would be even better!","commit_id":"61d4ab454a605f1c0f316a12047dabfb0d3923d2"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0cfae9271ebdef1038bfa8e62bec12f20f0d8e12","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        elif image_volume_type_id:"},{"line_number":366,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027image\u0027,"},{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_2047aa4e","line":370,"range":{"start_line":368,"start_character":0,"end_line":370,"end_character":57},"updated":"2020-06-02 09:50:11.000000000","message":"instead of this, we can use [1] which will return default_volume_type (if CONF.default_volume_type is set) else it will return __DEFAULT__ type\n\nafter this change, i think the changes below aren\u0027t required as well\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/volume/volume_types.py#L175","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"dce1c0ac81793f8f1e240695b64ef2dafd166dda","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_1d367ae8","line":370,"updated":"2020-05-29 15:12:58.000000000","message":"you could also simply do an else here and set\nidentifier \u003d {\u0027source\u0027: \u0027default volume type\u0027,\n                    \u0027id\u0027: volume_types.DEFAULT_VOLUME_TYPE}","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f5a9dfe50e7795b32b901955057f842ed74e4e4","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_c439f4d8","line":370,"in_reply_to":"ff570b3c_1d367ae8","updated":"2020-06-11 18:33:04.000000000","message":"By not having an else here, I can do a fallback at line 384 if it turns out that the value passed in isn\u0027t a legitimate volume_type.  This preserves the prior behavior of logging a message and allowing the volume to be built with no volume type, except now we allow the volume to be built with the __DEFAULT__ type.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f5a9dfe50e7795b32b901955057f842ed74e4e4","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        elif image_volume_type_id:"},{"line_number":366,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027image\u0027,"},{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_44820454","line":370,"range":{"start_line":368,"start_character":0,"end_line":370,"end_character":57},"in_reply_to":"ff570b3c_2047aa4e","updated":"2020-06-11 18:33:04.000000000","message":"I disagree.  That won\u0027t help in the situation where there\u0027s a image_volume_type_id on the image, but it contains a typo or something.  The old behavior was that the volume build would continue but with no volume type; the new behavior should be to continue the build but with the __DEFAULT__ volume type.  Otherwise, we are changing the API behavior too much IMO.  So I think we definitely want the fallback at line 384, and since we have that there, I think it\u0027s simpler to just grab the CONF value directly here instead of using the function.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8e23c46ebdf671902b8697b3879c5c3cd1360c31","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_c4a0d494","line":370,"in_reply_to":"ff570b3c_c439f4d8","updated":"2020-06-11 18:47:55.000000000","message":"I don\u0027t understand the image_volume_type_id relation here but the current behavior now is if the default_volume_type value is set wrong in the CONF file then we raise an exception.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"87274c67e564e4cc4e6877d34f04423a6dce678a","unresolved":false,"context_lines":[{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"},{"line_number":368,"context_line":"        elif CONF.default_volume_type:"},{"line_number":369,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027default volume type config\u0027,"},{"line_number":370,"context_line":"                          \u0027id\u0027: CONF.default_volume_type}"},{"line_number":371,"context_line":"        if identifier:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_ea14c1eb","line":370,"in_reply_to":"ff570b3c_c4a0d494","updated":"2020-06-11 20:44:36.000000000","message":"The point is that raising an exception is a big change in behavior and if that\u0027s what we wanted to do, it should have been mentioned in the spec.  Until Train, it would have resulted in the volume-create succeeding but with no volume_type.  So a lot of volume-create requests that previously would have succeded now fail.  I think we need to discuss what the expected behavior should be here.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0cfae9271ebdef1038bfa8e62bec12f20f0d8e12","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("},{"line_number":374,"context_line":"                    context, identifier[\u0027id\u0027])"},{"line_number":375,"context_line":"            except (exception.VolumeTypeNotFound,"},{"line_number":376,"context_line":"                    exception.VolumeTypeNotFoundByName,"},{"line_number":377,"context_line":"                    exception.InvalidVolumeType):"},{"line_number":378,"context_line":"                identifier[\"default\"] \u003d volume_types.DEFAULT_VOLUME_TYPE"},{"line_number":379,"context_line":"                LOG.exception(\"Failed to find volume type from \""},{"line_number":380,"context_line":"                              \"source %(source)s, identifier %(id)s, \""},{"line_number":381,"context_line":"                              \"will use %(default)s type\", identifier)"},{"line_number":382,"context_line":"        # if this next call raises, we\u0027re completely hosed anyway,"},{"line_number":383,"context_line":"        # so just let \u0027er rip"},{"line_number":384,"context_line":"        return objects.VolumeType.get_by_name_or_id("},{"line_number":385,"context_line":"            context, volume_types.DEFAULT_VOLUME_TYPE)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def execute(self, context, size, snapshot, image_id, source_volume,"},{"line_number":388,"context_line":"                availability_zone, volume_type, metadata, key_manager,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_6084025f","line":385,"range":{"start_line":375,"start_character":12,"end_line":385,"end_character":54},"updated":"2020-06-02 09:50:11.000000000","message":"if we set wrong value/name in CONF.default_volume_type then this will assign the default volume type to the volume instead of error out on the wrong name/value set in conf file which is incorrect behavior","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8e23c46ebdf671902b8697b3879c5c3cd1360c31","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("},{"line_number":374,"context_line":"                    context, identifier[\u0027id\u0027])"},{"line_number":375,"context_line":"            except (exception.VolumeTypeNotFound,"},{"line_number":376,"context_line":"                    exception.VolumeTypeNotFoundByName,"},{"line_number":377,"context_line":"                    exception.InvalidVolumeType):"},{"line_number":378,"context_line":"                identifier[\"default\"] \u003d volume_types.DEFAULT_VOLUME_TYPE"},{"line_number":379,"context_line":"                LOG.exception(\"Failed to find volume type from \""},{"line_number":380,"context_line":"                              \"source %(source)s, identifier %(id)s, \""},{"line_number":381,"context_line":"                              \"will use %(default)s type\", identifier)"},{"line_number":382,"context_line":"        # if this next call raises, we\u0027re completely hosed anyway,"},{"line_number":383,"context_line":"        # so just let \u0027er rip"},{"line_number":384,"context_line":"        return objects.VolumeType.get_by_name_or_id("},{"line_number":385,"context_line":"            context, volume_types.DEFAULT_VOLUME_TYPE)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def execute(self, context, size, snapshot, image_id, source_volume,"},{"line_number":388,"context_line":"                availability_zone, volume_type, metadata, key_manager,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_449fa464","line":385,"range":{"start_line":375,"start_character":12,"end_line":385,"end_character":54},"in_reply_to":"ff570b3c_39329d10","updated":"2020-06-11 18:47:55.000000000","message":"The change mentioned results in an exception (see L#186) if the CONF.default_volume_type value is wrong (volume type doesn\u0027t exist).\nIf the user has set a wrong volume type value in conf and we still continue to create a volume with default type seems pretty wrong as they need to know they\u0027ve set the wrong type and should correct it.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9f5a9dfe50e7795b32b901955057f842ed74e4e4","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                return objects.VolumeType.get_by_name_or_id("},{"line_number":374,"context_line":"                    context, identifier[\u0027id\u0027])"},{"line_number":375,"context_line":"            except (exception.VolumeTypeNotFound,"},{"line_number":376,"context_line":"                    exception.VolumeTypeNotFoundByName,"},{"line_number":377,"context_line":"                    exception.InvalidVolumeType):"},{"line_number":378,"context_line":"                identifier[\"default\"] \u003d volume_types.DEFAULT_VOLUME_TYPE"},{"line_number":379,"context_line":"                LOG.exception(\"Failed to find volume type from \""},{"line_number":380,"context_line":"                              \"source %(source)s, identifier %(id)s, \""},{"line_number":381,"context_line":"                              \"will use %(default)s type\", identifier)"},{"line_number":382,"context_line":"        # if this next call raises, we\u0027re completely hosed anyway,"},{"line_number":383,"context_line":"        # so just let \u0027er rip"},{"line_number":384,"context_line":"        return objects.VolumeType.get_by_name_or_id("},{"line_number":385,"context_line":"            context, volume_types.DEFAULT_VOLUME_TYPE)"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    def execute(self, context, size, snapshot, image_id, source_volume,"},{"line_number":388,"context_line":"                availability_zone, volume_type, metadata, key_manager,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff570b3c_39329d10","line":385,"range":{"start_line":375,"start_character":12,"end_line":385,"end_character":54},"in_reply_to":"ff570b3c_6084025f","updated":"2020-06-11 18:33:04.000000000","message":"That behavior was introduced by I4da0c13b5b3f8174a30b8557f968d6b9e641b091 and I don\u0027t see anything on the spec about that.  The pre-default-volume-type behavior was to log a message and continue, which would result in volume creation (but with an empty volume_type).  It\u0027s not obvious to me why we shouldn\u0027t continue with volume creation, using the __DEFAULT__ type, which would be basically equivalent to the old behavior.","commit_id":"cbc2078cf1b1be9109aa6a9492190cba6a321a4d"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"eec02ab1d66d4cf56f83b8940df746a409300d2e","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        identifier \u003d None"},{"line_number":359,"context_line":"        if source_volume:"},{"line_number":360,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027volume\u0027,"},{"line_number":361,"context_line":"                          \u0027id\u0027: source_volume[\u0027volume_type_id\u0027]}"},{"line_number":362,"context_line":"        elif snapshot:"},{"line_number":363,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027snapshot\u0027,"},{"line_number":364,"context_line":"                          \u0027id\u0027: snapshot[\u0027volume_type_id\u0027]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_1c723ce6","line":361,"range":{"start_line":361,"start_character":45,"end_line":361,"end_character":46},"updated":"2020-06-17 14:44:52.000000000","message":"The function logic itself looks good, but not sure if all the callers have well deal with this change or not.\n\nDoes it need to change to below format?\n.get(\u0027volume_type_id\u0027)","commit_id":"eb002799923492023d227ffc3b4528608827002e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"55461b710425f2b12a909558828ebb42d14dab50","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        identifier \u003d None"},{"line_number":359,"context_line":"        if source_volume:"},{"line_number":360,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027volume\u0027,"},{"line_number":361,"context_line":"                          \u0027id\u0027: source_volume[\u0027volume_type_id\u0027]}"},{"line_number":362,"context_line":"        elif snapshot:"},{"line_number":363,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027snapshot\u0027,"},{"line_number":364,"context_line":"                          \u0027id\u0027: snapshot[\u0027volume_type_id\u0027]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_746d1a31","line":361,"range":{"start_line":361,"start_character":45,"end_line":361,"end_character":46},"in_reply_to":"bf51134e_1c723ce6","updated":"2020-06-19 14:02:38.000000000","message":"I think we\u0027re OK because these should be our oslo versioned objects, which can handle this.  (Besides, the old code was using this syntax.)","commit_id":"eb002799923492023d227ffc3b4528608827002e"},{"author":{"_account_id":28948,"name":"Liang Fang","email":"liang.a.fang@intel.com","username":"liang"},"change_message_id":"eec02ab1d66d4cf56f83b8940df746a409300d2e","unresolved":false,"context_lines":[{"line_number":361,"context_line":"                          \u0027id\u0027: source_volume[\u0027volume_type_id\u0027]}"},{"line_number":362,"context_line":"        elif snapshot:"},{"line_number":363,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027snapshot\u0027,"},{"line_number":364,"context_line":"                          \u0027id\u0027: snapshot[\u0027volume_type_id\u0027]}"},{"line_number":365,"context_line":"        elif image_volume_type_id:"},{"line_number":366,"context_line":"            identifier \u003d {\u0027source\u0027: \u0027image\u0027,"},{"line_number":367,"context_line":"                          \u0027id\u0027: image_volume_type_id}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_3722b9a5","line":364,"range":{"start_line":364,"start_character":40,"end_line":364,"end_character":41},"updated":"2020-06-17 14:44:52.000000000","message":"just a reminder, maybe not worth to do change.\n.get(\u0027\u0027)","commit_id":"eb002799923492023d227ffc3b4528608827002e"}]}