)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"27b638610b4233ab190dd313a9574d7e9715851b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ivan Kolodyazhny \u003ce0ne@e0ne.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-03-31 21:36:13 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor Horizon API for cinder"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Internal API for Cinder wants volume type as required argument for"},{"line_number":10,"context_line":"volume_create function. But default None value will make code cleaner"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7f6b1bfe_44e6b318","line":7,"updated":"2020-10-15 12:46:09.000000000","message":"As noted below, it is just a clarification of required/optional parameters rather than refactoring.\n\"Clarifying optional parameters in create_volume\" makes more sense to me.","commit_id":"12150ee1aaf26d925dbcd59d8cdd5537018256e8"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"27b638610b4233ab190dd313a9574d7e9715851b","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Internal API for Cinder wants volume type as required argument for"},{"line_number":10,"context_line":"volume_create function. But default None value will make code cleaner"},{"line_number":11,"context_line":"and remove few potential point of refusal."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Co-Authored-By: Ivan Kolodyazhny \u003ce0ne@e0ne.info\u003e"},{"line_number":14,"context_line":"Change-Id: I6afb74d2375f5111089dc817a4925ab7f6d3c492"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7f6b1bfe_64180f1d","line":11,"updated":"2020-10-15 12:46:09.000000000","message":"To me, this change just clarifies what are required parameters and what are optional. Optional parameters for create_volume are passed as \"keyword\" arguments, while we can still pass them by positional arguments.","commit_id":"12150ee1aaf26d925dbcd59d8cdd5537018256e8"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":37598,"name":"Ivan Anfimov","display_name":"Ivan Anfimov","email":"lazekteam@gmail.com","username":"anfimovir"},"change_message_id":"821517f9cc6515b2fc14ca0f022448e1793c236b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"898a7ea0_3c19c751","updated":"2025-05-21 17:52:57.000000000","message":"@jjasek@redhat.com hello, please close this MR.","commit_id":"12150ee1aaf26d925dbcd59d8cdd5537018256e8"}],"openstack_dashboard/api/cinder.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"27b638610b4233ab190dd313a9574d7e9715851b","unresolved":false,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"@profiler.trace"},{"line_number":389,"context_line":"def volume_create(request, size, name, description, volume_type\u003dNone,"},{"line_number":390,"context_line":"                  snapshot_id\u003dNone, metadata\u003dNone, image_id\u003dNone,"},{"line_number":391,"context_line":"                  availability_zone\u003dNone, source_volid\u003dNone,"},{"line_number":392,"context_line":"                  group_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f6b1bfe_64e3ef34","line":389,"range":{"start_line":389,"start_character":52,"end_line":389,"end_character":69},"updated":"2020-10-15 12:46:09.000000000","message":"I am okay with this change as the cinder API does not always require volume_type argument and if None is specified cinder determines it based on the situation (snapshot, volume or image).","commit_id":"12150ee1aaf26d925dbcd59d8cdd5537018256e8"}],"openstack_dashboard/dashboards/project/volumes/forms.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"21d8943f75ee9b1f8251be2c8a337177598b9b6e","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                          data[\u0027size\u0027],"},{"line_number":430,"context_line":"                                          data[\u0027name\u0027],"},{"line_number":431,"context_line":"                                          data[\u0027description\u0027],"},{"line_number":432,"context_line":"                                          volume_type,"},{"line_number":433,"context_line":"                                          snapshot_id\u003dsnapshot_id,"},{"line_number":434,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":435,"context_line":"                                          metadata\u003dmetadata,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_4a2fb387","line":432,"range":{"start_line":432,"start_character":42,"end_line":432,"end_character":54},"updated":"2019-07-30 13:01:21.000000000","message":"I am struggling to understand what is the intention of this change? You changed api.cinder.volume_create so that \u0027volume_type\u0027 argument is now optional, but you still continue to pass \u0027volume_type\u0027 here. Why do you need to pass volume_type? \nYour change in api.cinder and this file look inconsistent.","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"3ac7c0a2911e2d6b531b6334973dbd381426d531","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                          data[\u0027size\u0027],"},{"line_number":430,"context_line":"                                          data[\u0027name\u0027],"},{"line_number":431,"context_line":"                                          data[\u0027description\u0027],"},{"line_number":432,"context_line":"                                          volume_type,"},{"line_number":433,"context_line":"                                          snapshot_id\u003dsnapshot_id,"},{"line_number":434,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":435,"context_line":"                                          metadata\u003dmetadata,"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_eac3d255","line":432,"range":{"start_line":432,"start_character":42,"end_line":432,"end_character":54},"in_reply_to":"7faddb67_42889ec4","updated":"2020-03-31 13:34:57.000000000","message":"From Cinder API perspective, an empty string and None value for volume type are the same.\n\nWe pass None as default to not specify it excplicitly","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"370b1b265eddd4adf34ce45661a7f03fa442812d","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                          data[\u0027size\u0027],"},{"line_number":430,"context_line":"                                          data[\u0027name\u0027],"},{"line_number":431,"context_line":"                                          data[\u0027description\u0027],"},{"line_number":432,"context_line":"                                          volume_type,"},{"line_number":433,"context_line":"                                          snapshot_id\u003dsnapshot_id,"},{"line_number":434,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":435,"context_line":"                                          metadata\u003dmetadata,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_8e45aab6","line":432,"range":{"start_line":432,"start_character":42,"end_line":432,"end_character":54},"in_reply_to":"7faddb67_4a2fb387","updated":"2019-07-31 13:23:00.000000000","message":"We need to pass volume type here because a user can provide a non-default value","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"32d6f8b8755a6d93960cb38a556679a1a5305b42","unresolved":false,"context_lines":[{"line_number":429,"context_line":"                                          data[\u0027size\u0027],"},{"line_number":430,"context_line":"                                          data[\u0027name\u0027],"},{"line_number":431,"context_line":"                                          data[\u0027description\u0027],"},{"line_number":432,"context_line":"                                          volume_type,"},{"line_number":433,"context_line":"                                          snapshot_id\u003dsnapshot_id,"},{"line_number":434,"context_line":"                                          image_id\u003dimage_id,"},{"line_number":435,"context_line":"                                          metadata\u003dmetadata,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_42889ec4","line":432,"range":{"start_line":432,"start_character":42,"end_line":432,"end_character":54},"in_reply_to":"7faddb67_8e45aab6","updated":"2019-09-05 10:40:03.000000000","message":"Thus, I have the following questions:\n\n- What does \"None\" (of the default value of api.cinder.volume_create()) mean?\n- Do \u0027\u0027 (empty string) and None have same meaning? If so, we should use one of them more consistently. Otherwise I cannot have a confident that we can maintain the code reliably.\n\nThe above are the main reasons I am confused with this patch and its purpose.\n\nIn addition, all optional parameters are passed \u003cname\u003e\u003d\u003cvar\u003e style. volume_type is now optional in the function definition, so volume_type\u003dvolume_type looks more consistent.","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"27b638610b4233ab190dd313a9574d7e9715851b","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                                     % snapshot.size)"},{"line_number":377,"context_line":"                    raise ValidationError(error_message)"},{"line_number":378,"context_line":"                az \u003d None"},{"line_number":379,"context_line":"                volume_type \u003d \"\""},{"line_number":380,"context_line":"            elif (data.get(\"image_source\", None) and"},{"line_number":381,"context_line":"                  source_type in [\u0027\u0027, None, \u0027image_source\u0027]):"},{"line_number":382,"context_line":"                # Create from Snapshot"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f6b1bfe_e4825f4f","side":"PARENT","line":379,"updated":"2020-10-15 12:46:09.000000000","message":"volume_type here cannot be dropped because data[\u0027type\u0027] is populated even when \u0027snapshot_source\u0027 is specified in the create from. I confirmed this by adding a log message in handle() here. This happens because the type filed is just hidden in the form.\n\nIf you would like to refer data[\u0027type\u0027] directly, it should be updated based on \u0027volume_source_type\u0027 for example in clean(). Otherwise, this does not work.\n\nI am not sure which is better. Most parameters are adjusted here. Adjusting only \"volume_type\" parameter  in other place is clear or not.\n\nThe same comment applies to L.405 too.","commit_id":"0d569a45de5ca170b64429a0faa6e0f80004c009"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"27b638610b4233ab190dd313a9574d7e9715851b","unresolved":false,"context_lines":[{"line_number":425,"context_line":""},{"line_number":426,"context_line":"            metadata \u003d {}"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"            group_id \u003d data.get(\u0027group\u0027) or None"},{"line_number":429,"context_line":"            volume \u003d cinder.volume_create(request,"},{"line_number":430,"context_line":"                                          data[\u0027size\u0027],"},{"line_number":431,"context_line":"                                          data[\u0027name\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f6b1bfe_c45f63e3","line":428,"updated":"2020-10-15 12:46:09.000000000","message":"It would be nice to move it to around L.365 as we get most values from the input data there.","commit_id":"12150ee1aaf26d925dbcd59d8cdd5537018256e8"}],"openstack_dashboard/dashboards/project/volumes/tests.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"21d8943f75ee9b1f8251be2c8a337177598b9b6e","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            filters\u003d{\u0027visibility\u0027: \u0027shared\u0027, \u0027status\u0027: \u0027active\u0027})"},{"line_number":538,"context_line":"        self.mock_volume_create.assert_called_once_with("},{"line_number":539,"context_line":"            test.IsHttpRequest(), formData[\u0027size\u0027], formData[\u0027name\u0027],"},{"line_number":540,"context_line":"            formData[\u0027description\u0027], \u0027\u0027, metadata\u003d{}, snapshot_id\u003dNone,"},{"line_number":541,"context_line":"            group_id\u003dNone, image_id\u003dNone, availability_zone\u003dNone,"},{"line_number":542,"context_line":"            source_volid\u003dvolume.id)"},{"line_number":543,"context_line":"        self.mock_group_list.assert_called_once_with(test.IsHttpRequest())"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_0a53fb18","line":540,"range":{"start_line":540,"start_character":37,"end_line":540,"end_character":39},"updated":"2019-07-30 13:01:21.000000000","message":"I cannot understand why this change is related to api.cinder change. You mentioned in the commit message that None is the default value and cinder handles it gracefully but you specify an empty string now. It looks like different and inconsistent from what you mention in the commit message.","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"370b1b265eddd4adf34ce45661a7f03fa442812d","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            filters\u003d{\u0027visibility\u0027: \u0027shared\u0027, \u0027status\u0027: \u0027active\u0027})"},{"line_number":538,"context_line":"        self.mock_volume_create.assert_called_once_with("},{"line_number":539,"context_line":"            test.IsHttpRequest(), formData[\u0027size\u0027], formData[\u0027name\u0027],"},{"line_number":540,"context_line":"            formData[\u0027description\u0027], \u0027\u0027, metadata\u003d{}, snapshot_id\u003dNone,"},{"line_number":541,"context_line":"            group_id\u003dNone, image_id\u003dNone, availability_zone\u003dNone,"},{"line_number":542,"context_line":"            source_volid\u003dvolume.id)"},{"line_number":543,"context_line":"        self.mock_group_list.assert_called_once_with(test.IsHttpRequest())"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_8e1e0ab9","line":540,"range":{"start_line":540,"start_character":37,"end_line":540,"end_character":39},"in_reply_to":"7faddb67_0a53fb18","updated":"2019-07-31 13:23:00.000000000","message":"it\u0027s a value from user-filled form (mocked in line #493)","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"32d6f8b8755a6d93960cb38a556679a1a5305b42","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            filters\u003d{\u0027visibility\u0027: \u0027shared\u0027, \u0027status\u0027: \u0027active\u0027})"},{"line_number":538,"context_line":"        self.mock_volume_create.assert_called_once_with("},{"line_number":539,"context_line":"            test.IsHttpRequest(), formData[\u0027size\u0027], formData[\u0027name\u0027],"},{"line_number":540,"context_line":"            formData[\u0027description\u0027], \u0027\u0027, metadata\u003d{}, snapshot_id\u003dNone,"},{"line_number":541,"context_line":"            group_id\u003dNone, image_id\u003dNone, availability_zone\u003dNone,"},{"line_number":542,"context_line":"            source_volid\u003dvolume.id)"},{"line_number":543,"context_line":"        self.mock_group_list.assert_called_once_with(test.IsHttpRequest())"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_226d622e","line":540,"range":{"start_line":540,"start_character":37,"end_line":540,"end_character":39},"in_reply_to":"7faddb67_8e1e0ab9","updated":"2019-09-05 10:40:03.000000000","message":"I know it, but if None means the default volume type, it is better to use None rather than \u0027\u0027 (empty string). It looks more consistent to me. If it is just because the volume create form returns an empty string, we should use None and an empty string more consistently.","commit_id":"e7bd4e38a9467b966cd232c22b650e4d584ac2c3"}]}