)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7527fe3613723646104ea23e024f9ca9f8b3703d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     zhangbailin \u003czhangbailin@inspur.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-09-26 23:16:08 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support volume_type in compute api"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add compute API version for when a ``volume_type`` is requested."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_a7c8fe4c","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":38},"updated":"2018-09-28 03:10:57.000000000","message":"Maybe it\u0027s good to say \"Verify the volume type in the request\"","commit_id":"a7a96786ec1f71174923aca3c629735a8eb290fd"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"1a7cab5df87d148f8f2b93cbe8938319c5fd8a3b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     zhangbailin \u003czhangbailin@inspur.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-09-28 04:48:40 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Verify the volume type is supported in compute API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add compute API validation for when a ``volume_type`` is requested."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Part of bp boot-instance-specific-storage-backend"},{"line_number":12,"context_line":"Change-Id: I45bd42908d44a0f05e1231febab926b23232b57b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f79a3b5_084d3e70","line":9,"range":{"start_line":7,"start_character":0,"end_line":9,"end_character":67},"updated":"2018-09-28 09:05:56.000000000","message":"Just say that you are adding a new microversion to\nsupport specify ``volume_type`` when boot instances.","commit_id":"4d9a76b88797c47c209332348560a0cfe6ca97bc"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a1fc7a014df28d6ba7ef2e0fa10a556f4f80df31","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     zhangbailin \u003czhangbailin@inspur.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-09-28 04:48:40 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Verify the volume type is supported in compute API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add compute API validation for when a ``volume_type`` is requested."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Part of bp boot-instance-specific-storage-backend"},{"line_number":12,"context_line":"Change-Id: I45bd42908d44a0f05e1231febab926b23232b57b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3f79a3b5_648f3a2e","line":9,"range":{"start_line":7,"start_character":0,"end_line":9,"end_character":67},"in_reply_to":"3f79a3b5_084d3e70","updated":"2018-09-28 12:29:04.000000000","message":"Done","commit_id":"4d9a76b88797c47c209332348560a0cfe6ca97bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2018-09-29 08:13:06 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add compute API version for when a ``volume_type`` is requested"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Part of bp boot-instance-specific-storage-backend"},{"line_number":10,"context_line":"Change-Id: I45bd42908d44a0f05e1231febab926b23232b57b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3f79a3b5_da64b74b","line":8,"updated":"2018-10-01 16:31:06.000000000","message":"I would mention something like, \"This adds validation in the compute API for when a block_device_mapping_v2 request specifies a volume_type. Note that this is all noop code right now since users cannot specify that field in the API until the microversion is added which enables that feature. In other words, this is just plumbing.\"","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2018-09-29 08:13:06 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add compute API version for when a ``volume_type`` is requested"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Part of bp boot-instance-specific-storage-backend"},{"line_number":10,"context_line":"Change-Id: I45bd42908d44a0f05e1231febab926b23232b57b"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3f79a3b5_f76d0167","line":8,"in_reply_to":"3f79a3b5_da64b74b","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b531f975e7e1a1bf703bc65ebb9cd1f4ed9baf74","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     zhangbailin \u003czhangbailin@inspur.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-10-09 23:21:31 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add compute API version for when a ``volume_type`` is requested"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds validation in the compute API for when a"},{"line_number":10,"context_line":"block_device_mapping_v2 request specifies a ``volume_type``. Note that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"3f79a3b5_c5a396af","line":7,"range":{"start_line":7,"start_character":16,"end_line":7,"end_character":23},"updated":"2018-10-11 16:06:45.000000000","message":"validation","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b65c8e23a4b6d9dc30b145fc913605ce3054e112","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     zhangbailin \u003czhangbailin@inspur.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-10-09 23:21:31 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add compute API version for when a ``volume_type`` is requested"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds validation in the compute API for when a"},{"line_number":10,"context_line":"block_device_mapping_v2 request specifies a ``volume_type``. Note that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"3f79a3b5_b33b3d37","line":7,"range":{"start_line":7,"start_character":16,"end_line":7,"end_character":23},"in_reply_to":"3f79a3b5_c5a396af","updated":"2018-10-11 21:06:03.000000000","message":"Done","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"}],"doc/source/user/block-device-mapping.rst":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"12a9e9e22af9c11aff1082935d6357b561f82499","unresolved":false,"context_lines":[{"line_number":161,"context_line":"  usage is to set it to 0 for the boot device and leave it as None for any"},{"line_number":162,"context_line":"  other devices."},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"* volume_type - Microversion 2.67 to the servers create API to support"},{"line_number":165,"context_line":"  specifying volume type when booting instances. When we snapshot a"},{"line_number":166,"context_line":"  volume-backed server, the block_device_mapping_v2 image metadata will"},{"line_number":167,"context_line":"  include the volume_type from the BDM record so if the user then creates"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f79a3b5_bc5a374d","line":164,"range":{"start_line":164,"start_character":16,"end_line":164,"end_character":41},"updated":"2018-09-29 06:57:57.000000000","message":"I\u0027m not english expert, but I suggest change this to:\n\"Added in microversion 2.67 to...\"","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"759fde30e2074ef2af07fc7a9dd197f2f78f0001","unresolved":false,"context_lines":[{"line_number":161,"context_line":"  usage is to set it to 0 for the boot device and leave it as None for any"},{"line_number":162,"context_line":"  other devices."},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"* volume_type - Microversion 2.67 to the servers create API to support"},{"line_number":165,"context_line":"  specifying volume type when booting instances. When we snapshot a"},{"line_number":166,"context_line":"  volume-backed server, the block_device_mapping_v2 image metadata will"},{"line_number":167,"context_line":"  include the volume_type from the BDM record so if the user then creates"}],"source_content_type":"text/x-rst","patch_set":7,"id":"3f79a3b5_5f16754c","line":164,"range":{"start_line":164,"start_character":16,"end_line":164,"end_character":41},"in_reply_to":"3f79a3b5_bc5a374d","updated":"2018-09-29 07:10:02.000000000","message":"Done","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"}],"nova/api/openstack/common.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":535,"context_line":"    return api_version_request.is_supported(req, \u00272.60\u0027)"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"def supports_volume_type(req):"},{"line_number":539,"context_line":"    \"\"\"Check to see if the requested API version is high enough for volume_type"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    Microversion 2.67 adds support for specifying a volume_type when booting a"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_3ad84bee","line":538,"updated":"2018-10-01 16:31:06.000000000","message":"Everything under nova/api/openstack/* should be in the final change in the series which adds the new microversion. *This* change should just be the nova/compute/api.py validation code and nova/volume/cinder.py client side volume API code, plus tests.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":535,"context_line":"    return api_version_request.is_supported(req, \u00272.60\u0027)"},{"line_number":536,"context_line":""},{"line_number":537,"context_line":""},{"line_number":538,"context_line":"def supports_volume_type(req):"},{"line_number":539,"context_line":"    \"\"\"Check to see if the requested API version is high enough for volume_type"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"    Microversion 2.67 adds support for specifying a volume_type when booting a"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_573f15de","line":538,"in_reply_to":"3f79a3b5_3ad84bee","updated":"2018-10-02 03:50:17.000000000","message":"Remove this to the final patch, and will do a test case in that patch.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"}],"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                    flavor_id, ctxt\u003dcontext, read_deleted\u003d\"no\")"},{"line_number":581,"context_line":""},{"line_number":582,"context_line":"            supports_multiattach \u003d common.supports_multiattach_volume(req)"},{"line_number":583,"context_line":"            supports_volume_type \u003d common.supports_volume_type(req)"},{"line_number":584,"context_line":"            (instances, resv_id) \u003d self.compute_api.create(context,"},{"line_number":585,"context_line":"                            inst_type,"},{"line_number":586,"context_line":"                            image_uuid,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_ba921bfb","line":583,"updated":"2018-10-01 16:31:06.000000000","message":"Move this into the final change in the series that actually adds the new microversion, otherwise this is weird without any schema validation changes.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                    flavor_id, ctxt\u003dcontext, read_deleted\u003d\"no\")"},{"line_number":581,"context_line":""},{"line_number":582,"context_line":"            supports_multiattach \u003d common.supports_multiattach_volume(req)"},{"line_number":583,"context_line":"            supports_volume_type \u003d common.supports_volume_type(req)"},{"line_number":584,"context_line":"            (instances, resv_id) \u003d self.compute_api.create(context,"},{"line_number":585,"context_line":"                            inst_type,"},{"line_number":586,"context_line":"                            image_uuid,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_972e0da9","line":583,"in_reply_to":"3f79a3b5_ba921bfb","updated":"2018-10-02 03:50:17.000000000","message":"True, move it to the final patch.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"}],"nova/compute/api.py":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"524a4830d3be831202bcf8ecef826082ad02d279","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"CINDER_V3_VOLUME_TYPE_MIN_COMPUTE_VERSION \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_2ea94624","line":109,"range":{"start_line":109,"start_character":0,"end_line":109,"end_character":46},"updated":"2018-09-27 03:33:42.000000000","message":"As Matt and I menioned in your another patch\u0027s review, this seems not needed for now, this is only needed if we have something new to do in the compute/manager.py and we will bump the RPC version of the compute manager service, and then we add this thing in API layer to check whether the compute can work with this new request. For your current patch, 1. there seems to be that there are no new changes in compute manager, so you don\u0027t need this; 2. if there will be some new changes in the compute manager, then you should do that first and then bump the service version, and finally do this.","commit_id":"a7a96786ec1f71174923aca3c629735a8eb290fd"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a3ab6c04a36d6c08ec10fb581e0fff24ff55e790","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"CINDER_V3_VOLUME_TYPE_MIN_COMPUTE_VERSION \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a9831055","line":109,"range":{"start_line":109,"start_character":0,"end_line":109,"end_character":46},"in_reply_to":"3f79a3b5_2ea94624","updated":"2018-09-27 03:46:37.000000000","message":"Before this, may be there is the compute manager need to be change. As Matt mentioned, I need to reorganize it.","commit_id":"a7a96786ec1f71174923aca3c629735a8eb290fd"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7527fe3613723646104ea23e024f9ca9f8b3703d","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"CINDER_V3_VOLUME_TYPE_MIN_COMPUTE_VERSION \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_67bd06b0","line":109,"range":{"start_line":109,"start_character":0,"end_line":109,"end_character":46},"in_reply_to":"3f79a3b5_a9831055","updated":"2018-09-28 03:10:57.000000000","message":"Need to change \"CINDER_V3_VOLUME_TYPE_MIN_COMPUTE_VERSION \u003d 35\" to \"MIN_COMPUTE_VOLUME_TYPE \u003d 35\", to make sure that compute supports the minimum version of ``volume_type``.","commit_id":"a7a96786ec1f71174923aca3c629735a8eb290fd"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"1a7cab5df87d148f8f2b93cbe8938319c5fd8a3b","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"MIN_COMPUTE_VOLUME_TYPE \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_68ab92d6","line":109,"range":{"start_line":109,"start_character":26,"end_line":109,"end_character":28},"updated":"2018-09-28 09:05:56.000000000","message":"36","commit_id":"4d9a76b88797c47c209332348560a0cfe6ca97bc"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a1fc7a014df28d6ba7ef2e0fa10a556f4f80df31","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"MIN_COMPUTE_VOLUME_TYPE \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0498c673","line":109,"range":{"start_line":109,"start_character":26,"end_line":109,"end_character":28},"in_reply_to":"3f79a3b5_68ab92d6","updated":"2018-09-28 12:29:04.000000000","message":"Done","commit_id":"4d9a76b88797c47c209332348560a0cfe6ca97bc"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"18d90359ed677608b737b6a1c2d896937539c272","unresolved":false,"context_lines":[{"line_number":106,"context_line":"MIN_COMPUTE_MULTIATTACH \u003d 27"},{"line_number":107,"context_line":"MIN_COMPUTE_TRUSTED_CERTS \u003d 31"},{"line_number":108,"context_line":"MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION \u003d 34"},{"line_number":109,"context_line":"MIN_COMPUTE_VOLUME_TYPE \u003d 35"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"# FIXME(danms): Keep a global cache of the cells we find the"},{"line_number":112,"context_line":"# first time we look. This needs to be refreshed on a timer or"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_471fa008","line":109,"range":{"start_line":109,"start_character":26,"end_line":109,"end_character":28},"updated":"2018-09-28 12:35:52.000000000","message":"36","commit_id":"2c736b23fabd096770fa008d9ac8130c4566bca7"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"12a9e9e22af9c11aff1082935d6357b561f82499","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"                raise exception.InvalidGetVolumeTypes("},{"line_number":1401,"context_line":"                    volume_type\u003dvolume_type)"},{"line_number":1402,"context_line":""},{"line_number":1403,"context_line":"    def _check_volume_supports(self, context, bdm,"},{"line_number":1404,"context_line":"                               supports_volume_type\u003dFalse):"},{"line_number":1405,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1406,"context_line":"        # support volume_type is not used, fail the request."}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_fc12ef60","line":1403,"range":{"start_line":1403,"start_character":8,"end_line":1403,"end_character":29},"updated":"2018-09-29 06:57:57.000000000","message":"the function name is too general, consider change to _check_supports_volume_type()","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"759fde30e2074ef2af07fc7a9dd197f2f78f0001","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"                raise exception.InvalidGetVolumeTypes("},{"line_number":1401,"context_line":"                    volume_type\u003dvolume_type)"},{"line_number":1402,"context_line":""},{"line_number":1403,"context_line":"    def _check_volume_supports(self, context, bdm,"},{"line_number":1404,"context_line":"                               supports_volume_type\u003dFalse):"},{"line_number":1405,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1406,"context_line":"        # support volume_type is not used, fail the request."}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_ff20a19c","line":1403,"range":{"start_line":1403,"start_character":8,"end_line":1403,"end_character":29},"in_reply_to":"3f79a3b5_fc12ef60","updated":"2018-09-29 07:10:02.000000000","message":"Done","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"12a9e9e22af9c11aff1082935d6357b561f82499","unresolved":false,"context_lines":[{"line_number":1436,"context_line":""},{"line_number":1437,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1438,"context_line":"            # NOTE(Brin Zhang): Ensure the validity of volume_type"},{"line_number":1439,"context_line":"            self._check_requested_volume_type(context, bdm)"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"            # NOTE(Brin Zhang): Just make sure the volume type can be"},{"line_number":1442,"context_line":"            # supported when the bdm.volume_type is not None in request."},{"line_number":1443,"context_line":"            try:"},{"line_number":1444,"context_line":"                self._check_volume_supports(context, bdm, supports_volume_type)"},{"line_number":1445,"context_line":"            except (exception.VolumeTypeNotSupportedOldMicroversion,"},{"line_number":1446,"context_line":"                    exception.VolumeTypeSupportNotYetAvailable):"},{"line_number":1447,"context_line":"                raise"},{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"            # NOTE(vish): For now, just make sure the volumes are accessible."},{"line_number":1450,"context_line":"            # Additionally, check that the volume can be attached to this"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_3c1dc772","line":1447,"range":{"start_line":1439,"start_character":11,"end_line":1447,"end_character":21},"updated":"2018-09-29 06:57:57.000000000","message":"Could check if support volume type before calling cinder to validate volume type, this can avoid useless cross project calls.","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"759fde30e2074ef2af07fc7a9dd197f2f78f0001","unresolved":false,"context_lines":[{"line_number":1436,"context_line":""},{"line_number":1437,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1438,"context_line":"            # NOTE(Brin Zhang): Ensure the validity of volume_type"},{"line_number":1439,"context_line":"            self._check_requested_volume_type(context, bdm)"},{"line_number":1440,"context_line":""},{"line_number":1441,"context_line":"            # NOTE(Brin Zhang): Just make sure the volume type can be"},{"line_number":1442,"context_line":"            # supported when the bdm.volume_type is not None in request."},{"line_number":1443,"context_line":"            try:"},{"line_number":1444,"context_line":"                self._check_volume_supports(context, bdm, supports_volume_type)"},{"line_number":1445,"context_line":"            except (exception.VolumeTypeNotSupportedOldMicroversion,"},{"line_number":1446,"context_line":"                    exception.VolumeTypeSupportNotYetAvailable):"},{"line_number":1447,"context_line":"                raise"},{"line_number":1448,"context_line":""},{"line_number":1449,"context_line":"            # NOTE(vish): For now, just make sure the volumes are accessible."},{"line_number":1450,"context_line":"            # Additionally, check that the volume can be attached to this"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_df50450c","line":1447,"range":{"start_line":1439,"start_character":11,"end_line":1447,"end_character":21},"in_reply_to":"3f79a3b5_3c1dc772","updated":"2018-09-29 07:10:02.000000000","message":"Yeah, lgtm. In the previous function, it will be modified in the next patch.","commit_id":"80b75403c7bd8a7bbe7ca626ae6e1704a68f1b04"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_dd2a717c","line":1365,"updated":"2018-10-01 16:31:06.000000000","message":"I expected to see the validation of the requested volume type in this patch also - to make sure the volume type exists based on name or ID, and cache the results so we don\u0027t need to check that more than once per requested volume type.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_1a39c4d6","line":1365,"in_reply_to":"3f79a3b5_dd2a717c","updated":"2018-10-02 03:50:17.000000000","message":"Ok, move the validation of the requested volume type function from the final patch to this patch.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1369,"context_line":"        if volume_type and not supports_volume_type:"},{"line_number":1370,"context_line":"            raise exception.VolumeTypeNotSupportedOldMicroversion()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_3a74cbcf","line":1367,"range":{"start_line":1367,"start_character":13,"end_line":1367,"end_character":36},"updated":"2018-10-01 16:31:06.000000000","message":"Replace with \"If a volume_type is specified and the ...\".","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1369,"context_line":"        if volume_type and not supports_volume_type:"},{"line_number":1370,"context_line":"            raise exception.VolumeTypeNotSupportedOldMicroversion()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_9afcf40c","line":1367,"range":{"start_line":1367,"start_character":13,"end_line":1367,"end_character":36},"in_reply_to":"3f79a3b5_3a74cbcf","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1369,"context_line":"        if volume_type and not supports_volume_type:"},{"line_number":1370,"context_line":"            raise exception.VolumeTypeNotSupportedOldMicroversion()"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1373,"context_line":"            context, [\u0027nova-compute\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_fa2733c7","line":1370,"range":{"start_line":1369,"start_character":8,"end_line":1370,"end_character":67},"updated":"2018-10-01 16:31:06.000000000","message":"We shouldn\u0027t need this - the API request schema validation will handle this for us. The reason we check this explicitly for multiattach volumes is that nova doesn\u0027t support creating multiattach volumes, so it\u0027s not part of the compute API request schema, and we have to get the volume details (of a pre-existing volume) to determine if that volume is multiattach\u003dTrue.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":1366,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1367,"context_line":"        # If volume.volume_type\u003dTrue and the microversion to"},{"line_number":1368,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1369,"context_line":"        if volume_type and not supports_volume_type:"},{"line_number":1370,"context_line":"            raise exception.VolumeTypeNotSupportedOldMicroversion()"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1373,"context_line":"            context, [\u0027nova-compute\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_9717ed54","line":1370,"range":{"start_line":1369,"start_character":8,"end_line":1370,"end_character":67},"in_reply_to":"3f79a3b5_fa2733c7","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":1372,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1373,"context_line":"            context, [\u0027nova-compute\u0027])"},{"line_number":1374,"context_line":"        if volume_type and min_compute_version \u003c MIN_COMPUTE_VOLUME_TYPE:"},{"line_number":1375,"context_line":"            raise exception.VolumeTypeSupportNotYetAvailable()"},{"line_number":1376,"context_line":""},{"line_number":1377,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1378,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_fa009305","line":1375,"updated":"2018-10-01 16:31:06.000000000","message":"You should have a test for this case.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":1372,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1373,"context_line":"            context, [\u0027nova-compute\u0027])"},{"line_number":1374,"context_line":"        if volume_type and min_compute_version \u003c MIN_COMPUTE_VOLUME_TYPE:"},{"line_number":1375,"context_line":"            raise exception.VolumeTypeSupportNotYetAvailable()"},{"line_number":1376,"context_line":""},{"line_number":1377,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1378,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_f5dc1972","line":1375,"in_reply_to":"3f79a3b5_fa009305","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                try:"},{"line_number":1404,"context_line":"                    self._check_supports_volume_type(context, bdm,"},{"line_number":1405,"context_line":"                                                     supports_volume_type)"},{"line_number":1406,"context_line":"                except (exception.VolumeTypeNotSupportedOldMicroversion,"},{"line_number":1407,"context_line":"                        exception.VolumeTypeSupportNotYetAvailable):"},{"line_number":1408,"context_line":"                    raise"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"            # NOTE(vish): For now, just make sure the volumes are accessible."},{"line_number":1411,"context_line":"            # Additionally, check that the volume can be attached to this"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_ba49bb8e","line":1408,"range":{"start_line":1406,"start_character":16,"end_line":1408,"end_character":25},"updated":"2018-10-01 16:31:06.000000000","message":"You don\u0027t need this...it\u0027s redundant.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                try:"},{"line_number":1404,"context_line":"                    self._check_supports_volume_type(context, bdm,"},{"line_number":1405,"context_line":"                                                     supports_volume_type)"},{"line_number":1406,"context_line":"                except (exception.VolumeTypeNotSupportedOldMicroversion,"},{"line_number":1407,"context_line":"                        exception.VolumeTypeSupportNotYetAvailable):"},{"line_number":1408,"context_line":"                    raise"},{"line_number":1409,"context_line":""},{"line_number":1410,"context_line":"            # NOTE(vish): For now, just make sure the volumes are accessible."},{"line_number":1411,"context_line":"            # Additionally, check that the volume can be attached to this"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_35d7f151","line":1408,"range":{"start_line":1406,"start_character":16,"end_line":1408,"end_character":25},"in_reply_to":"3f79a3b5_ba49bb8e","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_requested_volume_type(self, context, bdm,"},{"line_number":1366,"context_line":"                                     volume_type, volume_types):"},{"line_number":1367,"context_line":"        \"\"\"If we are specifying a volume type, we need to get the"},{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_8108ab80","line":1365,"updated":"2018-10-05 16:06:31.000000000","message":"None of this is tested.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_requested_volume_type(self, context, bdm,"},{"line_number":1366,"context_line":"                                     volume_type, volume_types):"},{"line_number":1367,"context_line":"        \"\"\"If we are specifying a volume type, we need to get the"},{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_0406d786","line":1365,"in_reply_to":"3f79a3b5_8108ab80","updated":"2018-10-08 06:13:38.000000000","message":"Yeah, add this test case.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"},{"line_number":1369,"context_line":"        is available."},{"line_number":1370,"context_line":"        \"\"\""},{"line_number":1371,"context_line":"        def _get_volume_type_by_name(volume_type):"},{"line_number":1372,"context_line":"            for vol_type in volume_types:"},{"line_number":1373,"context_line":"                if volume_type \u003d\u003d vol_type[\u0027name\u0027]:"},{"line_number":1374,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_81faeb74","line":1371,"range":{"start_line":1371,"start_character":13,"end_line":1371,"end_character":16},"updated":"2018-10-05 16:06:31.000000000","message":"This method doesn\u0027t return anything, so having \"get\" in the name is odd.\n\nI\u0027d probably do something like this instead:\n\ndef _get_volume_type_name(volume_type_id_or_name):\n    for vol_type in volume_types:\n        if (volume_type_id_or_name \u003d\u003d vol_type[\u0027id\u0027] or\n                volume_type_id_or_name \u003d\u003d vol_type[\u0027name\u0027]):\n            return vol_type[\u0027name\u0027]\n    raise exception.VolumeTypeNotFound(id_or_name\u003dvolume_type_id_or_name)\n\nThen you can use the result of that method to store the name in the bdm.volume_type field.\n\nYou don\u0027t really need an inner method for it either...","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"},{"line_number":1369,"context_line":"        is available."},{"line_number":1370,"context_line":"        \"\"\""},{"line_number":1371,"context_line":"        def _get_volume_type_by_name(volume_type):"},{"line_number":1372,"context_line":"            for vol_type in volume_types:"},{"line_number":1373,"context_line":"                if volume_type \u003d\u003d vol_type[\u0027name\u0027]:"},{"line_number":1374,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_844d07c7","line":1371,"range":{"start_line":1371,"start_character":13,"end_line":1371,"end_character":16},"in_reply_to":"3f79a3b5_81faeb74","updated":"2018-10-08 06:13:38.000000000","message":"Yeah, this inner method doesnot need, because wo can use volume type lists to get all volume types, so rewrite this method.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1373,"context_line":"                if volume_type \u003d\u003d vol_type[\u0027name\u0027]:"},{"line_number":1374,"context_line":"                    break"},{"line_number":1375,"context_line":"            else:"},{"line_number":1376,"context_line":"                LOG.debug(\"The volume type %s is not invalid.\", volume_type)"},{"line_number":1377,"context_line":"                raise exception.InvalidBDMVolumeType(id_or_name\u003dvolume_type)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_61c8ef4c","line":1376,"updated":"2018-10-05 16:06:31.000000000","message":"We don\u0027t really need to log this here.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1373,"context_line":"                if volume_type \u003d\u003d vol_type[\u0027name\u0027]:"},{"line_number":1374,"context_line":"                    break"},{"line_number":1375,"context_line":"            else:"},{"line_number":1376,"context_line":"                LOG.debug(\"The volume type %s is not invalid.\", volume_type)"},{"line_number":1377,"context_line":"                raise exception.InvalidBDMVolumeType(id_or_name\u003dvolume_type)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_a4fa8374","line":1376,"in_reply_to":"3f79a3b5_61c8ef4c","updated":"2018-10-08 06:13:38.000000000","message":"Remove it.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1374,"context_line":"                    break"},{"line_number":1375,"context_line":"            else:"},{"line_number":1376,"context_line":"                LOG.debug(\"The volume type %s is not invalid.\", volume_type)"},{"line_number":1377,"context_line":"                raise exception.InvalidBDMVolumeType(id_or_name\u003dvolume_type)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"},{"line_number":1380,"context_line":"            # NOTE(Brin Zhang): Verify that the specified volume type exists."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_619a2f61","line":1377,"range":{"start_line":1377,"start_character":32,"end_line":1377,"end_character":52},"updated":"2018-10-05 16:06:31.000000000","message":"This exception is not defined in this patch, so clearly you\u0027re lacking test coverage of this case.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1374,"context_line":"                    break"},{"line_number":1375,"context_line":"            else:"},{"line_number":1376,"context_line":"                LOG.debug(\"The volume type %s is not invalid.\", volume_type)"},{"line_number":1377,"context_line":"                raise exception.InvalidBDMVolumeType(id_or_name\u003dvolume_type)"},{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"},{"line_number":1380,"context_line":"            # NOTE(Brin Zhang): Verify that the specified volume type exists."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_0479f7f9","line":1377,"range":{"start_line":1377,"start_character":32,"end_line":1377,"end_character":52},"in_reply_to":"3f79a3b5_619a2f61","updated":"2018-10-08 06:13:38.000000000","message":"It seems to have no use, remove it.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"},{"line_number":1380,"context_line":"            # NOTE(Brin Zhang): Verify that the specified volume type exists."},{"line_number":1381,"context_line":"            # And save the volume type name in the block_device_mapping table."},{"line_number":1382,"context_line":"            if uuidutils.is_uuid_like(volume_type):"},{"line_number":1383,"context_line":"                try:"},{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_411cd313","line":1381,"range":{"start_line":1381,"start_character":51,"end_line":1381,"end_character":77},"updated":"2018-10-05 16:06:31.000000000","message":"BlockDeviceMapping object.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1378,"context_line":""},{"line_number":1379,"context_line":"        try:"},{"line_number":1380,"context_line":"            # NOTE(Brin Zhang): Verify that the specified volume type exists."},{"line_number":1381,"context_line":"            # And save the volume type name in the block_device_mapping table."},{"line_number":1382,"context_line":"            if uuidutils.is_uuid_like(volume_type):"},{"line_number":1383,"context_line":"                try:"},{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_6439cb3e","line":1381,"range":{"start_line":1381,"start_character":51,"end_line":1381,"end_character":77},"in_reply_to":"3f79a3b5_411cd313","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"            # And save the volume type name in the block_device_mapping table."},{"line_number":1382,"context_line":"            if uuidutils.is_uuid_like(volume_type):"},{"line_number":1383,"context_line":"                try:"},{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("},{"line_number":1385,"context_line":"                        context, volume_type)"},{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e1bb9fe1","line":1385,"range":{"start_line":1384,"start_character":20,"end_line":1385,"end_character":45},"updated":"2018-10-05 16:06:31.000000000","message":"We have a list of volume types already, we shouldn\u0027t need to call the cinder API again.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"            # And save the volume type name in the block_device_mapping table."},{"line_number":1382,"context_line":"            if uuidutils.is_uuid_like(volume_type):"},{"line_number":1383,"context_line":"                try:"},{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("},{"line_number":1385,"context_line":"                        context, volume_type)"},{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e4007b6c","line":1385,"range":{"start_line":1384,"start_character":20,"end_line":1385,"end_character":45},"in_reply_to":"3f79a3b5_e1bb9fe1","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("},{"line_number":1385,"context_line":"                        context, volume_type)"},{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."},{"line_number":1389,"context_line":"                    bdm[\u0027volume_type\u0027] \u003d vol_type[\u0027name\u0027]"},{"line_number":1390,"context_line":"                except exception.VolumeTypeNotFound:"},{"line_number":1391,"context_line":"                    # If the uuid like name, try to get the volume type using"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_c189c31e","line":1388,"range":{"start_line":1387,"start_character":44,"end_line":1388,"end_character":40},"updated":"2018-10-05 16:06:31.000000000","message":"This comment doesn\u0027t really make sense. It doesn\u0027t matter if we pass a volume_type ID or name to the volume create API, cinder will lookup the volume type appropriately. If we are storing the name internally for consistency, then just say that.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"                    vol_type \u003d self.volume_api.get_volume_type_by_id("},{"line_number":1385,"context_line":"                        context, volume_type)"},{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."},{"line_number":1389,"context_line":"                    bdm[\u0027volume_type\u0027] \u003d vol_type[\u0027name\u0027]"},{"line_number":1390,"context_line":"                except exception.VolumeTypeNotFound:"},{"line_number":1391,"context_line":"                    # If the uuid like name, try to get the volume type using"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e40e3b8e","line":1388,"range":{"start_line":1387,"start_character":44,"end_line":1388,"end_character":40},"in_reply_to":"3f79a3b5_c189c31e","updated":"2018-10-08 06:13:38.000000000","message":"Yes, I am storing the name internally for consistency.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."},{"line_number":1389,"context_line":"                    bdm[\u0027volume_type\u0027] \u003d vol_type[\u0027name\u0027]"},{"line_number":1390,"context_line":"                except exception.VolumeTypeNotFound:"},{"line_number":1391,"context_line":"                    # If the uuid like name, try to get the volume type using"},{"line_number":1392,"context_line":"                    # name."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_c1cd4393","line":1389,"range":{"start_line":1389,"start_character":20,"end_line":1389,"end_character":38},"updated":"2018-10-05 16:06:31.000000000","message":"Use attribute notation: bdm.volume_type","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"                    # If the requested volume type is uuid, convert and store"},{"line_number":1387,"context_line":"                    # it as volume type name, because volume type name cannot"},{"line_number":1388,"context_line":"                    # be None in Cinder."},{"line_number":1389,"context_line":"                    bdm[\u0027volume_type\u0027] \u003d vol_type[\u0027name\u0027]"},{"line_number":1390,"context_line":"                except exception.VolumeTypeNotFound:"},{"line_number":1391,"context_line":"                    # If the uuid like name, try to get the volume type using"},{"line_number":1392,"context_line":"                    # name."}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_44404fa3","line":1389,"range":{"start_line":1389,"start_character":20,"end_line":1389,"end_character":38},"in_reply_to":"3f79a3b5_c1cd4393","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1393,"context_line":"                    _get_volume_type_by_name(volume_type)"},{"line_number":1394,"context_line":"            else:"},{"line_number":1395,"context_line":"                _get_volume_type_by_name(volume_type)"},{"line_number":1396,"context_line":"        except exception.CinderConnectionFailed:"},{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e18e3f25","line":1396,"updated":"2018-10-05 16:06:31.000000000","message":"Remove this.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1393,"context_line":"                    _get_volume_type_by_name(volume_type)"},{"line_number":1394,"context_line":"            else:"},{"line_number":1395,"context_line":"                _get_volume_type_by_name(volume_type)"},{"line_number":1396,"context_line":"        except exception.CinderConnectionFailed:"},{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_c4821fc8","line":1396,"in_reply_to":"3f79a3b5_e18e3f25","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_a1cb67a2","line":1400,"range":{"start_line":1400,"start_character":36,"end_line":1400,"end_character":56},"updated":"2018-10-05 16:06:31.000000000","message":"This isn\u0027t used.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_44a36f44","line":1400,"range":{"start_line":1400,"start_character":36,"end_line":1400,"end_character":56},"in_reply_to":"3f79a3b5_a1cb67a2","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1404,"context_line":"            context, [\u0027nova-compute\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_c1cc63c0","line":1401,"range":{"start_line":1401,"start_character":40,"end_line":1401,"end_character":63},"updated":"2018-10-05 16:06:31.000000000","message":"Nothing about this is checking microversions, it\u0027s checking the minimum nova-compute service version across the deployment since we don\u0027t know at this point where the server is going to be scheduled.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"    def _check_supports_volume_type(self, context, volume_type,"},{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1404,"context_line":"            context, [\u0027nova-compute\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_4a9059f4","line":1401,"range":{"start_line":1401,"start_character":40,"end_line":1401,"end_character":63},"in_reply_to":"3f79a3b5_c1cc63c0","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1404,"context_line":"            context, [\u0027nova-compute\u0027])"},{"line_number":1405,"context_line":"        if volume_type and min_compute_version \u003c MIN_COMPUTE_VOLUME_TYPE:"},{"line_number":1406,"context_line":"            raise exception.VolumeTypeSupportNotYetAvailable()"},{"line_number":1407,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_01ba5b0a","line":1404,"range":{"start_line":1403,"start_character":8,"end_line":1404,"end_character":38},"updated":"2018-10-05 16:06:31.000000000","message":"We don\u0027t need to run this if volume_type is None - if volume_type is never None in this method, then remove it from the conditional below.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1400,"context_line":"                                    supports_volume_type\u003dFalse):"},{"line_number":1401,"context_line":"        # If a volume_type is specified and the microversion to"},{"line_number":1402,"context_line":"        # support volume_type is not used, fail the request."},{"line_number":1403,"context_line":"        min_compute_version \u003d objects.service.get_minimum_version_all_cells("},{"line_number":1404,"context_line":"            context, [\u0027nova-compute\u0027])"},{"line_number":1405,"context_line":"        if volume_type and min_compute_version \u003c MIN_COMPUTE_VOLUME_TYPE:"},{"line_number":1406,"context_line":"            raise exception.VolumeTypeSupportNotYetAvailable()"},{"line_number":1407,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_6abe7565","line":1404,"range":{"start_line":1403,"start_character":8,"end_line":1404,"end_character":38},"in_reply_to":"3f79a3b5_01ba5b0a","updated":"2018-10-08 06:13:38.000000000","message":"To deal with this method in for loop, to ensure this method is called only once if the minimum nova-compute service version is new enough to support volume_type.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1432,"context_line":"            if volume_type:"},{"line_number":1433,"context_line":"                # NOTE(Brin Zhang): Just make sure the volume type can be"},{"line_number":1434,"context_line":"                # supported when the bdm.volume_type is not None in request."},{"line_number":1435,"context_line":"                self._check_supports_volume_type(context, bdm,"},{"line_number":1436,"context_line":"                                                 supports_volume_type)"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"                if not volume_types:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_616b0f86","line":1435,"range":{"start_line":1435,"start_character":58,"end_line":1435,"end_character":61},"updated":"2018-10-05 16:06:31.000000000","message":"This isn\u0027t used.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1432,"context_line":"            if volume_type:"},{"line_number":1433,"context_line":"                # NOTE(Brin Zhang): Just make sure the volume type can be"},{"line_number":1434,"context_line":"                # supported when the bdm.volume_type is not None in request."},{"line_number":1435,"context_line":"                self._check_supports_volume_type(context, bdm,"},{"line_number":1436,"context_line":"                                                 supports_volume_type)"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"                if not volume_types:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_0463371b","line":1435,"range":{"start_line":1435,"start_character":58,"end_line":1435,"end_character":61},"in_reply_to":"3f79a3b5_616b0f86","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":1441,"context_line":"                    try:"},{"line_number":1442,"context_line":"                        volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1443,"context_line":"                            context)"},{"line_number":1444,"context_line":"                    except exception.CinderConnectionFailed:"},{"line_number":1445,"context_line":"                        raise"},{"line_number":1446,"context_line":"                    except Exception:"},{"line_number":1447,"context_line":"                        LOG.error(\"Failed to get volume types.\")"},{"line_number":1448,"context_line":"                        raise exception.InvalidGetVolumeTypes()"},{"line_number":1449,"context_line":"                # NOTE(Brin Zhang): Ensure the validity of volume_type."},{"line_number":1450,"context_line":"                self._check_requested_volume_type(context, bdm,"},{"line_number":1451,"context_line":"                                                  volume_type, volume_types)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e15f1fe2","line":1448,"range":{"start_line":1444,"start_character":20,"end_line":1448,"end_character":63},"updated":"2018-10-05 16:06:31.000000000","message":"Why even make a distinction here? Either of these is going to result in a 500 response from the server create API so what does it matter to even try/except/raise here?","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":1441,"context_line":"                    try:"},{"line_number":1442,"context_line":"                        volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1443,"context_line":"                            context)"},{"line_number":1444,"context_line":"                    except exception.CinderConnectionFailed:"},{"line_number":1445,"context_line":"                        raise"},{"line_number":1446,"context_line":"                    except Exception:"},{"line_number":1447,"context_line":"                        LOG.error(\"Failed to get volume types.\")"},{"line_number":1448,"context_line":"                        raise exception.InvalidGetVolumeTypes()"},{"line_number":1449,"context_line":"                # NOTE(Brin Zhang): Ensure the validity of volume_type."},{"line_number":1450,"context_line":"                self._check_requested_volume_type(context, bdm,"},{"line_number":1451,"context_line":"                                                  volume_type, volume_types)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_e42b9b98","line":1448,"range":{"start_line":1444,"start_character":20,"end_line":1448,"end_character":63},"in_reply_to":"3f79a3b5_e15f1fe2","updated":"2018-10-08 06:13:38.000000000","message":"Yes, if get volume typs failed, will be return 500, this process is not make sense.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            block_device_mapping, shutdown_terminate,"},{"line_number":864,"context_line":"            instance_group, check_server_group_quota, filter_properties,"},{"line_number":865,"context_line":"            key_pair, tags, trusted_certs, supports_multiattach,"},{"line_number":866,"context_line":"            supports_volume_type, network_metadata\u003dNone):"},{"line_number":867,"context_line":"        # Check quotas"},{"line_number":868,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":869,"context_line":"                context, instance_type, min_count, max_count)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_113fde22","line":866,"range":{"start_line":866,"start_character":12,"end_line":866,"end_character":32},"updated":"2018-10-08 23:38:56.000000000","message":"Is this used?","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":863,"context_line":"            block_device_mapping, shutdown_terminate,"},{"line_number":864,"context_line":"            instance_group, check_server_group_quota, filter_properties,"},{"line_number":865,"context_line":"            key_pair, tags, trusted_certs, supports_multiattach,"},{"line_number":866,"context_line":"            supports_volume_type, network_metadata\u003dNone):"},{"line_number":867,"context_line":"        # Check quotas"},{"line_number":868,"context_line":"        num_instances \u003d compute_utils.check_num_instances_quota("},{"line_number":869,"context_line":"                context, instance_type, min_count, max_count)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_79b15801","line":866,"range":{"start_line":866,"start_character":12,"end_line":866,"end_character":32},"in_reply_to":"3f79a3b5_113fde22","updated":"2018-10-09 07:52:15.000000000","message":"Remove this parameter.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"               block_device_mapping, auto_disk_config, filter_properties,"},{"line_number":1119,"context_line":"               reservation_id\u003dNone, legacy_bdm\u003dTrue, shutdown_terminate\u003dFalse,"},{"line_number":1120,"context_line":"               check_server_group_quota\u003dFalse, tags\u003dNone,"},{"line_number":1121,"context_line":"               supports_multiattach\u003dFalse, supports_volume_type\u003dFalse,"},{"line_number":1122,"context_line":"               trusted_certs\u003dNone):"},{"line_number":1123,"context_line":"        \"\"\"Verify all the input parameters regardless of the provisioning"},{"line_number":1124,"context_line":"        strategy being performed and schedule the instance(s) for"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_d112c6b3","line":1121,"range":{"start_line":1121,"start_character":43,"end_line":1121,"end_character":63},"updated":"2018-10-08 23:38:56.000000000","message":"Do we need this?","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1118,"context_line":"               block_device_mapping, auto_disk_config, filter_properties,"},{"line_number":1119,"context_line":"               reservation_id\u003dNone, legacy_bdm\u003dTrue, shutdown_terminate\u003dFalse,"},{"line_number":1120,"context_line":"               check_server_group_quota\u003dFalse, tags\u003dNone,"},{"line_number":1121,"context_line":"               supports_multiattach\u003dFalse, supports_volume_type\u003dFalse,"},{"line_number":1122,"context_line":"               trusted_certs\u003dNone):"},{"line_number":1123,"context_line":"        \"\"\"Verify all the input parameters regardless of the provisioning"},{"line_number":1124,"context_line":"        strategy being performed and schedule the instance(s) for"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_99b474f3","line":1121,"range":{"start_line":1121,"start_character":43,"end_line":1121,"end_character":63},"in_reply_to":"3f79a3b5_d112c6b3","updated":"2018-10-09 07:52:15.000000000","message":"Remove this parameter.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_requested_volume_type(self, bdm, volume_type_id_or_name,"},{"line_number":1366,"context_line":"                                     volume_types):"},{"line_number":1367,"context_line":"        \"\"\"If we are specifying a volume type, we need to get the"},{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_71cc1234","line":1365,"range":{"start_line":1365,"start_character":37,"end_line":1365,"end_character":41},"updated":"2018-10-08 23:38:56.000000000","message":"This isn\u0027t used, so this could be a staticmethod.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1362,"context_line":""},{"line_number":1363,"context_line":"            bdm.update_or_create()"},{"line_number":1364,"context_line":""},{"line_number":1365,"context_line":"    def _check_requested_volume_type(self, bdm, volume_type_id_or_name,"},{"line_number":1366,"context_line":"                                     volume_types):"},{"line_number":1367,"context_line":"        \"\"\"If we are specifying a volume type, we need to get the"},{"line_number":1368,"context_line":"        volume type details from Cinder and make sure the ``volume_type``"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_59c6dc6b","line":1365,"range":{"start_line":1365,"start_character":37,"end_line":1365,"end_character":41},"in_reply_to":"3f79a3b5_71cc1234","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        # NOTE(brinzhang): Verify that the specified volume type exists."},{"line_number":1373,"context_line":"        # And save the volume type name internally for consistency in the"},{"line_number":1374,"context_line":"        # BlockDeviceMapping object, because volume type name cannot be"},{"line_number":1375,"context_line":"        # None in Cinder."},{"line_number":1376,"context_line":"        try:"},{"line_number":1377,"context_line":"            for vol_type in volume_types:"},{"line_number":1378,"context_line":"                if (volume_type_id_or_name \u003d\u003d vol_type[\u0027id\u0027] or"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_51d1d6c9","line":1375,"range":{"start_line":1374,"start_character":35,"end_line":1375,"end_character":25},"updated":"2018-10-08 23:38:56.000000000","message":"nit: I\u0027d probably remove this part of the comment since it\u0027s just confusing. It could be misinterpreted as BlockDeviceMapping.volume_type being required.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        # NOTE(brinzhang): Verify that the specified volume type exists."},{"line_number":1373,"context_line":"        # And save the volume type name internally for consistency in the"},{"line_number":1374,"context_line":"        # BlockDeviceMapping object, because volume type name cannot be"},{"line_number":1375,"context_line":"        # None in Cinder."},{"line_number":1376,"context_line":"        try:"},{"line_number":1377,"context_line":"            for vol_type in volume_types:"},{"line_number":1378,"context_line":"                if (volume_type_id_or_name \u003d\u003d vol_type[\u0027id\u0027] or"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_79add882","line":1375,"range":{"start_line":1374,"start_character":35,"end_line":1375,"end_character":25},"in_reply_to":"3f79a3b5_51d1d6c9","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"                            volume_type_id_or_name \u003d\u003d vol_type[\u0027name\u0027]):"},{"line_number":1380,"context_line":"                    bdm.volume_type \u003d vol_type[\u0027name\u0027]"},{"line_number":1381,"context_line":"                    break"},{"line_number":1382,"context_line":"        except Exception:"},{"line_number":1383,"context_line":"            raise exception.VolumeTypeNotFound("},{"line_number":1384,"context_line":"                id_or_name\u003dvolume_type_id_or_name)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def _check_supports_volume_type(self, context):"},{"line_number":1387,"context_line":"        # NOTE(brinzhang): Checking the minimum nova-compute service"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_71e5f2a3","line":1384,"range":{"start_line":1382,"start_character":8,"end_line":1384,"end_character":50},"updated":"2018-10-08 23:38:56.000000000","message":"This isn\u0027t used...I think what you want is something like this:\n\nfor vol_type in volume_types:\n    if (...):\n       bdm.volume_type \u003d vol_type[\u0027name\u0027]\n       break\nelse:\n    raise exception.VolumeTypeNotFound(...)","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1379,"context_line":"                            volume_type_id_or_name \u003d\u003d vol_type[\u0027name\u0027]):"},{"line_number":1380,"context_line":"                    bdm.volume_type \u003d vol_type[\u0027name\u0027]"},{"line_number":1381,"context_line":"                    break"},{"line_number":1382,"context_line":"        except Exception:"},{"line_number":1383,"context_line":"            raise exception.VolumeTypeNotFound("},{"line_number":1384,"context_line":"                id_or_name\u003dvolume_type_id_or_name)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def _check_supports_volume_type(self, context):"},{"line_number":1387,"context_line":"        # NOTE(brinzhang): Checking the minimum nova-compute service"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_19bce4b3","line":1384,"range":{"start_line":1382,"start_character":8,"end_line":1384,"end_character":50},"in_reply_to":"3f79a3b5_71e5f2a3","updated":"2018-10-09 07:52:15.000000000","message":"Yes, it\u0027s better.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            raise exception.VolumeTypeNotFound("},{"line_number":1384,"context_line":"                id_or_name\u003dvolume_type_id_or_name)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def _check_supports_volume_type(self, context):"},{"line_number":1387,"context_line":"        # NOTE(brinzhang): Checking the minimum nova-compute service"},{"line_number":1388,"context_line":"        # version across the deployment. Just make sure the volume"},{"line_number":1389,"context_line":"        # type can be supported when the bdm.volume_type is requested."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_d1772607","line":1386,"updated":"2018-10-08 23:38:56.000000000","message":"nit: looks like this can be a staticmethod since self isn\u0027t used.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"            raise exception.VolumeTypeNotFound("},{"line_number":1384,"context_line":"                id_or_name\u003dvolume_type_id_or_name)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def _check_supports_volume_type(self, context):"},{"line_number":1387,"context_line":"        # NOTE(brinzhang): Checking the minimum nova-compute service"},{"line_number":1388,"context_line":"        # version across the deployment. Just make sure the volume"},{"line_number":1389,"context_line":"        # type can be supported when the bdm.volume_type is requested."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_59c7bc3e","line":1386,"in_reply_to":"3f79a3b5_d1772607","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"        # If the minimum nova-compute service version is new enough to"},{"line_number":1396,"context_line":"        # support volume_type, return True, to make sure this method is"},{"line_number":1397,"context_line":"        # called only once in the for loop."},{"line_number":1398,"context_line":"        return True"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1401,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_514c76c8","line":1398,"updated":"2018-10-08 23:38:56.000000000","message":"We probably don\u0027t really need this, we could just set volume_type_is_supported\u003dTrue if this method doesn\u0027t raise an exception.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1395,"context_line":"        # If the minimum nova-compute service version is new enough to"},{"line_number":1396,"context_line":"        # support volume_type, return True, to make sure this method is"},{"line_number":1397,"context_line":"        # called only once in the for loop."},{"line_number":1398,"context_line":"        return True"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1401,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_3923e008","line":1398,"in_reply_to":"3f79a3b5_514c76c8","updated":"2018-10-09 07:52:15.000000000","message":"Yes, this don\u0027t need, remove this.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        # called only once in the for loop."},{"line_number":1398,"context_line":"        return True"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1401,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"},{"line_number":1402,"context_line":"                      supports_volume_type\u003dFalse):"},{"line_number":1403,"context_line":"        # Make sure that the boot indexes make sense."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_91b68e89","line":1400,"updated":"2018-10-08 23:38:56.000000000","message":"You have two test cases for the error cases above, but we should have a positive test with two BDMs with volume_types specified so that we can assert that the _check_* methods above are only called once - to make sure your local optimization variables are being used.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1397,"context_line":"        # called only once in the for loop."},{"line_number":1398,"context_line":"        return True"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"    def _validate_bdm(self, context, instance, instance_type,"},{"line_number":1401,"context_line":"                      block_device_mappings, supports_multiattach\u003dFalse,"},{"line_number":1402,"context_line":"                      supports_volume_type\u003dFalse):"},{"line_number":1403,"context_line":"        # Make sure that the boot indexes make sense."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_79415863","line":1400,"in_reply_to":"3f79a3b5_91b68e89","updated":"2018-10-09 07:52:15.000000000","message":"Yes, this need to add a test case.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1419,"context_line":"            raise exception.InvalidBDMBootSequence()"},{"line_number":1420,"context_line":""},{"line_number":1421,"context_line":"        volume_types \u003d None"},{"line_number":1422,"context_line":"        volume_type_is_supported \u003d False"},{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_11183ecb","line":1422,"updated":"2018-10-08 23:38:56.000000000","message":"This is just to avoid calling _check_supports_volume_type more than once in the for loop correct?","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1419,"context_line":"            raise exception.InvalidBDMBootSequence()"},{"line_number":1420,"context_line":""},{"line_number":1421,"context_line":"        volume_types \u003d None"},{"line_number":1422,"context_line":"        volume_type_is_supported \u003d False"},{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_59633c46","line":1422,"in_reply_to":"3f79a3b5_11183ecb","updated":"2018-10-09 07:52:15.000000000","message":"Yeah.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1421,"context_line":"        volume_types \u003d None"},{"line_number":1422,"context_line":"        volume_type_is_supported \u003d False"},{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"},{"line_number":1426,"context_line":"                if supports_volume_type and not volume_type_is_supported:"},{"line_number":1427,"context_line":"                    volume_type_is_supported \u003d \\"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_f11ca2c3","line":1424,"range":{"start_line":1424,"start_character":29,"end_line":1424,"end_character":34},"updated":"2018-10-08 23:38:56.000000000","message":"nit: we probably don\u0027t need the get() usage here because this should default to None in the object, so just use bdm.volume_type.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1421,"context_line":"        volume_types \u003d None"},{"line_number":1422,"context_line":"        volume_type_is_supported \u003d False"},{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"},{"line_number":1426,"context_line":"                if supports_volume_type and not volume_type_is_supported:"},{"line_number":1427,"context_line":"                    volume_type_is_supported \u003d \\"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_59fa5c44","line":1424,"range":{"start_line":1424,"start_character":29,"end_line":1424,"end_character":34},"in_reply_to":"3f79a3b5_f11ca2c3","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"},{"line_number":1426,"context_line":"                if supports_volume_type and not volume_type_is_supported:"},{"line_number":1427,"context_line":"                    volume_type_is_supported \u003d \\"},{"line_number":1428,"context_line":"                        self._check_supports_volume_type(context)"},{"line_number":1429,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_910c4e94","line":1426,"range":{"start_line":1426,"start_character":19,"end_line":1426,"end_character":39},"updated":"2018-10-08 23:38:56.000000000","message":"I don\u0027t think we need this check. If bdm.volume_type is not None, that means the user requested it in the POST body, and if they used the correct microversion then it passed the request schema validation, otherwise it would have failed with a 400 response already, so I think we can remove this parameter and how it\u0027s passed down through various methods.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":1423,"context_line":"        for bdm in block_device_mappings:"},{"line_number":1424,"context_line":"            volume_type \u003d bdm.get(\u0027volume_type\u0027)"},{"line_number":1425,"context_line":"            if volume_type:"},{"line_number":1426,"context_line":"                if supports_volume_type and not volume_type_is_supported:"},{"line_number":1427,"context_line":"                    volume_type_is_supported \u003d \\"},{"line_number":1428,"context_line":"                        self._check_supports_volume_type(context)"},{"line_number":1429,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_d993ccfe","line":1426,"range":{"start_line":1426,"start_character":19,"end_line":1426,"end_character":39},"in_reply_to":"3f79a3b5_910c4e94","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0678ddbe4fe8f10a78ed2b7fb0528129694b5e97","unresolved":false,"context_lines":[{"line_number":1372,"context_line":"        # And save the volume type name internally for consistency in the"},{"line_number":1373,"context_line":"        # BlockDeviceMapping object."},{"line_number":1374,"context_line":"        for vol_type in volume_types:"},{"line_number":1375,"context_line":"            if (volume_type_id_or_name \u003d\u003d vol_type[\u0027id\u0027] or"},{"line_number":1376,"context_line":"                        volume_type_id_or_name \u003d\u003d vol_type[\u0027name\u0027]):"},{"line_number":1377,"context_line":"                bdm.volume_type \u003d vol_type[\u0027name\u0027]"},{"line_number":1378,"context_line":"                break"},{"line_number":1379,"context_line":"        else:"},{"line_number":1380,"context_line":"            raise exception.VolumeTypeNotFound("}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_eefd8e09","line":1377,"range":{"start_line":1375,"start_character":12,"end_line":1377,"end_character":50},"updated":"2018-10-09 16:35:12.000000000","message":"I\u0027d like to see a positive test for this method where we have two BDMs and the volume type for one is using the ID and the other is using the name, and we assert that the resulting bdm.volume_type for both is the volume type name, not the ID.","commit_id":"f19245217655858c7cb3af8a4d62780a9497a4c6"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"4ee4bc14e95f64bd5de9f5f0f9d238facc08187f","unresolved":false,"context_lines":[{"line_number":1372,"context_line":"        # And save the volume type name internally for consistency in the"},{"line_number":1373,"context_line":"        # BlockDeviceMapping object."},{"line_number":1374,"context_line":"        for vol_type in volume_types:"},{"line_number":1375,"context_line":"            if (volume_type_id_or_name \u003d\u003d vol_type[\u0027id\u0027] or"},{"line_number":1376,"context_line":"                        volume_type_id_or_name \u003d\u003d vol_type[\u0027name\u0027]):"},{"line_number":1377,"context_line":"                bdm.volume_type \u003d vol_type[\u0027name\u0027]"},{"line_number":1378,"context_line":"                break"},{"line_number":1379,"context_line":"        else:"},{"line_number":1380,"context_line":"            raise exception.VolumeTypeNotFound("}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_1437b000","line":1377,"range":{"start_line":1375,"start_character":12,"end_line":1377,"end_character":50},"in_reply_to":"3f79a3b5_eefd8e09","updated":"2018-10-10 03:12:08.000000000","message":"Yeah, your point is so good for this, I will add a new test case for this, if the user is using the volume type ID, assign volume_type to volume type name.","commit_id":"f19245217655858c7cb3af8a4d62780a9497a4c6"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea8f2b63a9e38948b98fefd8999cd6eaa206e832","unresolved":false,"context_lines":[{"line_number":1416,"context_line":"            volume_type \u003d bdm.volume_type"},{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_8bab6fd5","line":1419,"range":{"start_line":1419,"start_character":20,"end_line":1419,"end_character":61},"updated":"2018-10-11 17:12:54.000000000","message":"This implies it\u0027s checking for a specific type to me. Can you change this to something like \"_check_volume_type_compatibility()\" or \"_check_compute_supports_volume_type()\"?","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b65c8e23a4b6d9dc30b145fc913605ce3054e112","unresolved":false,"context_lines":[{"line_number":1416,"context_line":"            volume_type \u003d bdm.volume_type"},{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_33288dec","line":1419,"range":{"start_line":1419,"start_character":20,"end_line":1419,"end_character":61},"in_reply_to":"3f79a3b5_5da3dab1","updated":"2018-10-11 21:06:03.000000000","message":"Done","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":1416,"context_line":"            volume_type \u003d bdm.volume_type"},{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_5da3dab1","line":1419,"range":{"start_line":1419,"start_character":20,"end_line":1419,"end_character":61},"in_reply_to":"3f79a3b5_8bab6fd5","updated":"2018-10-11 18:29:00.000000000","message":"\"_check_compute_supports_volume_type\" sounds better to me of those two alternatives","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea8f2b63a9e38948b98fefd8999cd6eaa206e832","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_0b819f4d","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"updated":"2018-10-11 17:12:54.000000000","message":"This logic is a little confusing I think, since it seems like this would be run multiple times if we don\u0027t support this yet, and requires you to know that the check function will never return false, but explode if it\u0027s not supported.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b65c8e23a4b6d9dc30b145fc913605ce3054e112","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_f32d35fc","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"in_reply_to":"3f79a3b5_009e59b6","updated":"2018-10-11 21:06:03.000000000","message":"Done","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_fd7c2623","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"in_reply_to":"3f79a3b5_0b819f4d","updated":"2018-10-11 18:29:00.000000000","message":"In an earlier patch he was returning True if _check_supports_volume_type didn\u0027t raise and would set that as the value of the volume_type_is_supported variable. Is that what you\u0027re looking for? We won\u0027t hit _check_supports_volume_type multiple times because either we hit it once and the computes are new enough to support volume type or they aren\u0027t and we raise. If they are, we set the local variable so we don\u0027t check again.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"65493aae9de2251e612cae239740a6c780a4f0ad","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_9d7652d5","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"in_reply_to":"3f79a3b5_7d41f6a6","updated":"2018-10-11 18:42:36.000000000","message":"Would simply adding a comment on the call here that it will raise if not supported help?","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c2dd1b6bace3acee8a8d30703d58ec15ceace988","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_009e59b6","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"in_reply_to":"3f79a3b5_9d7652d5","updated":"2018-10-11 18:51:06.000000000","message":"Sure, it\u0027d help.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e3eb7f1700c27c7220bb92c22de6e8098ccd82a4","unresolved":false,"context_lines":[{"line_number":1417,"context_line":"            if volume_type:"},{"line_number":1418,"context_line":"                if not volume_type_is_supported:"},{"line_number":1419,"context_line":"                    self._check_supports_volume_type(context)"},{"line_number":1420,"context_line":"                    # This is just to avoid calling _check_supports_volume_type"},{"line_number":1421,"context_line":"                    # more than once in this for loop."},{"line_number":1422,"context_line":"                    volume_type_is_supported \u003d True"},{"line_number":1423,"context_line":""},{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_7d41f6a6","line":1422,"range":{"start_line":1420,"start_character":20,"end_line":1422,"end_character":51},"in_reply_to":"3f79a3b5_fd7c2623","updated":"2018-10-11 18:34:10.000000000","message":"\u003e In an earlier patch he was returning True if _check_supports_volume_type\n \u003e didn\u0027t raise and would set that as the value of the\n \u003e volume_type_is_supported variable. Is that what you\u0027re looking for?\n \u003e We won\u0027t hit _check_supports_volume_type multiple times because\n \u003e either we hit it once and the computes are new enough to support\n \u003e volume type or they aren\u0027t and we raise. If they are, we set the\n \u003e local variable so we don\u0027t check again.\n\nYeah, I know. The \"we\u0027ll only hit this once regardless of true or false but for different reasons\" is part of what I don\u0027t like. It\u0027s not a huge deal, I just think it\u0027s not easy to read this and understand why it\u0027s right...you have to go look and see that the above method explodes.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea8f2b63a9e38948b98fefd8999cd6eaa206e832","unresolved":false,"context_lines":[{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"},{"line_number":1426,"context_line":"                    # initialize our cache of volume types."},{"line_number":1427,"context_line":"                    volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1428,"context_line":"                        context)"},{"line_number":1429,"context_line":"                # NOTE(brinzhang): Ensure the validity of volume_type."},{"line_number":1430,"context_line":"                self._check_requested_volume_type(bdm, volume_type,"},{"line_number":1431,"context_line":"                                                  volume_types)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_4b5c7719","line":1428,"range":{"start_line":1427,"start_character":20,"end_line":1428,"end_character":32},"updated":"2018-10-11 17:12:54.000000000","message":"I dunno how others feel, but I think we should cache this. The logic can, I think, be:\n\n 0. Available types is empty\n 1. Check to see if the type is in available types\n 2. If not in types, refresh available types\n 3. Return true if type in available types\n\nThe only downside here is that if you delete a type, we won\u0027t catch it until we hit compute. However, I would think that (rare scenario) would be much better than always hitting the cinder api to list all volume types every single time we get one of these requests.\n\nThoughts?","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"65493aae9de2251e612cae239740a6c780a4f0ad","unresolved":false,"context_lines":[{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"},{"line_number":1426,"context_line":"                    # initialize our cache of volume types."},{"line_number":1427,"context_line":"                    volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1428,"context_line":"                        context)"},{"line_number":1429,"context_line":"                # NOTE(brinzhang): Ensure the validity of volume_type."},{"line_number":1430,"context_line":"                self._check_requested_volume_type(bdm, volume_type,"},{"line_number":1431,"context_line":"                                                  volume_types)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_7d27b6c9","line":1428,"range":{"start_line":1427,"start_character":20,"end_line":1428,"end_character":32},"in_reply_to":"3f79a3b5_3dc4de0d","updated":"2018-10-11 18:42:36.000000000","message":"I\u0027m fine with a cache using oslo.cache so it\u0027s shared like you said, and it would probably be good to put a timer on it (maybe expire every day or something?). However, I would also like that to be a follow up if that\u0027s OK since I don\u0027t think it should hold up the series (which I\u0027ve kind of been grinding through to get ready and merged before it falls out of the runway in another day or so).","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"},{"line_number":1426,"context_line":"                    # initialize our cache of volume types."},{"line_number":1427,"context_line":"                    volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1428,"context_line":"                        context)"},{"line_number":1429,"context_line":"                # NOTE(brinzhang): Ensure the validity of volume_type."},{"line_number":1430,"context_line":"                self._check_requested_volume_type(bdm, volume_type,"},{"line_number":1431,"context_line":"                                                  volume_types)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_9dbbb2bc","line":1428,"range":{"start_line":1427,"start_character":20,"end_line":1428,"end_character":32},"in_reply_to":"3f79a3b5_4b5c7719","updated":"2018-10-11 18:29:00.000000000","message":"I asked for this because originally he was checking of the volume type was a uuid and if so, he would try to \"show\" that volume type from the cinder API, and if it was a name, he\u0027d have to list and search the results anyway. So it\u0027s simpler to just list the volume types and check for id or name in that list.\n\nBy cache, do you mean cache in memory in the nova-api service rather than just once per request where volume_type is requested? I think that\u0027s excessive, especially given we hit the cinder API to get a volume\u0027s details every time anyone boots with a pre-existing volume. In other words, I just don\u0027t think caching in memory is necessary for this if we do it just once per request.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc7d81b9cf056c9595135d8b51ab8a7eb615de8d","unresolved":false,"context_lines":[{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"},{"line_number":1426,"context_line":"                    # initialize our cache of volume types."},{"line_number":1427,"context_line":"                    volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1428,"context_line":"                        context)"},{"line_number":1429,"context_line":"                # NOTE(brinzhang): Ensure the validity of volume_type."},{"line_number":1430,"context_line":"                self._check_requested_volume_type(bdm, volume_type,"},{"line_number":1431,"context_line":"                                                  volume_types)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_c01e210c","line":1428,"range":{"start_line":1427,"start_character":20,"end_line":1428,"end_character":32},"in_reply_to":"3f79a3b5_7d27b6c9","updated":"2018-10-11 19:03:59.000000000","message":"In proofing this out, I see we\u0027re passing context to the call, which means we could potentially have a limited list of volume types based on who we are right now. Which means no easy global cache. So, nevermind.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e3eb7f1700c27c7220bb92c22de6e8098ccd82a4","unresolved":false,"context_lines":[{"line_number":1424,"context_line":"                if not volume_types:"},{"line_number":1425,"context_line":"                    # In order to reduce the number of hit cinder APIs,"},{"line_number":1426,"context_line":"                    # initialize our cache of volume types."},{"line_number":1427,"context_line":"                    volume_types \u003d self.volume_api.get_all_volume_types("},{"line_number":1428,"context_line":"                        context)"},{"line_number":1429,"context_line":"                # NOTE(brinzhang): Ensure the validity of volume_type."},{"line_number":1430,"context_line":"                self._check_requested_volume_type(bdm, volume_type,"},{"line_number":1431,"context_line":"                                                  volume_types)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_3dc4de0d","line":1428,"range":{"start_line":1427,"start_character":20,"end_line":1428,"end_character":32},"in_reply_to":"3f79a3b5_9dbbb2bc","updated":"2018-10-11 18:34:10.000000000","message":"Yes, I mean once per api worker, probably with oslo.cache so we notice deletions eventually and presumably if you use a memcache kind of thing, then it\u0027s share-able between workers.\n\nI\u0027m not sure why caching it is excessive, and the reason I think it\u0027s different is because this is synchronous with the api call to nova, which means we have client-\u003enova-\u003ecinder as a blocking call. In the case of a pre-existing volume we can\u0027t help it, but here, we totally can. Just seems massively more efficient to me, and it\u0027s a good opportunity to do that.\n\nSame reason we avoid hitting neutron every time we need to look at network info that shouldn\u0027t really change all the time right?","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"}],"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4fc5d10fa30d0af89c210d9ca2350a041a3de577","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    code \u003d 409"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"class VolumeTypeNotSupportedOldMicroversion(Invalid):"},{"line_number":295,"context_line":"    msg_fmt \u003d _(\u0027Volume type is only supported starting with \u0027"},{"line_number":296,"context_line":"                \u0027compute API version 2.67.\u0027)"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_1a254fbf","line":294,"range":{"start_line":294,"start_character":6,"end_line":294,"end_character":43},"updated":"2018-10-01 16:31:06.000000000","message":"Don\u0027t need this.","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"3d663d00d57772101208fa529b30b9fedbdc723a","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    code \u003d 409"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"class VolumeTypeNotSupportedOldMicroversion(Invalid):"},{"line_number":295,"context_line":"    msg_fmt \u003d _(\u0027Volume type is only supported starting with \u0027"},{"line_number":296,"context_line":"                \u0027compute API version 2.67.\u0027)"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_7ab0f8dc","line":294,"range":{"start_line":294,"start_character":6,"end_line":294,"end_character":43},"in_reply_to":"3f79a3b5_1a254fbf","updated":"2018-10-02 03:50:17.000000000","message":"Done","commit_id":"b8b726f66a0dabfd88ae0c7d0654c6ce3adc1e3b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":639,"context_line":"    msg_fmt \u003d _(\"Volume %(volume_id)s could not be found.\")"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"class VolumeTypeNotFound(NotFound):"},{"line_number":643,"context_line":"    msg_fmt \u003d _(\"Volume type %(id_or_name)s could not be found.\")"},{"line_number":644,"context_line":""},{"line_number":645,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_31ff7a36","line":642,"range":{"start_line":642,"start_character":25,"end_line":642,"end_character":33},"updated":"2018-10-08 23:38:56.000000000","message":"Just remember that we\u0027ll need to return a 400 in this case to the user rather than a 404. That is something for the next change in the series though.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":639,"context_line":"    msg_fmt \u003d _(\"Volume %(volume_id)s could not be found.\")"},{"line_number":640,"context_line":""},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"class VolumeTypeNotFound(NotFound):"},{"line_number":643,"context_line":"    msg_fmt \u003d _(\"Volume type %(id_or_name)s could not be found.\")"},{"line_number":644,"context_line":""},{"line_number":645,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_5956dc2d","line":642,"range":{"start_line":642,"start_character":25,"end_line":642,"end_character":33},"in_reply_to":"3f79a3b5_31ff7a36","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    # This exception indicates the deployment is not yet new enough to support"},{"line_number":275,"context_line":"    # multiattach volumes, so a 409 HTTPConflict response is generally used"},{"line_number":276,"context_line":"    # for handling this in the API."},{"line_number":277,"context_line":"    msg_fmt \u003d _(\"Multiattach volume support is not yet available.\")"},{"line_number":278,"context_line":"    code \u003d 409"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_fddf4608","line":277,"range":{"start_line":277,"start_character":4,"end_line":277,"end_character":67},"updated":"2018-10-11 18:29:00.000000000","message":"He copied from this.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea8f2b63a9e38948b98fefd8999cd6eaa206e832","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    # This exception indicates the deployment is not yet new enough to support"},{"line_number":288,"context_line":"    # volume type, so a 409 HTTPConflict response is generally used"},{"line_number":289,"context_line":"    # for handling this in the API."},{"line_number":290,"context_line":"    msg_fmt \u003d _(\"Volume type support is not yet available.\")"},{"line_number":291,"context_line":"    code \u003d 409"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_8b94cf8d","line":290,"updated":"2018-10-11 17:12:54.000000000","message":"This might be confusing to the user. Maybe say \"Volume type per request is not yet available\" or something? The \"per request\" bit is the important thing to me. But, this is just a nit.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    # This exception indicates the deployment is not yet new enough to support"},{"line_number":288,"context_line":"    # volume type, so a 409 HTTPConflict response is generally used"},{"line_number":289,"context_line":"    # for handling this in the API."},{"line_number":290,"context_line":"    msg_fmt \u003d _(\"Volume type support is not yet available.\")"},{"line_number":291,"context_line":"    code \u003d 409"},{"line_number":292,"context_line":""},{"line_number":293,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_ddca8a44","line":290,"in_reply_to":"3f79a3b5_8b94cf8d","updated":"2018-10-11 18:29:00.000000000","message":"This is following a pattern from a couple of others (multiattach and I think trusted certs is similar). The \"per request\" is actually more confusing to me.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d2c70ae3c6a1d5ddb4093fa501a18a191488627b","unresolved":false,"context_lines":[{"line_number":2346,"context_line":""},{"line_number":2347,"context_line":""},{"line_number":2348,"context_line":"class CertificateValidationNotYetAvailable(NovaException):"},{"line_number":2349,"context_line":"    msg_fmt \u003d _(\"Image signature certificate validation support is \""},{"line_number":2350,"context_line":"                \"not yet available.\")"},{"line_number":2351,"context_line":"    code \u003d 409"},{"line_number":2352,"context_line":""},{"line_number":2353,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_7dd33603","line":2350,"range":{"start_line":2349,"start_character":3,"end_line":2350,"end_character":37},"updated":"2018-10-11 18:29:00.000000000","message":"same","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"}],"nova/tests/unit/api/openstack/compute/test_serversV21.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":4635,"context_line":"            test.MatchType(objects.Instance),"},{"line_number":4636,"context_line":"            test.MatchType(objects.Flavor),"},{"line_number":4637,"context_line":"            test.MatchType(objects.BlockDeviceMappingList),"},{"line_number":4638,"context_line":"            False, False)"},{"line_number":4639,"context_line":""},{"line_number":4640,"context_line":"    def test_create_instance_with_volumes_enabled(self):"},{"line_number":4641,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_812f4bf4","line":4638,"range":{"start_line":4638,"start_character":19,"end_line":4638,"end_character":24},"updated":"2018-10-05 16:06:31.000000000","message":"Use named kwargs here, e.g. supports_volume_type\u003dFalse. Otherwise it\u0027s unclear from the test what this parameter is.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":4635,"context_line":"            test.MatchType(objects.Instance),"},{"line_number":4636,"context_line":"            test.MatchType(objects.Flavor),"},{"line_number":4637,"context_line":"            test.MatchType(objects.BlockDeviceMappingList),"},{"line_number":4638,"context_line":"            False, False)"},{"line_number":4639,"context_line":""},{"line_number":4640,"context_line":"    def test_create_instance_with_volumes_enabled(self):"},{"line_number":4641,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_ed337ff6","line":4638,"range":{"start_line":4638,"start_character":19,"end_line":4638,"end_character":24},"in_reply_to":"3f79a3b5_812f4bf4","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"fa17769da0f4df21fc8895ec982a4f4a182f4f27","unresolved":false,"context_lines":[{"line_number":4635,"context_line":"            test.MatchType(objects.Instance),"},{"line_number":4636,"context_line":"            test.MatchType(objects.Flavor),"},{"line_number":4637,"context_line":"            test.MatchType(objects.BlockDeviceMappingList),"},{"line_number":4638,"context_line":"            False, False)"},{"line_number":4639,"context_line":""},{"line_number":4640,"context_line":"    def test_create_instance_with_volumes_enabled(self):"},{"line_number":4641,"context_line":"        params \u003d {\u0027block_device_mapping\u0027: self.bdm}"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_4485a809","line":4638,"range":{"start_line":4638,"start_character":19,"end_line":4638,"end_character":24},"in_reply_to":"3f79a3b5_812f4bf4","updated":"2018-10-08 09:12:08.000000000","message":"Emm...here, if change this to \"supports_volume_type\u003dFalse\", there are something wrong with excepted and actual return, so temporarily keep False instead of \"supports_volume_type\u003dFalse\".","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"                              shutdown_terminate, instance_group,"},{"line_number":4522,"context_line":"                              check_server_group_quota, filter_properties,"},{"line_number":4523,"context_line":"                              None, objects.TagList(), trusted_certs,"},{"line_number":4524,"context_line":"                              False, False)"},{"line_number":4525,"context_line":""},{"line_number":4526,"context_line":"        do_test()"},{"line_number":4527,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_4139333e","line":4524,"range":{"start_line":4524,"start_character":37,"end_line":4524,"end_character":42},"updated":"2018-10-05 16:06:31.000000000","message":"Use named kwargs here and below.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"                              shutdown_terminate, instance_group,"},{"line_number":4522,"context_line":"                              check_server_group_quota, filter_properties,"},{"line_number":4523,"context_line":"                              None, objects.TagList(), trusted_certs,"},{"line_number":4524,"context_line":"                              False, False)"},{"line_number":4525,"context_line":""},{"line_number":4526,"context_line":"        do_test()"},{"line_number":4527,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_4d7153ab","line":4524,"range":{"start_line":4524,"start_character":37,"end_line":4524,"end_character":42},"in_reply_to":"3f79a3b5_4139333e","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c2d51aa344b8dc3cf518c725b7fa8612dd4210c8","unresolved":false,"context_lines":[{"line_number":6217,"context_line":"        upgraded yet."},{"line_number":6218,"context_line":"        \"\"\""},{"line_number":6219,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":6220,"context_line":"        delattr(instance, \u0027id\u0027)"},{"line_number":6221,"context_line":"        volume_type \u003d \u0027fake-lvm-1\u0027"},{"line_number":6222,"context_line":"        self.assertRaises(exception.VolumeTypeSupportNotYetAvailable,"},{"line_number":6223,"context_line":"                          self.compute_api._check_supports_volume_type,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_c10ea38e","line":6220,"updated":"2018-10-05 16:06:31.000000000","message":"You don\u0027t need this.","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"7b106ee1032ce429d327ca59c1a5ae0ca98ae3f3","unresolved":false,"context_lines":[{"line_number":6217,"context_line":"        upgraded yet."},{"line_number":6218,"context_line":"        \"\"\""},{"line_number":6219,"context_line":"        instance \u003d self._create_instance_obj()"},{"line_number":6220,"context_line":"        delattr(instance, \u0027id\u0027)"},{"line_number":6221,"context_line":"        volume_type \u003d \u0027fake-lvm-1\u0027"},{"line_number":6222,"context_line":"        self.assertRaises(exception.VolumeTypeSupportNotYetAvailable,"},{"line_number":6223,"context_line":"                          self.compute_api._check_supports_volume_type,"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_2da5f703","line":6220,"in_reply_to":"3f79a3b5_c10ea38e","updated":"2018-10-08 06:13:38.000000000","message":"Done","commit_id":"569e51cce422d9eafb18a2997e3a02368c02bb0c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":6229,"context_line":"    @mock.patch(\u0027nova.objects.service.get_minimum_version_all_cells\u0027,"},{"line_number":6230,"context_line":"                return_value\u003dcompute_api.MIN_COMPUTE_VOLUME_TYPE)"},{"line_number":6231,"context_line":"    @mock.patch.object(cinder.API, \u0027get_all_volume_types\u0027)"},{"line_number":6232,"context_line":"    def test_validate_bdm_check_volume_type_raise_not_found("},{"line_number":6233,"context_line":"            self, mock_get_all_volume_types, get_min_version):"},{"line_number":6234,"context_line":"        \"\"\"Tests that _validate_bdm will fail if the requested volume type"},{"line_number":6235,"context_line":"        name or id does not match the volume types in Cinder."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_517f96bb","line":6232,"range":{"start_line":6232,"start_character":8,"end_line":6232,"end_character":59},"updated":"2018-10-08 23:38:56.000000000","message":"This test could just call _check_requested_volume_type directly correct?","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":6229,"context_line":"    @mock.patch(\u0027nova.objects.service.get_minimum_version_all_cells\u0027,"},{"line_number":6230,"context_line":"                return_value\u003dcompute_api.MIN_COMPUTE_VOLUME_TYPE)"},{"line_number":6231,"context_line":"    @mock.patch.object(cinder.API, \u0027get_all_volume_types\u0027)"},{"line_number":6232,"context_line":"    def test_validate_bdm_check_volume_type_raise_not_found("},{"line_number":6233,"context_line":"            self, mock_get_all_volume_types, get_min_version):"},{"line_number":6234,"context_line":"        \"\"\"Tests that _validate_bdm will fail if the requested volume type"},{"line_number":6235,"context_line":"        name or id does not match the volume types in Cinder."}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_f7fd6f07","line":6232,"range":{"start_line":6232,"start_character":8,"end_line":6232,"end_character":59},"in_reply_to":"3f79a3b5_517f96bb","updated":"2018-10-09 07:52:15.000000000","message":"Emm...I am sorry, this is just a draft.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0678ddbe4fe8f10a78ed2b7fb0528129694b5e97","unresolved":false,"context_lines":[{"line_number":4469,"context_line":"        volume_types \u003d [{\u0027id\u0027: \u0027fake_volume_type_id_1\u0027, \u0027name\u0027: \u0027fake_lvm_1\u0027},"},{"line_number":4470,"context_line":"                        {\u0027id\u0027: \u0027fake_volume_type_id_2\u0027, \u0027name\u0027: \u0027fake_lvm_2\u0027}]"},{"line_number":4471,"context_line":""},{"line_number":4472,"context_line":"        bdms \u003d [objects.BlockDeviceMapping("},{"line_number":4473,"context_line":"            **fake_block_device.AnonFakeDbBlockDeviceDict("},{"line_number":4474,"context_line":"                {"},{"line_number":4475,"context_line":"                    \u0027uuid\u0027: uuids.image_id,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_0ead6a31","line":4472,"range":{"start_line":4472,"start_character":8,"end_line":4472,"end_character":12},"updated":"2018-10-09 16:35:12.000000000","message":"We should have more than one, otherwise get_all_vol_types.assert_called_once_with and vol_type_supported.assert_called_once_with will always be True because we only looped once. If we loop over two BDMs with volumes types, then asserting that those methods are called only once makes sure that our optimization variables are being honored.","commit_id":"f19245217655858c7cb3af8a4d62780a9497a4c6"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"4ee4bc14e95f64bd5de9f5f0f9d238facc08187f","unresolved":false,"context_lines":[{"line_number":4469,"context_line":"        volume_types \u003d [{\u0027id\u0027: \u0027fake_volume_type_id_1\u0027, \u0027name\u0027: \u0027fake_lvm_1\u0027},"},{"line_number":4470,"context_line":"                        {\u0027id\u0027: \u0027fake_volume_type_id_2\u0027, \u0027name\u0027: \u0027fake_lvm_2\u0027}]"},{"line_number":4471,"context_line":""},{"line_number":4472,"context_line":"        bdms \u003d [objects.BlockDeviceMapping("},{"line_number":4473,"context_line":"            **fake_block_device.AnonFakeDbBlockDeviceDict("},{"line_number":4474,"context_line":"                {"},{"line_number":4475,"context_line":"                    \u0027uuid\u0027: uuids.image_id,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_5416885a","line":4472,"range":{"start_line":4472,"start_character":8,"end_line":4472,"end_character":12},"in_reply_to":"3f79a3b5_0ead6a31","updated":"2018-10-10 03:12:08.000000000","message":"Extend this dict and test with that.","commit_id":"f19245217655858c7cb3af8a4d62780a9497a4c6"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b531f975e7e1a1bf703bc65ebb9cd1f4ed9baf74","unresolved":false,"context_lines":[{"line_number":4508,"context_line":"            vol_type_supported.assert_called_once_with(self.context)"},{"line_number":4509,"context_line":"            get_all_vol_types.assert_called_once_with(self.context)"},{"line_number":4510,"context_line":""},{"line_number":4511,"context_line":"            vol_type_requested.assert_any_call(bdms[0], volume_type,"},{"line_number":4512,"context_line":"                                               volume_types)"},{"line_number":4513,"context_line":"            vol_type_requested.assert_any_call(bdms[1], volume_type,"},{"line_number":4514,"context_line":"                                               volume_types)"},{"line_number":4515,"context_line":""},{"line_number":4516,"context_line":"    @mock.patch.object(objects.service, \u0027get_minimum_version_all_cells\u0027,"},{"line_number":4517,"context_line":"                       return_value\u003dcompute_api.MIN_COMPUTE_VOLUME_TYPE)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_25912a47","line":4514,"range":{"start_line":4511,"start_character":12,"end_line":4514,"end_character":60},"updated":"2018-10-11 16:06:45.000000000","message":"This could just use:\n\nvol_type_requested.assert_has_calls([\n   mock.call(bdms[0], volume_type, volume_types),\n   mock.call(bdms[1], volume_type, volume_types)])\n\nBecause we\u0027ll iterate the list in order, the assertion should be predictable.","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b531f975e7e1a1bf703bc65ebb9cd1f4ed9baf74","unresolved":false,"context_lines":[{"line_number":4533,"context_line":"                    \u0027device_name\u0027: \u0027vda\u0027,"},{"line_number":4534,"context_line":"                    \u0027boot_index\u0027: 0,"},{"line_number":4535,"context_line":"                    \u0027volume_size\u0027: 3,"},{"line_number":4536,"context_line":"                    \u0027volume_type\u0027: \u0027fake_volume_type_id_1\u0027"},{"line_number":4537,"context_line":"                }))]"},{"line_number":4538,"context_line":""},{"line_number":4539,"context_line":"        self.compute_api._check_requested_volume_type(bdms[0],"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_c5915647","line":4536,"range":{"start_line":4536,"start_character":35,"end_line":4536,"end_character":58},"updated":"2018-10-11 16:06:45.000000000","message":"nit: use volume_types[0][\u0027id\u0027] here","commit_id":"3988ef2ef9d656393e5f3039aea1f05b8985ed55"}],"nova/virt/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"88a4377af7e778c8e29355387715dfecf3bd8996","unresolved":false,"context_lines":[{"line_number":265,"context_line":"    _valid_source \u003d \u0027volume\u0027"},{"line_number":266,"context_line":"    _valid_destination \u003d \u0027volume\u0027"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"    _proxy_as_attr_inherited \u003d set([\u0027volume_size\u0027, \u0027volume_id\u0027, \u0027volume_type\u0027])"},{"line_number":269,"context_line":"    _update_on_save \u003d {\u0027disk_bus\u0027: None,"},{"line_number":270,"context_line":"                       \u0027device_name\u0027: \u0027mount_device\u0027,"},{"line_number":271,"context_line":"                       \u0027device_type\u0027: None}"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_64089a2c","line":268,"updated":"2018-09-28 12:33:59.000000000","message":"Split these changes out into the compute-specific patch below  this in the series.","commit_id":"2c736b23fabd096770fa008d9ac8130c4566bca7"}],"nova/volume/cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":421,"context_line":"    return translate_cinder_exception(wrapper)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"def translate_volume_type_exception(method):"},{"line_number":425,"context_line":"    \"\"\"Transforms the exception for the volume type but keeps its"},{"line_number":426,"context_line":"       traceback intact."},{"line_number":427,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_11a27e40","line":424,"updated":"2018-10-08 23:38:56.000000000","message":"Should probably have a test to make sure this is used as well.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":421,"context_line":"    return translate_cinder_exception(wrapper)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"def translate_volume_type_exception(method):"},{"line_number":425,"context_line":"    \"\"\"Transforms the exception for the volume type but keeps its"},{"line_number":426,"context_line":"       traceback intact."},{"line_number":427,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_8d7e6ee1","line":424,"in_reply_to":"3f79a3b5_11a27e40","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a3ee64c21174514e23c9848c8bb698d91e04d5d4","unresolved":false,"context_lines":[{"line_number":421,"context_line":"    return translate_cinder_exception(wrapper)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":""},{"line_number":424,"context_line":"def translate_volume_type_exception(method):"},{"line_number":425,"context_line":"    \"\"\"Transforms the exception for the volume type but keeps its"},{"line_number":426,"context_line":"       traceback intact."},{"line_number":427,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_084b2cb1","line":424,"in_reply_to":"3f79a3b5_11a27e40","updated":"2018-10-09 09:01:32.000000000","message":"This method is not used, before this used to get_volume_type_by_id. I abandoned that method (get_volume_type_by_id), so there is no need to exist here.\n\nRemove it.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"997993556772ce6e56a275196481ff4f9b933a86","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        cinderclient(context).volume_snapshots.delete(snapshot_id)"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"    @translate_cinder_exception"},{"line_number":714,"context_line":"    def get_all_volume_types(self, context):"},{"line_number":715,"context_line":"        items \u003d cinderclient(context).volume_types.list()"},{"line_number":716,"context_line":"        rvals \u003d []"},{"line_number":717,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_f1a6e254","line":714,"updated":"2018-10-08 23:38:56.000000000","message":"Should have a test for this.","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b30b41a978e618bead89c89a2b417361744a1428","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        cinderclient(context).volume_snapshots.delete(snapshot_id)"},{"line_number":712,"context_line":""},{"line_number":713,"context_line":"    @translate_cinder_exception"},{"line_number":714,"context_line":"    def get_all_volume_types(self, context):"},{"line_number":715,"context_line":"        items \u003d cinderclient(context).volume_types.list()"},{"line_number":716,"context_line":"        rvals \u003d []"},{"line_number":717,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_ad81eac4","line":714,"in_reply_to":"3f79a3b5_f1a6e254","updated":"2018-10-09 07:52:15.000000000","message":"Done","commit_id":"c5d073867dac26907686d7393d9e58d9b70156c8"}]}