)]}'
{"cinder/api/contrib/types_manage.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e2cddd454face9524230ebc25f454f4c455f5a36","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            self._notify_volume_type_error("},{"line_number":164,"context_line":"                context, \u0027volume_type.delete\u0027, err, volume_type\u003dvol_type)"},{"line_number":165,"context_line":"            msg \u003d err.msg"},{"line_number":166,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        return webob.Response(status_int\u003dhttp_client.ACCEPTED)"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_5b1f25b6","line":166,"updated":"2020-08-05 13:36:40.000000000","message":"I think we need to re-think this except block (also, we\u0027ll need to catch one more exception if you adopt my suggestion in exception.py).\n\nVolumeTypeDefaultDeletionError: that\u0027s fine, you asked to delete something that you\u0027re not allowed to delete, so it\u0027s the caller\u0027s fault, 400 is good.\n\nVolumeTypeDefaultNotFound: I think this needs to raise a 500, because this can only be fixed on the server side, and subsequent requests will continue to fail until the server is configured properly.  (This gets raised by get_default_type(), so it could happen on a whole bunch of requests -- do you know what gets returned on those?)\n\nVolumeTypeDeletionError: I think this will be OK as a 400.","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8cbbf6b8c1d4627a089331748f1845748d455262","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            self._notify_volume_type_error("},{"line_number":164,"context_line":"                context, \u0027volume_type.delete\u0027, err, volume_type\u003dvol_type)"},{"line_number":165,"context_line":"            msg \u003d err.msg"},{"line_number":166,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        return webob.Response(status_int\u003dhttp_client.ACCEPTED)"},{"line_number":169,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_37eebdec","line":166,"in_reply_to":"9f560f44_5b1f25b6","updated":"2020-08-05 21:23:03.000000000","message":"Done","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"}],"cinder/api/v2/volumes.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                   \"be to specify multiattach enabled volume types.\")"},{"line_number":265,"context_line":"            versionutils.report_deprecated_feature(LOG, msg)"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        try:"},{"line_number":268,"context_line":"            new_volume \u003d self.volume_api.create("},{"line_number":269,"context_line":"                context, size, volume.get(\u0027display_name\u0027),"},{"line_number":270,"context_line":"                volume.get(\u0027display_description\u0027), **kwargs)"},{"line_number":271,"context_line":"        except exception.DefaultNotFound as err:"},{"line_number":272,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003derr.msg)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        retval \u003d self._view_builder.detail(req, new_volume)"},{"line_number":275,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_d6bfcccd","line":272,"range":{"start_line":267,"start_character":0,"end_line":272,"end_character":63},"updated":"2020-07-31 13:32:49.000000000","message":"this is a really good catch -- let\u0027s make sure we have a test somewhere for this.","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"}],"cinder/common/config.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7b2fb112270e331a38b5ccfc6d1365c79ef05e6b","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_6be444ad","line":111,"updated":"2020-08-17 15:57:46.000000000","message":"What is the reason for marking this required when we have a default value supplied?","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b7ce76f800391ed574b02a14bbae865d322da386","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_7e8c0035","line":111,"in_reply_to":"9f560f44_6be444ad","updated":"2020-08-17 16:27:26.000000000","message":"Seems redundant\nWill remove it","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c9291bd04b0c5c64f413f8e95eacfbe74fee30c7","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_d2725dad","line":111,"in_reply_to":"9f560f44_77ab831c","updated":"2020-08-17 19:43:01.000000000","message":"I think it\u0027s more like when you buy a \"batteries included\" electronic device with one of those crappy off-brand batteries--you know it will work long enough for you to get an idea of how the device works, but if you\u0027re smart you\u0027re going purchase a good battery soon.  I look at __DEFAULT__ as the Cinder version of a crappy battery.  We could just sell Cinder with the special little screwdriver you need to change the battery, but batteries included plus the screwdriver is even better.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"767c78c3da2539ad34865d072b8f388a3e415455","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_97f2976e","line":111,"in_reply_to":"9f560f44_7e8c0035","updated":"2020-08-17 19:17:03.000000000","message":"This actually does have a function.  If you run oslo-config-generator with the --minimal option, it includes only those options that have been registered with required\u003dTrue.  In order to encourage operators to actually configure this option, I think we should flag it as required.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"fabf719e10cbcfe4f126f3eb6e2b139c329ac864","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_77ab831c","line":111,"in_reply_to":"9f560f44_97f2976e","updated":"2020-08-17 19:24:16.000000000","message":"Doesn\u0027t that kind of defeat the purpose of having a default value? Not completely, but if we are going to try to do extra work to make sure they set this to something other than __DEFAULT__, let\u0027s just get rid of the default value and make them explicitly use that if they don\u0027t want something else.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"c31c4a432677cd809b3cd0bf734bb86c1c751ac2","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_d4db687a","line":111,"in_reply_to":"9f560f44_d2725dad","updated":"2020-08-17 23:53:37.000000000","message":"If having \u0027required\u003dTrue\u0027 makes it so that operators are more likely to see the default_volume_type, then I think that is the way to go.  More user friendly in my opinion.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"99f2f06cfcd80cf80810a17e7153ce7547fb7953","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               required\u003dTrue,"},{"line_number":112,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":113,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"},{"line_number":114,"context_line":"               help\u003d\u0027Default group type to use\u0027),"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_34be24d4","line":111,"in_reply_to":"9f560f44_d4db687a","updated":"2020-08-18 00:16:37.000000000","message":"Looks like the agreement is more towards keeping it and an additional check is always better so adding that again.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"cdffd27d6fa98367cfa544711b62566b18dc5496","unresolved":false,"context_lines":[{"line_number":106,"context_line":"                     \u0027unavailable, fall back to the value of \u0027"},{"line_number":107,"context_line":"                     \u0027default_availability_zone, then \u0027"},{"line_number":108,"context_line":"                     \u0027storage_availability_zone, instead of failing.\u0027),"},{"line_number":109,"context_line":"    cfg.StrOpt(\u0027default_volume_type\u0027,"},{"line_number":110,"context_line":"               default\u003d\u0027__DEFAULT__\u0027,"},{"line_number":111,"context_line":"               help\u003d\u0027Default volume type to use\u0027),"},{"line_number":112,"context_line":"    cfg.StrOpt(\u0027default_group_type\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"9f560f44_3703cb44","line":109,"range":{"start_line":109,"start_character":15,"end_line":109,"end_character":36},"updated":"2020-08-17 19:18:41.000000000","message":"See my comment on PS 14 about why I think this should be registered as \u0027required\u0027.","commit_id":"56c647a2b528b778eab6fd3f13c91db50046d974"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":4153,"context_line":""},{"line_number":4154,"context_line":"@require_admin_context"},{"line_number":4155,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":4156,"context_line":"def volume_type_destroy(context, id):"},{"line_number":4157,"context_line":"    utcnow \u003d timeutils.utcnow()"},{"line_number":4158,"context_line":"    session \u003d get_session()"},{"line_number":4159,"context_line":"    with session.begin():"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_af280de2","line":4156,"updated":"2020-07-30 22:59:20.000000000","message":"I wonder if we should change this function so that it will guarantee that there is always one non-deleted volume_type.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9724bcfa575d6142f522820165aa529a13fe9d42","unresolved":false,"context_lines":[{"line_number":4153,"context_line":""},{"line_number":4154,"context_line":"@require_admin_context"},{"line_number":4155,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":4156,"context_line":"def volume_type_destroy(context, id):"},{"line_number":4157,"context_line":"    utcnow \u003d timeutils.utcnow()"},{"line_number":4158,"context_line":"    session \u003d get_session()"},{"line_number":4159,"context_line":"    with session.begin():"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_d3bb5ec1","line":4156,"in_reply_to":"9f560f44_4401536b","updated":"2020-07-31 12:46:12.000000000","message":"Ok, I\u0027ll think about this some more.  I look at this like making the volume_type column non-nullable in the database ... we have checks in the code so that it should never happen, but we nonetheless made the column non-nullable.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":4153,"context_line":""},{"line_number":4154,"context_line":"@require_admin_context"},{"line_number":4155,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"},{"line_number":4156,"context_line":"def volume_type_destroy(context, id):"},{"line_number":4157,"context_line":"    utcnow \u003d timeutils.utcnow()"},{"line_number":4158,"context_line":"    session \u003d get_session()"},{"line_number":4159,"context_line":"    with session.begin():"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_4401536b","line":4156,"in_reply_to":"9f560f44_af280de2","updated":"2020-07-31 08:57:30.000000000","message":"This is only called by our destroy method in which we\u0027ve handled the case and not called from anywhere else (and i think it shouldn\u0027t be)\nMaking changes here seems redundant IMO.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e2cddd454face9524230ebc25f454f4c455f5a36","unresolved":false,"context_lines":[{"line_number":4159,"context_line":"    with session.begin():"},{"line_number":4160,"context_line":"        vol_types \u003d volume_type_get_all(context)"},{"line_number":4161,"context_line":"        if len(vol_types) \u003c\u003d 1:"},{"line_number":4162,"context_line":"            raise exception.VolumeTypeDefaultNotFound(volume_type_name\u003d\u0027\u0027)"},{"line_number":4163,"context_line":"        _volume_type_get(context, id, session)"},{"line_number":4164,"context_line":"        results \u003d model_query(context, models.Volume, session\u003dsession). \\"},{"line_number":4165,"context_line":"            filter_by(volume_type_id\u003did).all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_58fbab45","line":4162,"range":{"start_line":4162,"start_character":28,"end_line":4162,"end_character":53},"updated":"2020-08-05 13:36:40.000000000","message":"see my comment in exception.py","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8cbbf6b8c1d4627a089331748f1845748d455262","unresolved":false,"context_lines":[{"line_number":4159,"context_line":"    with session.begin():"},{"line_number":4160,"context_line":"        vol_types \u003d volume_type_get_all(context)"},{"line_number":4161,"context_line":"        if len(vol_types) \u003c\u003d 1:"},{"line_number":4162,"context_line":"            raise exception.VolumeTypeDefaultNotFound(volume_type_name\u003d\u0027\u0027)"},{"line_number":4163,"context_line":"        _volume_type_get(context, id, session)"},{"line_number":4164,"context_line":"        results \u003d model_query(context, models.Volume, session\u003dsession). \\"},{"line_number":4165,"context_line":"            filter_by(volume_type_id\u003did).all()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_d73941d7","line":4162,"range":{"start_line":4162,"start_character":28,"end_line":4162,"end_character":53},"in_reply_to":"9f560f44_58fbab45","updated":"2020-08-05 21:23:03.000000000","message":"Done","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"}],"cinder/exception.py":[{"author":{"_account_id":31534,"name":"Graham Lenton","email":"maharg101@gmail.com"},"change_message_id":"7616fe51a177f52fbd45e87c35dfe780a772f5da","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDeleteException(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The type %(volume_type_id)s requested cannot be deleted as \""},{"line_number":404,"context_line":"                \"it is the default type or default type set is wrong.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class GroupTypeNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_69709c09","line":404,"updated":"2020-07-24 11:26:54.000000000","message":"This message might be clearer as \".. cannot be deleted as it could not be found or is configured as the default type\".\n\nThese are the two conditions which cause the exception to be raised.","commit_id":"99eb1dae38ed38c27b1b82ea96d4ee779536ec48"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"5c54cfce444603b304ee29d89a6e2be63dfc59ea","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDeleteException(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The type %(volume_type_id)s requested cannot be deleted as \""},{"line_number":404,"context_line":"                \"it is the default type or default type set is wrong.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class GroupTypeNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_88ed8c27","line":404,"in_reply_to":"bf51134e_69709c09","updated":"2020-07-27 15:21:57.000000000","message":"If the type is not found then we get a different exception.\nThis exception states that the default type isn\u0027t set so we\u0027ve no idea if the type requested to delete is default or not.\nSo we should first set the default_volume_type before trying to delete any type.\nCreate volume operation will fail with the same reasons.","commit_id":"99eb1dae38ed38c27b1b82ea96d4ee779536ec48"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":399,"context_line":"                \"volumes present with the type.\")"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDefault(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The volume type %(volume_type_id)s is the default volume \""},{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_33b9fa79","line":402,"range":{"start_line":402,"start_character":6,"end_line":402,"end_character":23},"updated":"2020-07-31 13:32:49.000000000","message":"Let\u0027s think of a better name for this one so that it\u0027s self documenting when it appears in the code.  Maybe \u0027VolumeTypeDefaultCannotBeDeleted\u0027 or \u0027VolumeTypeDefaultDeletionError\u0027 (they\u0027re kind of long, but I can\u0027t think of a shorter one ATM).","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":399,"context_line":"                \"volumes present with the type.\")"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDefault(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The volume type %(volume_type_id)s is the default volume \""},{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_c5f88f4a","line":402,"range":{"start_line":402,"start_character":6,"end_line":402,"end_character":23},"in_reply_to":"9f560f44_33b9fa79","updated":"2020-08-04 13:29:49.000000000","message":"If VolumeType prefix is required then VolumeTypeDefaultDeletionError makes sense to me.","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class DefaultNotFound(NotFound):"},{"line_number":408,"context_line":"    message \u003d _(\"The request cannot be fulfilled as type %(volume_type_name)s \""},{"line_number":409,"context_line":"                \"set in default_volume_type is not found. check cinder.conf\")"},{"line_number":410,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_93a72650","line":407,"range":{"start_line":407,"start_character":6,"end_line":407,"end_character":21},"updated":"2020-07-31 13:32:49.000000000","message":"I suggest renaming this to \u0027VolumeTypeDefaultNotFound\u0027 (it\u0027s already tied to the volume_type by the message parameters, so might as well make it explicit in the class name).  Plus it will be consistent with the other VolumeType* exceptions.","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class DefaultNotFound(NotFound):"},{"line_number":408,"context_line":"    message \u003d _(\"The request cannot be fulfilled as type %(volume_type_name)s \""},{"line_number":409,"context_line":"                \"set in default_volume_type is not found. check cinder.conf\")"},{"line_number":410,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_059c470d","line":407,"range":{"start_line":407,"start_character":6,"end_line":407,"end_character":21},"in_reply_to":"9f560f44_93a72650","updated":"2020-08-04 13:29:49.000000000","message":"Done","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class DefaultNotFound(NotFound):"},{"line_number":408,"context_line":"    message \u003d _(\"The request cannot be fulfilled as type %(volume_type_name)s \""},{"line_number":409,"context_line":"                \"set in default_volume_type is not found. check cinder.conf\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class GroupTypeNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_f37942fc","line":409,"range":{"start_line":409,"start_character":57,"end_line":409,"end_character":75},"updated":"2020-07-31 13:32:49.000000000","message":"An end user can\u0027t do this, and we are logging the message about checking cinder.conf for the operator, so I think you can leave this part off.\n\nAlso, this message will also apply to project-level defaults when they\u0027re implemented, so I think don\u0027t mention the config setting name.  I think just a generic message like:\n\n\"The request cannot be fulfilled as the default volume type \u0027%(volume_type_name)s\u0027 cannot be found.\"","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class DefaultNotFound(NotFound):"},{"line_number":408,"context_line":"    message \u003d _(\"The request cannot be fulfilled as type %(volume_type_name)s \""},{"line_number":409,"context_line":"                \"set in default_volume_type is not found. check cinder.conf\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class GroupTypeNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_e5a95324","line":409,"range":{"start_line":409,"start_character":57,"end_line":409,"end_character":75},"in_reply_to":"9f560f44_f37942fc","updated":"2020-08-04 13:29:49.000000000","message":"Done","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e2cddd454face9524230ebc25f454f4c455f5a36","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDefaultDeletionError(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The volume type %(volume_type_id)s is the default volume \""},{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_58f1eb94","line":404,"updated":"2020-08-05 13:36:40.000000000","message":"To address your concern about the exception raised in the DB layer, how about adding a new exception?\n\n  class VolumeTypeDeletionError(CinderException):\n      message \u003d _(\"The volume type %(volume_type_id)s is the only currently defined \"\n                  \"volume type and cannot be deleted.\")","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8cbbf6b8c1d4627a089331748f1845748d455262","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"class VolumeTypeDefaultDeletionError(CinderException):"},{"line_number":403,"context_line":"    message \u003d _(\"The volume type %(volume_type_id)s is the default volume \""},{"line_number":404,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_f73c45c9","line":404,"in_reply_to":"9f560f44_58f1eb94","updated":"2020-08-05 21:23:03.000000000","message":"Done","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b3dc105fa5b1c926a2cd23b4840ff038f6c8a55b","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"},{"line_number":413,"context_line":"    message \u003d _(\"The request cannot be fulfilled as the default volume type \""},{"line_number":414,"context_line":"                \"%(volume_type_name)s cannot be found.\")"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_cdc37288","line":412,"range":{"start_line":412,"start_character":32,"end_line":412,"end_character":40},"updated":"2020-08-05 22:53:12.000000000","message":"I think we want this to be based on CinderException since it\u0027s ultimately a 500.\n\nThe other two are 400s, so maybe make them subclasses of Invalid.","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4c0edca254dccc309734e3b989a2cc180f945a77","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"},{"line_number":413,"context_line":"    message \u003d _(\"The request cannot be fulfilled as the default volume type \""},{"line_number":414,"context_line":"                \"%(volume_type_name)s cannot be found.\")"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_84414cac","line":412,"range":{"start_line":412,"start_character":32,"end_line":412,"end_character":40},"in_reply_to":"9f560f44_64da3878","updated":"2020-08-06 13:56:46.000000000","message":"Done","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f781c341175358e07bfc8cf3c2adc4d8d9761ec9","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"},{"line_number":413,"context_line":"    message \u003d _(\"The request cannot be fulfilled as the default volume type \""},{"line_number":414,"context_line":"                \"%(volume_type_name)s cannot be found.\")"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_64da3878","line":412,"range":{"start_line":412,"start_character":32,"end_line":412,"end_character":40},"in_reply_to":"9f560f44_894de53f","updated":"2020-08-06 13:24:28.000000000","message":"\u003e I think you are on the right track, though.  How about changing the\n \u003e name of this exception to \u0027VolumeTypeDefaultMisconfiguredError\u0027 and\n \u003e making it a subclass of Invalid?\n\nSorry, I meant CinderException (500)","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"89bd474185c79620cea4d71d7addab8f24107763","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"},{"line_number":413,"context_line":"    message \u003d _(\"The request cannot be fulfilled as the default volume type \""},{"line_number":414,"context_line":"                \"%(volume_type_name)s cannot be found.\")"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_894de53f","line":412,"range":{"start_line":412,"start_character":32,"end_line":412,"end_character":40},"in_reply_to":"9f560f44_8dd33ad5","updated":"2020-08-06 12:34:15.000000000","message":"You need to remember that the default_volume_type is now a required configuration option.  If it is ever not found, that is a server-side error.\n\nI think you are on the right track, though.  How about changing the name of this exception to \u0027VolumeTypeDefaultMisconfiguredError\u0027 and making it a subclass of Invalid?","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a02de33d2ddd2fd66b5d8f289bb021676e41a1d5","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                \"type and cannot be deleted.\")"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"class VolumeTypeDefaultNotFound(NotFound):"},{"line_number":413,"context_line":"    message \u003d _(\"The request cannot be fulfilled as the default volume type \""},{"line_number":414,"context_line":"                \"%(volume_type_name)s cannot be found.\")"},{"line_number":415,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_8dd33ad5","line":412,"range":{"start_line":412,"start_character":32,"end_line":412,"end_character":40},"in_reply_to":"9f560f44_cdc37288","updated":"2020-08-06 01:08:39.000000000","message":"Not really sure regarding this.\nthis will also be raised with cinder type-default command, although it requires a change in cinder.conf, the default type set and requested is not found sounds valid to me as well.\nAnd if we really want this to be config specific then we should change it to some invalid config exception.","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"}],"cinder/tests/functional/test_volumes.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # Should be gone"},{"line_number":79,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_create_default_volume_type_exists(self):"},{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_8479b030","line":81,"updated":"2020-07-30 22:59:20.000000000","message":"The idea behind the original test was to make a create-volume request that does not contain a volume type, but verify that the created volume did indeed have a type.  So I think the original name was better.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # Should be gone"},{"line_number":79,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_create_default_volume_type_exists(self):"},{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_67cbb971","line":81,"in_reply_to":"9f560f44_8479b030","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_create_default_volume_type_exists(self):"},{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"},{"line_number":85,"context_line":"        self.flags(default_volume_type\u003dvolume_types.DEFAULT_VOLUME_TYPE)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_649a9c63","line":82,"range":{"start_line":82,"start_character":42,"end_line":82,"end_character":73},"updated":"2020-07-30 22:59:20.000000000","message":"This part isn\u0027t necessarily true anymore, so I think you can delete it.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def test_create_default_volume_type_exists(self):"},{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"},{"line_number":85,"context_line":"        self.flags(default_volume_type\u003dvolume_types.DEFAULT_VOLUME_TYPE)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_c7ca856b","line":82,"range":{"start_line":82,"start_character":42,"end_line":82,"end_character":73},"in_reply_to":"9f560f44_649a9c63","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"},{"line_number":85,"context_line":"        self.flags(default_volume_type\u003dvolume_types.DEFAULT_VOLUME_TYPE)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # Create volume"},{"line_number":88,"context_line":"        created_volume \u003d self.api.post_volume({\u0027volume\u0027: {\u0027size\u0027: 1}})"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_a4ccb46a","line":85,"updated":"2020-07-30 22:59:20.000000000","message":"So now we can\u0027t un-configure the default volume type, because it\u0027s required to have a value by the way it was defined.  So I think you can just delete lines 84-85.  (I think we\u0027re dealing with a real config object in these tests, and the flags just give you an easy way to override it.)","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        \"\"\"Verify volume_type is not None (should be system default type.)\"\"\""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        # un-configure operator default volume type"},{"line_number":85,"context_line":"        self.flags(default_volume_type\u003dvolume_types.DEFAULT_VOLUME_TYPE)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        # Create volume"},{"line_number":88,"context_line":"        created_volume \u003d self.api.post_volume({\u0027volume\u0027: {\u0027size\u0027: 1}})"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_e7a4e9bb","line":85,"in_reply_to":"9f560f44_a4ccb46a","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027creating\u0027])"},{"line_number":94,"context_line":"        self.assertEqual(\u0027available\u0027, found_volume[\u0027status\u0027])"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # It should have the system default volume_type"},{"line_number":97,"context_line":"        self.assertEqual(volume_types.DEFAULT_VOLUME_TYPE,"},{"line_number":98,"context_line":"                         found_volume[\u0027volume_type\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # Delete the volume"},{"line_number":101,"context_line":"        self.api.delete_volume(created_volume_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_84f53086","line":98,"range":{"start_line":96,"start_character":0,"end_line":98,"end_character":53},"updated":"2020-07-30 22:59:20.000000000","message":"OK, so this is no longer accurate.  It should have a volume_type, we just don\u0027t know what it is.  I think this could just be a assertIsNotNone(found_volume[\u0027volume_type\u0027])","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027creating\u0027])"},{"line_number":94,"context_line":"        self.assertEqual(\u0027available\u0027, found_volume[\u0027status\u0027])"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # It should have the system default volume_type"},{"line_number":97,"context_line":"        self.assertEqual(volume_types.DEFAULT_VOLUME_TYPE,"},{"line_number":98,"context_line":"                         found_volume[\u0027volume_type\u0027])"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        # Delete the volume"},{"line_number":101,"context_line":"        self.api.delete_volume(created_volume_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_e78d0932","line":98,"range":{"start_line":96,"start_character":0,"end_line":98,"end_character":53},"in_reply_to":"9f560f44_84f53086","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027creating\u0027])"},{"line_number":90,"context_line":"        self.assertEqual(\u0027available\u0027, found_volume[\u0027status\u0027])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        # It should have the system default volume_type"},{"line_number":93,"context_line":"        self.assertIsNotNone(found_volume[\u0027volume_type\u0027])"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        # Delete the volume"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_f31ae27a","line":92,"updated":"2020-07-31 13:32:49.000000000","message":"s/the system default/a/","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027creating\u0027])"},{"line_number":90,"context_line":"        self.assertEqual(\u0027available\u0027, found_volume[\u0027status\u0027])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        # It should have the system default volume_type"},{"line_number":93,"context_line":"        self.assertIsNotNone(found_volume[\u0027volume_type\u0027])"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        # Delete the volume"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_f82e1690","line":92,"in_reply_to":"9f560f44_f31ae27a","updated":"2020-08-04 13:29:49.000000000","message":"Done","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        self.api.delete_volume(created_volume_id)"},{"line_number":97,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027deleting\u0027])"},{"line_number":98,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_create_volume_specified_type(self):"},{"line_number":101,"context_line":"        \"\"\"Verify volume_type is not default.\"\"\""},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_16f784f7","line":99,"updated":"2020-07-31 13:32:49.000000000","message":"I know it seems redundant, but we need another test here.  The one above makes sure the volume has a volume_type when we don\u0027t touch the config and there isn\u0027t a volume_type in the request.  The test below makes sure that the volume gets the volume_type we request when a type is included in the volume-create request.\n\nThe \"missing\" test is to configure a default volume type explicitly and make sure that we get it when a volume-create request is made that doesn\u0027t include a volume_type.  I think create a new v_t like in the below test, then set it as the default, and check to make sure it\u0027s the v_t on the volume at the end.  Something like that.","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        self.api.delete_volume(created_volume_id)"},{"line_number":97,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027deleting\u0027])"},{"line_number":98,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_create_volume_specified_type(self):"},{"line_number":101,"context_line":"        \"\"\"Verify volume_type is not default.\"\"\""},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_5820e29b","line":99,"in_reply_to":"9f560f44_16f784f7","updated":"2020-08-04 13:29:49.000000000","message":"Done","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b3dc105fa5b1c926a2cd23b4840ff038f6c8a55b","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        self.api.delete_volume(created_volume_id)"},{"line_number":121,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027deleting\u0027])"},{"line_number":122,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_create_volume_specified_type(self):"},{"line_number":125,"context_line":"        \"\"\"Verify volume_type is not default.\"\"\""},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_cd1ff205","line":123,"updated":"2020-08-05 22:53:12.000000000","message":"I wonder if we should have a test_create_volume_with_bad_default_vol_type functional test.  You have a unit test, but that doesn\u0027t look for a 500 coming from the API.","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a02de33d2ddd2fd66b5d8f289bb021676e41a1d5","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        self.api.delete_volume(created_volume_id)"},{"line_number":121,"context_line":"        found_volume \u003d self._poll_volume_while(created_volume_id, [\u0027deleting\u0027])"},{"line_number":122,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def test_create_volume_specified_type(self):"},{"line_number":125,"context_line":"        \"\"\"Verify volume_type is not default.\"\"\""},{"line_number":126,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9f560f44_28749cfd","line":123,"in_reply_to":"9f560f44_cd1ff205","updated":"2020-08-06 01:08:39.000000000","message":"Done","commit_id":"0a9eba85ff9322a03e788bdac9d57d4f9639e45e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"89bd474185c79620cea4d71d7addab8f24107763","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def test_create_volume_default_type(self):"},{"line_number":102,"context_line":"        \"\"\"Verify volume_type is default.\"\"\""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        my_vol_type_name \u003d \u0027default_type\u0027"},{"line_number":105,"context_line":"        self.api.create_type(my_vol_type_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f560f44_0908b5d7","line":102,"range":{"start_line":102,"start_character":11,"end_line":102,"end_character":40},"updated":"2020-08-06 12:34:15.000000000","message":"Nit: \"Verify that the configured default_volume_type is used\"","commit_id":"1879c337dda1cf13d5e7549ee28d8936d265a9ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4c0edca254dccc309734e3b989a2cc180f945a77","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        self.assertIsNone(found_volume)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def test_create_volume_default_type(self):"},{"line_number":102,"context_line":"        \"\"\"Verify volume_type is default.\"\"\""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        my_vol_type_name \u003d \u0027default_type\u0027"},{"line_number":105,"context_line":"        self.api.create_type(my_vol_type_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9f560f44_a44290a9","line":102,"range":{"start_line":102,"start_character":11,"end_line":102,"end_character":40},"in_reply_to":"9f560f44_0908b5d7","updated":"2020-08-06 13:56:46.000000000","message":"Done","commit_id":"1879c337dda1cf13d5e7549ee28d8936d265a9ba"}],"cinder/tests/unit/test_volume_types.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7b2fb112270e331a38b5ccfc6d1365c79ef05e6b","unresolved":false,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def test_non_existent_vol_type_shouldnt_delete(self):"},{"line_number":274,"context_line":"        \"\"\"Ensures that volume type creation fails with invalid args.\"\"\""},{"line_number":275,"context_line":"        # create a dummy type as DB requires atleast 1 type to perform the"},{"line_number":276,"context_line":"        # delete operation"},{"line_number":277,"context_line":"        volume_types.create(self.ctxt, self.vol_type1_name)"},{"line_number":278,"context_line":"        self.assertRaises(exception.VolumeTypeNotFound,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_8b4ef893","line":275,"range":{"start_line":275,"start_character":45,"end_line":275,"end_character":52},"updated":"2020-08-17 15:57:46.000000000","message":"at least","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b7ce76f800391ed574b02a14bbae865d322da386","unresolved":false,"context_lines":[{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def test_non_existent_vol_type_shouldnt_delete(self):"},{"line_number":274,"context_line":"        \"\"\"Ensures that volume type creation fails with invalid args.\"\"\""},{"line_number":275,"context_line":"        # create a dummy type as DB requires atleast 1 type to perform the"},{"line_number":276,"context_line":"        # delete operation"},{"line_number":277,"context_line":"        volume_types.create(self.ctxt, self.vol_type1_name)"},{"line_number":278,"context_line":"        self.assertRaises(exception.VolumeTypeNotFound,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_de826cc8","line":275,"range":{"start_line":275,"start_character":45,"end_line":275,"end_character":52},"in_reply_to":"9f560f44_8b4ef893","updated":"2020-08-17 16:27:26.000000000","message":"Done","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"}],"cinder/tests/unit/volume/flows/api/test_create_volume.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":53,"context_line":"         \u0027param_source_vol\u0027: None,"},{"line_number":54,"context_line":"         \u0027param_snap\u0027: None,"},{"line_number":55,"context_line":"         \u0027param_img_vol_type_id\u0027: None,"},{"line_number":56,"context_line":"         \u0027config_value\u0027: volume_types.DEFAULT_VOLUME_TYPE,"},{"line_number":57,"context_line":"         \u0027expected_vol_type\u0027: volume_types.DEFAULT_VOLUME_TYPE},"},{"line_number":58,"context_line":"        # case set 1: if a volume_type is passed, should always be selected"},{"line_number":59,"context_line":"        {\u0027param_vol_type\u0027: fake_vol_type,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_af3c8dc8","line":56,"updated":"2020-07-30 22:59:20.000000000","message":"I\u0027m not sure it still makes sense to have this in the data any more, but I don\u0027t have a better idea.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8517b2c9d05156014d78d890849702a185947c1e","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        test_fn \u003d create_volume.ExtractVolumeRequestTask._get_volume_type"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        if config_value:"},{"line_number":194,"context_line":"            self.assertRaises(exception.DefaultNotFound,"},{"line_number":195,"context_line":"                              test_fn,"},{"line_number":196,"context_line":"                              self.context,"},{"line_number":197,"context_line":"                              None,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_ecd493eb","line":194,"range":{"start_line":194,"start_character":30,"end_line":194,"end_character":55},"updated":"2020-08-04 18:12:16.000000000","message":"This needs an update","commit_id":"7ea871691f70535f7ac2ef9a9aaff228e28ce124"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fcff3623b9d6b1221254278d5645aefea547b570","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        test_fn \u003d create_volume.ExtractVolumeRequestTask._get_volume_type"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"        if config_value:"},{"line_number":194,"context_line":"            self.assertRaises(exception.DefaultNotFound,"},{"line_number":195,"context_line":"                              test_fn,"},{"line_number":196,"context_line":"                              self.context,"},{"line_number":197,"context_line":"                              None,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_37053412","line":194,"range":{"start_line":194,"start_character":30,"end_line":194,"end_character":55},"in_reply_to":"9f560f44_ecd493eb","updated":"2020-08-04 18:38:56.000000000","message":"Done","commit_id":"7ea871691f70535f7ac2ef9a9aaff228e28ce124"}],"cinder/volume/volume_types.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    elevated \u003d context if context.is_admin else context.elevated()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    delete \u003d True"},{"line_number":112,"context_line":"    try:"},{"line_number":113,"context_line":"        default_type \u003d get_default_volume_type()"},{"line_number":114,"context_line":"        # don\u0027t allow delete if the type requested is the default type"},{"line_number":115,"context_line":"        if id \u003d\u003d default_type[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_6fb395eb","line":112,"range":{"start_line":112,"start_character":0,"end_line":112,"end_character":8},"updated":"2020-07-30 22:59:20.000000000","message":"if you make the change i suggest at line 197, you won\u0027t need this try.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    elevated \u003d context if context.is_admin else context.elevated()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    delete \u003d True"},{"line_number":112,"context_line":"    try:"},{"line_number":113,"context_line":"        default_type \u003d get_default_volume_type()"},{"line_number":114,"context_line":"        # don\u0027t allow delete if the type requested is the default type"},{"line_number":115,"context_line":"        if id \u003d\u003d default_type[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_27f701f8","line":112,"range":{"start_line":112,"start_character":0,"end_line":112,"end_character":8},"in_reply_to":"9f560f44_6fb395eb","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        default_type \u003d get_default_volume_type()"},{"line_number":114,"context_line":"        # don\u0027t allow delete if the type requested is the default type"},{"line_number":115,"context_line":"        if id \u003d\u003d default_type[\u0027id\u0027]:"},{"line_number":116,"context_line":"            delete \u003d False"},{"line_number":117,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":118,"context_line":"        # if the configured default doesn\u0027t exist"},{"line_number":119,"context_line":"        delete \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_0f0ad988","line":116,"range":{"start_line":116,"start_character":12,"end_line":116,"end_character":26},"updated":"2020-07-30 22:59:20.000000000","message":"instead, i think you should raise the old VolumeTypeDefault (the one you\u0027re replacing with VolumeTypeDeleteException) exception here, and you won\u0027t need the \u0027delete\u0027 flag any more.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        default_type \u003d get_default_volume_type()"},{"line_number":114,"context_line":"        # don\u0027t allow delete if the type requested is the default type"},{"line_number":115,"context_line":"        if id \u003d\u003d default_type[\u0027id\u0027]:"},{"line_number":116,"context_line":"            delete \u003d False"},{"line_number":117,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":118,"context_line":"        # if the configured default doesn\u0027t exist"},{"line_number":119,"context_line":"        delete \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_87dd2d72","line":116,"range":{"start_line":116,"start_character":12,"end_line":116,"end_character":26},"in_reply_to":"9f560f44_0f0ad988","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f21e54d38b20a84863a5a13dd7a2327ba5874da0","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        # TODO(zhiteng) consider add notification to warn admin"},{"line_number":195,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"},{"line_number":196,"context_line":"                      \u0027Please check default_volume_type config:\u0027)"},{"line_number":197,"context_line":"        raise exception.VolumeTypeNotFoundByName(volume_type_name\u003dname)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    return vol_type"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_afa3ed9d","line":197,"range":{"start_line":197,"start_character":14,"end_line":197,"end_character":71},"updated":"2020-07-30 22:59:20.000000000","message":"I don\u0027t know that this is the exception we want to raise here.  It\u0027s not simply that the volume type wasn\u0027t found, it\u0027s that the configured *default* volume type doesn\u0027t exist.  What I\u0027m thinking is that if this is raised when a user made a volume-create request, and CONF.default_volume_type\u003d\u003d\u0027foo\u0027 but foo doesn\u0027t exist, the user gets \"Volume type with name \u0027foo\u0027 could not be found\" and they wonder what this means because they didn\u0027t ask for volume type foo.","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"82b444246c11c8a07fc95dac0aeb6d4b85a649a3","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        # TODO(zhiteng) consider add notification to warn admin"},{"line_number":195,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"},{"line_number":196,"context_line":"                      \u0027Please check default_volume_type config:\u0027)"},{"line_number":197,"context_line":"        raise exception.VolumeTypeNotFoundByName(volume_type_name\u003dname)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    return vol_type"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_47f4b5f2","line":197,"range":{"start_line":197,"start_character":14,"end_line":197,"end_character":71},"in_reply_to":"9f560f44_afa3ed9d","updated":"2020-07-31 08:57:30.000000000","message":"Done","commit_id":"9b83d4b090686d4bca2337a50167c127fe3e41ff"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"edf64ca53c5dfed81b39c873d5bddd7bdecb8055","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":186,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":187,"context_line":"        # flag, record this issue and raise exception"},{"line_number":188,"context_line":"        # TODO(zhiteng) consider add notification to warn admin"},{"line_number":189,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"},{"line_number":190,"context_line":"                      \u0027Please check default_volume_type config:\u0027)"},{"line_number":191,"context_line":"        raise exception.DefaultNotFound(volume_type_name\u003dname)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_1304b67a","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":63},"updated":"2020-07-31 13:32:49.000000000","message":"I think you can remove this, we\u0027re not going to do it.  This is from when the volume-create would still succeed with a None volume_type.  Now the admin will get plenty of notification, namely by all the users who can\u0027t create volumes!","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"46083a1a2d0ded30853bd0f9518d97f33d47b7db","unresolved":false,"context_lines":[{"line_number":185,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":186,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":187,"context_line":"        # flag, record this issue and raise exception"},{"line_number":188,"context_line":"        # TODO(zhiteng) consider add notification to warn admin"},{"line_number":189,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"},{"line_number":190,"context_line":"                      \u0027Please check default_volume_type config:\u0027)"},{"line_number":191,"context_line":"        raise exception.DefaultNotFound(volume_type_name\u003dname)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_38252eac","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":63},"in_reply_to":"9f560f44_1304b67a","updated":"2020-08-04 13:29:49.000000000","message":"Done","commit_id":"d43219d44a5e66855c9f1de283452ad14ae4dd2c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1b5f6e846b4e2427d1e7e0c1824305329771c634","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"def destroy(context, id):"},{"line_number":105,"context_line":"    \"\"\"Marks volume types as deleted.\"\"\""},{"line_number":106,"context_line":"    if id is None:"},{"line_number":107,"context_line":"        msg \u003d _(\"id cannot be None\")"},{"line_number":108,"context_line":"        raise exception.InvalidVolumeType(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_5d7a87f0","line":105,"updated":"2020-08-04 21:49:38.000000000","message":"You need to document here that there must always be at least one volume type in a cinder installation and how this function accomplishes that.","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"28333b8a357983ddc58d23b065c3a73d17433258","unresolved":false,"context_lines":[{"line_number":102,"context_line":""},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"def destroy(context, id):"},{"line_number":105,"context_line":"    \"\"\"Marks volume types as deleted.\"\"\""},{"line_number":106,"context_line":"    if id is None:"},{"line_number":107,"context_line":"        msg \u003d _(\"id cannot be None\")"},{"line_number":108,"context_line":"        raise exception.InvalidVolumeType(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_40a29baf","line":105,"in_reply_to":"9f560f44_5d7a87f0","updated":"2020-08-05 09:39:17.000000000","message":"Done","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1b5f6e846b4e2427d1e7e0c1824305329771c634","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    elevated \u003d context if context.is_admin else context.elevated()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    # Not found exception is handled by the caller"},{"line_number":112,"context_line":"    default_type \u003d get_default_volume_type()"},{"line_number":113,"context_line":"    # don\u0027t allow delete if the type requested is the default type"},{"line_number":114,"context_line":"    if id \u003d\u003d default_type[\u0027id\u0027]:"},{"line_number":115,"context_line":"        raise exception.VolumeTypeDefaultDeletionError(volume_type_id\u003did)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_9d0b7f7b","line":112,"updated":"2020-08-04 21:49:38.000000000","message":"This is key to your scheme, that no volume-type deletions are allowed until the default volume type config option specifies an existing volume-type. I guess what I\u0027m saying is that you might want to say something stronger at line 111.  But the other thing I\u0027m thinking is that if someone refactors the function at line 179 so it doesn\u0027t raise, or puts line 112 into a try/catch, or eliminates line 112 as unnecessary because we check at line 114 to make sure the default volume-type isn\u0027t deleted (all of which might seem reasonable in isolation), then it will be possible to delete all the volume types from cinder.  I really think we need a check at the database layer as a safeguard.","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"28333b8a357983ddc58d23b065c3a73d17433258","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    elevated \u003d context if context.is_admin else context.elevated()"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    # Not found exception is handled by the caller"},{"line_number":112,"context_line":"    default_type \u003d get_default_volume_type()"},{"line_number":113,"context_line":"    # don\u0027t allow delete if the type requested is the default type"},{"line_number":114,"context_line":"    if id \u003d\u003d default_type[\u0027id\u0027]:"},{"line_number":115,"context_line":"        raise exception.VolumeTypeDefaultDeletionError(volume_type_id\u003did)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_5be9eab4","line":112,"in_reply_to":"9f560f44_9d0b7f7b","updated":"2020-08-05 09:39:17.000000000","message":"Future refactoring makes sense for an additional check in DB but what i was confused about is which exception to raise at DB cause we don\u0027t know which condition was modified. With your reasoning, most probably if code reaches DB and only 1 type is exists then possibly the default type isn\u0027t set properly so i will raise that there.","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1b5f6e846b4e2427d1e7e0c1824305329771c634","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"def get_default_volume_type():"},{"line_number":180,"context_line":"    \"\"\"Get the default volume type.\"\"\""},{"line_number":181,"context_line":"    name \u003d CONF.default_volume_type"},{"line_number":182,"context_line":"    ctxt \u003d context.get_admin_context()"},{"line_number":183,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_fdea7bd5","line":180,"updated":"2020-08-04 21:49:38.000000000","message":"Can you beef up this docstring?  It\u0027s really important that this function raises an exception when the default volume type isn\u0027t found, so please add something like:\n\n:raises VolumeTypeDefaultNotFound: when the configured default is not found","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"28333b8a357983ddc58d23b065c3a73d17433258","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"def get_default_volume_type():"},{"line_number":180,"context_line":"    \"\"\"Get the default volume type.\"\"\""},{"line_number":181,"context_line":"    name \u003d CONF.default_volume_type"},{"line_number":182,"context_line":"    ctxt \u003d context.get_admin_context()"},{"line_number":183,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d6b2f148","line":180,"in_reply_to":"9f560f44_fdea7bd5","updated":"2020-08-05 09:39:17.000000000","message":"Done","commit_id":"562203fdc6688cdf21ef3cca5c92cde1182f7807"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"7b2fb112270e331a38b5ccfc6d1365c79ef05e6b","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    # delete operation."},{"line_number":125,"context_line":"    default_type \u003d get_default_volume_type()"},{"line_number":126,"context_line":"    # don\u0027t allow delete if the type requested is the default type"},{"line_number":127,"context_line":"    if id \u003d\u003d default_type.get(\u0027id\u0027):"},{"line_number":128,"context_line":"        raise exception.VolumeTypeDefaultDeletionError(volume_type_id\u003did)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    return db.volume_type_destroy(elevated, id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_4b02200d","line":127,"range":{"start_line":127,"start_character":7,"end_line":127,"end_character":9},"updated":"2020-08-17 15:57:46.000000000","message":"Probably would be better not to use a Python keyword for variable name.","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b7ce76f800391ed574b02a14bbae865d322da386","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    # delete operation."},{"line_number":125,"context_line":"    default_type \u003d get_default_volume_type()"},{"line_number":126,"context_line":"    # don\u0027t allow delete if the type requested is the default type"},{"line_number":127,"context_line":"    if id \u003d\u003d default_type.get(\u0027id\u0027):"},{"line_number":128,"context_line":"        raise exception.VolumeTypeDefaultDeletionError(volume_type_id\u003did)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    return db.volume_type_destroy(elevated, id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9f560f44_1ee6a417","line":127,"range":{"start_line":127,"start_character":7,"end_line":127,"end_character":9},"in_reply_to":"9f560f44_4b02200d","updated":"2020-08-17 16:27:26.000000000","message":"Makes sense but this is the type id received at L#104 and is used at L#116.\nAlso there are multiple occurrences in our API doing the same.\nI think refactoring all the places together would be better?","commit_id":"871b81186428109707ad9f2a95361c6eaf00888e"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"890c7fea702b3f9961980e214cfb48773992b760","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    vol_type \u003d {}"},{"line_number":201,"context_line":"    try:"},{"line_number":202,"context_line":"        vol_type \u003d get_volume_type_by_name(ctxt, name)"},{"line_number":203,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":204,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":205,"context_line":"        # flag, record this issue and raise exception"},{"line_number":206,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_0492c04b","line":203,"updated":"2020-08-18 21:17:03.000000000","message":"I think you should also catch InvalidVolumeType here so that if default_volume_type is explicitly configured as None, we\u0027ll raise the DefaultMisconfiguredError.","commit_id":"12a1121549832d8b54dd9f0aac8acc574931c848"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1d6ea32ebcb7adb7b1c0305d12d145a496eafc99","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    vol_type \u003d {}"},{"line_number":201,"context_line":"    try:"},{"line_number":202,"context_line":"        vol_type \u003d get_volume_type_by_name(ctxt, name)"},{"line_number":203,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":204,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":205,"context_line":"        # flag, record this issue and raise exception"},{"line_number":206,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_caa60f54","line":203,"in_reply_to":"9f560f44_0492c04b","updated":"2020-08-19 02:07:51.000000000","message":"I will add it but out of curiosity I\u0027ve a doubt, I\u0027m not sure how it can be configured as None, the config option is parsed as a string so if we set it to None, at L#198, the value of name is \u0027None\u0027 which results in volume type not found exception.\nThe only possible way is setting it to \u0027\u0027 (blank string) but that also doesn\u0027t raises InvalidVolumeType exception (L#185 only checks for None and not for blank strings).","commit_id":"12a1121549832d8b54dd9f0aac8acc574931c848"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"3791523c1b46cbbbb1ccc08ea0bf73fdd4f69466","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    vol_type \u003d {}"},{"line_number":201,"context_line":"    try:"},{"line_number":202,"context_line":"        vol_type \u003d get_volume_type_by_name(ctxt, name)"},{"line_number":203,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":204,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":205,"context_line":"        # flag, record this issue and raise exception"},{"line_number":206,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_5d7122de","line":203,"in_reply_to":"9f560f44_15a4a8b8","updated":"2020-08-19 10:42:28.000000000","message":"Yeah,\ndefault_volume_type \u003d\ndefault_volume_type \u003d \u0027\u0027\nare treated as same cases with same not found error\nERROR cinder.volume.volume_types [None req-82fc21cf-777d-4fc0-ba34-b782457caaeb admin admin] Default volume type is not found. Please check default_volume_type config:: cinder.exception.VolumeTypeNotFoundByName: Volume type with name  could not be found.","commit_id":"12a1121549832d8b54dd9f0aac8acc574931c848"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a389cc1ba833de54cd63c962abcc50be45a0dec6","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    vol_type \u003d {}"},{"line_number":201,"context_line":"    try:"},{"line_number":202,"context_line":"        vol_type \u003d get_volume_type_by_name(ctxt, name)"},{"line_number":203,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":204,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":205,"context_line":"        # flag, record this issue and raise exception"},{"line_number":206,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_f563f4da","line":203,"in_reply_to":"9f560f44_caa60f54","updated":"2020-08-19 02:09:43.000000000","message":"Done","commit_id":"12a1121549832d8b54dd9f0aac8acc574931c848"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b9e8880f7b4e409532bb57f285794ca6c4aaaffd","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    vol_type \u003d {}"},{"line_number":201,"context_line":"    try:"},{"line_number":202,"context_line":"        vol_type \u003d get_volume_type_by_name(ctxt, name)"},{"line_number":203,"context_line":"    except exception.VolumeTypeNotFoundByName:"},{"line_number":204,"context_line":"        # Couldn\u0027t find volume type with the name in default_volume_type"},{"line_number":205,"context_line":"        # flag, record this issue and raise exception"},{"line_number":206,"context_line":"        LOG.exception(\u0027Default volume type is not found. \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_15a4a8b8","line":203,"in_reply_to":"9f560f44_caa60f54","updated":"2020-08-19 02:43:40.000000000","message":"Your explanation makes sense, i.e., that whatever value is there will be treated as a string.  So I guess the degenerative case to consider is if someone has\n\n  default_volume_type \u003d\n\nin the config file, which I think is basically what you said here, that is, setting the value to the empty string \u0027\u0027.","commit_id":"12a1121549832d8b54dd9f0aac8acc574931c848"}],"releasenotes/notes/allow-deleting-__DEFAULT__-type-d35dfb5d89760b9b.yaml":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"e2cddd454face9524230ebc25f454f4c455f5a36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9f560f44_7bc4a9f5","line":12,"updated":"2020-08-05 13:36:40.000000000","message":"I have some ideas about how we can improve this and the commit message.  I\u0027ll try some ideas out in the etherpad we were using to discuss this issue:\n\nhttps://etherpad.opendev.org/p/__DEFAULT___type_discussion","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8cbbf6b8c1d4627a089331748f1845748d455262","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9f560f44_17ebb9db","line":12,"in_reply_to":"9f560f44_7bc4a9f5","updated":"2020-08-05 21:23:03.000000000","message":"Thanks a lot for the detailed release note, i knew you had something better in mind.\nDone","commit_id":"5bdf203b393397933f1596db2417a0e4ba682b45"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6a4c18f1f5837506a4439d32736e2b5122390c68","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    * A request to delete the currently configured ``default_volume_type``"},{"line_number":31,"context_line":"      will fail.  (You can delete that volume-type, but you cannot do it"},{"line_number":32,"context_line":"      while it is the value of the configuration option.)"},{"line_number":33,"context_line":"    * There must always be at least one volume-type defined in a Cinder"},{"line_number":34,"context_line":"      installation."},{"line_number":35,"context_line":"    * If the ``default_volume_type`` is misconfigured (that is, if the value"},{"line_number":36,"context_line":"      refers to a non-existent volume-type), requests that rely on the"},{"line_number":37,"context_line":"      default volume-type (for example, a volume-create request that does"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"9f560f44_49346d4a","line":34,"range":{"start_line":33,"start_character":6,"end_line":34,"end_character":19},"updated":"2020-08-06 12:59:46.000000000","message":"I think we should add:\n\nThis is enforced by the type-delete call.\n\n(Just to make it clear that we\u0027re not relying on the operator for this, it\u0027s enforced in the code.)","commit_id":"1879c337dda1cf13d5e7549ee28d8936d265a9ba"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"4c0edca254dccc309734e3b989a2cc180f945a77","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    * A request to delete the currently configured ``default_volume_type``"},{"line_number":31,"context_line":"      will fail.  (You can delete that volume-type, but you cannot do it"},{"line_number":32,"context_line":"      while it is the value of the configuration option.)"},{"line_number":33,"context_line":"    * There must always be at least one volume-type defined in a Cinder"},{"line_number":34,"context_line":"      installation."},{"line_number":35,"context_line":"    * If the ``default_volume_type`` is misconfigured (that is, if the value"},{"line_number":36,"context_line":"      refers to a non-existent volume-type), requests that rely on the"},{"line_number":37,"context_line":"      default volume-type (for example, a volume-create request that does"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"9f560f44_448294c9","line":34,"range":{"start_line":33,"start_character":6,"end_line":34,"end_character":19},"in_reply_to":"9f560f44_49346d4a","updated":"2020-08-06 13:56:46.000000000","message":"Done","commit_id":"1879c337dda1cf13d5e7549ee28d8936d265a9ba"}]}