)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c316964ae22ceeabc497232305595fe947bfede4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Allow PUT /servers/{server_id}/os-volume_attachments/{volume_id}``"},{"line_number":10,"context_line":"to support specifying ``delete_on_termination`` field in the request"},{"line_number":11,"context_line":"body. This allows updating the attached volume\u0027s flag that controls"},{"line_number":12,"context_line":"whether or not it is automatically deleted when the instance is deleted."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Blueprint: destroy-instance-with-datavolume"},{"line_number":15,"context_line":"Change-Id: I6ccac4e17f56b40e67c79d40f32558ef414685ea"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df33271e_12a3dfb3","line":12,"updated":"2020-03-31 01:43:47.000000000","message":"I think we also need to add the description: When we request \u0027volumeId\u0027 and \u0027delete_on_termination\u0027 in the requst body to swap volume, since the new microversion it will be support updating the swapping volume\u0027s delete flag.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Allow PUT /servers/{server_id}/os-volume_attachments/{volume_id}``"},{"line_number":10,"context_line":"to support specifying ``delete_on_termination`` field in the request"},{"line_number":11,"context_line":"body. This allows updating the attached volume\u0027s flag that controls"},{"line_number":12,"context_line":"whether or not it is automatically deleted when the instance is deleted."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Blueprint: destroy-instance-with-datavolume"},{"line_number":15,"context_line":"Change-Id: I6ccac4e17f56b40e67c79d40f32558ef414685ea"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"df33271e_81baf66a","line":12,"in_reply_to":"df33271e_12a3dfb3","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"}],"api-ref/source/os-volume-attachments.inc":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-dot-volume-req.json"},{"line_number":221,"context_line":"   :language: javascript"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"Response"}],"source_content_type":"text/x-c++src","patch_set":21,"id":"df33271e_f7c46df7","line":220,"range":{"start_line":220,"start_character":59,"end_line":220,"end_character":85},"updated":"2020-03-30 23:28:35.000000000","message":"\u0027update-dot-volume-attachment-req.json\u0027 this is causing on api-ref error.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"e970e9f95846010850a8d8bd1b04c9123bc6e332","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-dot-volume-req.json"},{"line_number":221,"context_line":"   :language: javascript"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"Response"}],"source_content_type":"text/x-c++src","patch_set":21,"id":"df33271e_6d024a3f","line":220,"range":{"start_line":220,"start_character":66,"end_line":220,"end_character":69},"updated":"2020-03-31 01:50:26.000000000","message":"What does the mean of \u0027dot\u0027 in \u0027update-dot-volume-attachment-req\u0027?","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e60cac8e9dc19de37c1e5eda00280954e5fd9cc7","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-dot-volume-req.json"},{"line_number":221,"context_line":"   :language: javascript"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"Response"}],"source_content_type":"text/x-c++src","patch_set":21,"id":"df33271e_8d786ec4","line":220,"range":{"start_line":220,"start_character":66,"end_line":220,"end_character":69},"in_reply_to":"df33271e_6d024a3f","updated":"2020-03-31 01:59:22.000000000","message":"delete_on_termination ?","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-dot-volume-req.json"},{"line_number":221,"context_line":"   :language: javascript"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"Response"}],"source_content_type":"text/x-c++src","patch_set":21,"id":"df33271e_5363a842","line":220,"range":{"start_line":220,"start_character":66,"end_line":220,"end_character":69},"in_reply_to":"df33271e_8d786ec4","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":214,"context_line":"  - volumeAttachment: volumeAttachment_put"},{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json"}],"source_content_type":"text/x-c++src","patch_set":22,"id":"df33271e_7c37994e","line":217,"updated":"2020-03-31 11:45:28.000000000","message":"We have additional optional parameters in the request allowed by the schema https://review.opendev.org/#/c/693828/22/nova/api/openstack/compute/schemas/volumes.py@99 However there we need to mention that they can only be provided in the request with the same value as in the system because we don\u0027t support updating them (yet).","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8441b9493df57870f7ec1f5077e7ee46735aa969","unresolved":false,"context_lines":[{"line_number":214,"context_line":"  - volumeAttachment: volumeAttachment_put"},{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json"}],"source_content_type":"text/x-c++src","patch_set":22,"id":"df33271e_be6f4bfa","line":217,"in_reply_to":"df33271e_7c37994e","updated":"2020-03-31 20:48:21.000000000","message":"+1.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"757dcfbab8e77c6ebe1688e354f4be6112afcd75","unresolved":false,"context_lines":[{"line_number":214,"context_line":"  - volumeAttachment: volumeAttachment_put"},{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json"}],"source_content_type":"text/x-c++src","patch_set":22,"id":"df33271e_2114bc43","line":217,"in_reply_to":"df33271e_7c37994e","updated":"2020-03-31 16:19:33.000000000","message":"I\u0027m not very practiced (at all) in the art of api-ref, so I\u0027ve made some changes, but it may be quicker for someone more familiar to just fix these.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"**Example Update a volume attachment: JSON request**"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":".. literalinclude:: ../../doc/api_samples/os-volumes/v2.85/update-volume-attachment-delete-flag-req.json"},{"line_number":221,"context_line":"   :language: javascript"}],"source_content_type":"text/x-c++src","patch_set":22,"id":"df33271e_dc7ea517","line":218,"updated":"2020-03-31 11:45:28.000000000","message":"(v2.85)","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8441b9493df57870f7ec1f5077e7ee46735aa969","unresolved":false,"context_lines":[{"line_number":214,"context_line":"  - volumeAttachment: volumeAttachment_put"},{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_4433c9f6","line":218,"range":{"start_line":217,"start_character":0,"end_line":218,"end_character":39},"updated":"2020-03-31 20:48:21.000000000","message":"these does not mention the min_version: 2.85. we need to define new var.","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a2de3ba7dd57a0dcea83e739bef760c778f313f7","unresolved":false,"context_lines":[{"line_number":214,"context_line":"  - volumeAttachment: volumeAttachment_put"},{"line_number":215,"context_line":"  - volumeId: volumeId_swap"},{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_f8dd5e70","line":218,"range":{"start_line":217,"start_character":0,"end_line":218,"end_character":39},"in_reply_to":"df33271e_4433c9f6","updated":"2020-04-01 01:00:31.000000000","message":"Done","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8441b9493df57870f7ec1f5077e7ee46735aa969","unresolved":false,"context_lines":[{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":".. note:: Other than ``volumeId``, as of v2.85 only"}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_3029a49a","line":219,"range":{"start_line":219,"start_character":9,"end_line":219,"end_character":39},"updated":"2020-03-31 20:48:21.000000000","message":"this is defined as required and min_version as 2.70, we need to define the new var in with required as false and min_version as 2.85.\n\n- https://github.com/openstack/nova/blob/15974f476f524afbe3d7aef27077b1bbbfc4b9b0/api-ref/source/parameters.yaml#L2391","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a2de3ba7dd57a0dcea83e739bef760c778f313f7","unresolved":false,"context_lines":[{"line_number":216,"context_line":"  - delete_on_termination: delete_on_termination_put_req"},{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":".. note:: Other than ``volumeId``, as of v2.85 only"}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_d8da1a76","line":219,"range":{"start_line":219,"start_character":9,"end_line":219,"end_character":39},"in_reply_to":"df33271e_3029a49a","updated":"2020-04-01 01:00:31.000000000","message":"Done","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8441b9493df57870f7ec1f5077e7ee46735aa969","unresolved":false,"context_lines":[{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":".. note:: Other than ``volumeId``, as of v2.85 only"},{"line_number":223,"context_line":"          ``delete_on_termination`` may be changed from the current"}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_c454f900","line":220,"range":{"start_line":220,"start_character":0,"end_line":220,"end_character":26},"updated":"2020-03-31 20:48:21.000000000","message":"ditto to define as new var with min_version as 2.85","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a2de3ba7dd57a0dcea83e739bef760c778f313f7","unresolved":false,"context_lines":[{"line_number":217,"context_line":"  - device: attachment_device_resp"},{"line_number":218,"context_line":"  - serverId: attachment_server_id_resp"},{"line_number":219,"context_line":"  - tag: device_tag_bdm_attachment_resp"},{"line_number":220,"context_line":"  - id: attachment_id_resp"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":".. note:: Other than ``volumeId``, as of v2.85 only"},{"line_number":223,"context_line":"          ``delete_on_termination`` may be changed from the current"}],"source_content_type":"text/x-c++src","patch_set":23,"id":"df33271e_18e1222b","line":220,"range":{"start_line":220,"start_character":0,"end_line":220,"end_character":26},"in_reply_to":"df33271e_c454f900","updated":"2020-04-01 01:00:31.000000000","message":"Done","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef6c58fb59a6ed856d814267a83d5c22403e75ce","unresolved":false,"context_lines":[{"line_number":177,"context_line":".. note:: This action only valid when the server is in ACTIVE, PAUSED and RESIZED state,"},{"line_number":178,"context_line":"          or a conflict(409) error will be returned."},{"line_number":179,"context_line":""},{"line_number":180,"context_line":".. warning:: When updating volumeId, this API is typically meant to"},{"line_number":181,"context_line":"             only be used as part of a larger orchestrated volume"},{"line_number":182,"context_line":"             migration operation initiated in the block storage"},{"line_number":183,"context_line":"             service via the ``os-retype`` or ``os-migrate_volume``"}],"source_content_type":"text/x-c++src","patch_set":25,"id":"df33271e_ceb4ebf4","line":180,"range":{"start_line":180,"start_character":27,"end_line":180,"end_character":35},"updated":"2020-04-02 11:03:26.000000000","message":"``literal``","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef6c58fb59a6ed856d814267a83d5c22403e75ce","unresolved":false,"context_lines":[{"line_number":186,"context_line":"             hard reboot the server to update details within the guest"},{"line_number":187,"context_line":"             such as block storage serial IDs. Furthermore, updating"},{"line_number":188,"context_line":"             volumeId via this API is only implemented by `certain"},{"line_number":189,"context_line":"             compute drivers`_."},{"line_number":190,"context_line":""},{"line_number":191,"context_line":".. _certain compute drivers: https://docs.openstack.org/nova/latest/user/support-matrix.html#operation_swap_volume"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-c++src","patch_set":25,"id":"df33271e_2ec3d758","line":189,"updated":"2020-04-02 11:03:26.000000000","message":"this would read much better raw if you formatted like so:\n\n  .. warning::\n\n     When updating ``volumeId``, ...","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef6c58fb59a6ed856d814267a83d5c22403e75ce","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":".. _certain compute drivers: https://docs.openstack.org/nova/latest/user/support-matrix.html#operation_swap_volume"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Policy default role is \u0027rule:system_admin_or_owner\u0027, its scope is"},{"line_number":194,"context_line":"[system, project], which allow project members or system admins to"},{"line_number":195,"context_line":"change the fields of an attached volume of a server. Policy defaults"},{"line_number":196,"context_line":"enable only users with the administrative role to change ``volumeId``"}],"source_content_type":"text/x-c++src","patch_set":25,"id":"df33271e_ee1e0f0a","line":193,"range":{"start_line":193,"start_character":23,"end_line":193,"end_character":51},"updated":"2020-04-02 11:03:26.000000000","message":"nit: ``literal``","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ef6c58fb59a6ed856d814267a83d5c22403e75ce","unresolved":false,"context_lines":[{"line_number":191,"context_line":".. _certain compute drivers: https://docs.openstack.org/nova/latest/user/support-matrix.html#operation_swap_volume"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"Policy default role is \u0027rule:system_admin_or_owner\u0027, its scope is"},{"line_number":194,"context_line":"[system, project], which allow project members or system admins to"},{"line_number":195,"context_line":"change the fields of an attached volume of a server. Policy defaults"},{"line_number":196,"context_line":"enable only users with the administrative role to change ``volumeId``"},{"line_number":197,"context_line":"via this operation. Cloud providers can change these permissions"}],"source_content_type":"text/x-c++src","patch_set":25,"id":"df33271e_2e1597e5","line":194,"range":{"start_line":194,"start_character":0,"end_line":194,"end_character":17},"updated":"2020-04-02 11:03:26.000000000","message":"``literal``","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"}],"api-ref/source/parameters.yaml":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":2299,"context_line":"    A flag indicating if the attached volume will be deleted when the server is"},{"line_number":2300,"context_line":"    deleted."},{"line_number":2301,"context_line":"  in: body"},{"line_number":2302,"context_line":"  required: true"},{"line_number":2303,"context_line":"  type: boolean"},{"line_number":2304,"context_line":"  min_version: 2.85"},{"line_number":2305,"context_line":"deleted:"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"df33271e_379c155c","line":2302,"range":{"start_line":2302,"start_character":12,"end_line":2302,"end_character":16},"updated":"2020-03-30 23:28:35.000000000","message":"this is optional","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":2299,"context_line":"    A flag indicating if the attached volume will be deleted when the server is"},{"line_number":2300,"context_line":"    deleted."},{"line_number":2301,"context_line":"  in: body"},{"line_number":2302,"context_line":"  required: true"},{"line_number":2303,"context_line":"  type: boolean"},{"line_number":2304,"context_line":"  min_version: 2.85"},{"line_number":2305,"context_line":"deleted:"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"df33271e_3368a41b","line":2302,"range":{"start_line":2302,"start_character":12,"end_line":2302,"end_character":16},"in_reply_to":"df33271e_379c155c","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"}],"nova/api/openstack/api_version_request.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10b06c72638e4bb2bb9d096a6f0fe4be42b800b5","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    * 2.83 - Allow more filter parameters for ``GET /servers/detail`` and"},{"line_number":228,"context_line":"             ``GET /servers`` for non-admin."},{"line_number":229,"context_line":"    * 2.84 - Add new API"},{"line_number":230,"context_line":"             ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}``"},{"line_number":231,"context_line":"             which support for specifying ``delete_on_termination`` field in"},{"line_number":232,"context_line":"             the request body to re-config the attached volume whether to"},{"line_number":233,"context_line":"             delete when the instance is deleted."}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_6c28b9e1","line":230,"updated":"2020-03-27 17:46:02.000000000","message":"Doesn\u0027t mention PUT in here...","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":227,"context_line":"    * 2.83 - Allow more filter parameters for ``GET /servers/detail`` and"},{"line_number":228,"context_line":"             ``GET /servers`` for non-admin."},{"line_number":229,"context_line":"    * 2.84 - Add new API"},{"line_number":230,"context_line":"             ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}``"},{"line_number":231,"context_line":"             which support for specifying ``delete_on_termination`` field in"},{"line_number":232,"context_line":"             the request body to re-config the attached volume whether to"},{"line_number":233,"context_line":"             delete when the instance is deleted."}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_ec62093e","line":230,"in_reply_to":"df33271e_6c28b9e1","updated":"2020-03-27 17:49:15.000000000","message":"I completely re-wrote this but must have dropped it by accident during the merge.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"}],"nova/api/openstack/compute/rest_api_version_history.rst":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10b06c72638e4bb2bb9d096a6f0fe4be42b800b5","unresolved":false,"context_lines":[{"line_number":1109,"context_line":"2.84"},{"line_number":1110,"context_line":"----"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"Adds the ability to specify ``delete_on_termination`` in the"},{"line_number":1113,"context_line":"``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` API, which"},{"line_number":1114,"context_line":"allows changing the behavior of volume deletion on instance deletion."}],"source_content_type":"text/x-rst","patch_set":20,"id":"df33271e_2c0fd189","line":1112,"updated":"2020-03-27 17:46:02.000000000","message":"The release note mentions PATCH and this mentions PUT, but neither mention both...\n\nAlso, it doesn\u0027t look like the API reference has been updated in this change - why is that?\n\nhttps://docs.openstack.org/api-ref/compute/?expanded\u003dupdate-a-volume-attachment-detail#update-a-volume-attachment","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":1109,"context_line":"2.84"},{"line_number":1110,"context_line":"----"},{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"Adds the ability to specify ``delete_on_termination`` in the"},{"line_number":1113,"context_line":"``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` API, which"},{"line_number":1114,"context_line":"allows changing the behavior of volume deletion on instance deletion."}],"source_content_type":"text/x-rst","patch_set":20,"id":"df33271e_4c517562","line":1112,"in_reply_to":"df33271e_2c0fd189","updated":"2020-03-27 17:49:15.000000000","message":"It\u0027s updated in the last patch in the series, and per gmann\u0027s comment, will be squashed back into this once we review the diffs I just made (per my comment on it).","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"}],"nova/api/openstack/compute/schemas/volumes.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"update_volume_attachment_v285 \u003d copy.deepcopy(update_volume_attachment)"},{"line_number":99,"context_line":"# Allow device, tag, and delete_on_termination to be specified for RESTfulness,"},{"line_number":100,"context_line":"# even though we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":102,"context_line":"    \u0027properties\u0027][\u0027device\u0027] \u003d copy.deepcopy(create_volume_attachment["},{"line_number":103,"context_line":"        \u0027properties\u0027][\u0027volumeAttachment\u0027][\u0027properties\u0027][\u0027device\u0027])"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_3ccd1131","line":100,"updated":"2020-03-31 11:45:28.000000000","message":"Does RESTfulness means that the PUT body needs to be aligned with the POST request body? I think it should be aligned with the GET response body. Isn\u0027t it? If yes then we need to allow specifying (but not changing) id, and serverId as well. \n\n{\n    \"volumeAttachment\": {\n        \"delete_on_termination\": true,\n        \"device\": \"/dev/sdb\",\n        \"id\": \"a07f71dc-8151-4e7d-a0cc-cd24a3f11113\",\n        \"serverId\": \"2aad99d3-7aa4-41e9-b4e6-3f960b115d68\",\n        \"tag\": \"foo\",\n        \"volumeId\": \"a07f71dc-8151-4e7d-a0cc-cd24a3f11113\"\n    }\n}","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"757dcfbab8e77c6ebe1688e354f4be6112afcd75","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"update_volume_attachment_v285 \u003d copy.deepcopy(update_volume_attachment)"},{"line_number":99,"context_line":"# Allow device, tag, and delete_on_termination to be specified for RESTfulness,"},{"line_number":100,"context_line":"# even though we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":102,"context_line":"    \u0027properties\u0027][\u0027device\u0027] \u003d copy.deepcopy(create_volume_attachment["},{"line_number":103,"context_line":"        \u0027properties\u0027][\u0027volumeAttachment\u0027][\u0027properties\u0027][\u0027device\u0027])"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_a14e0c6c","line":100,"in_reply_to":"df33271e_3ccd1131","updated":"2020-03-31 16:19:33.000000000","message":"Don\u0027t we already? We\u0027re copying the update attachment, which was copied from create, which specifies all those things. I add back device below, which is the only place update differs from create.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8441b9493df57870f7ec1f5077e7ee46735aa969","unresolved":false,"context_lines":[{"line_number":95,"context_line":"del update_volume_attachment[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":96,"context_line":"    \u0027properties\u0027][\u0027device\u0027]"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"update_volume_attachment_v285 \u003d copy.deepcopy(update_volume_attachment)"},{"line_number":99,"context_line":"# Allow device, tag, and delete_on_termination to be specified for RESTfulness,"},{"line_number":100,"context_line":"# even though we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":102,"context_line":"    \u0027properties\u0027][\u0027device\u0027] \u003d copy.deepcopy(create_volume_attachment["},{"line_number":103,"context_line":"        \u0027properties\u0027][\u0027volumeAttachment\u0027][\u0027properties\u0027][\u0027device\u0027])"},{"line_number":104,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":105,"context_line":"                              \u0027properties\u0027][\u0027tag\u0027] \u003d parameter_types.tag"},{"line_number":106,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":107,"context_line":"    \u0027properties\u0027][\u0027delete_on_termination\u0027] \u003d parameter_types.boolean"},{"line_number":108,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":109,"context_line":"    \u0027properties\u0027][\u0027serverId\u0027] \u003d parameter_types.server_id"},{"line_number":110,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":111,"context_line":"    \u0027properties\u0027][\u0027id\u0027] \u003d parameter_types.attachment_id"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"index_query \u003d {"},{"line_number":114,"context_line":"    \u0027type\u0027: \u0027object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"df33271e_a4a7350b","line":111,"range":{"start_line":98,"start_character":0,"end_line":111,"end_character":55},"updated":"2020-03-31 20:48:21.000000000","message":"suggestion: this is good but not easily readable. we can define this new schema as raw also instead of deepcopy multiple schemas.\n\nupdate_volume_attachment_v285 \u003d {\n    \u0027type\u0027: \u0027object\u0027,\n    \u0027properties\u0027: {\n        \u0027volumeAttachment\u0027: {\n            \u0027type\u0027: \u0027object\u0027,\n            \u0027properties\u0027: {\n                \u0027volumeId\u0027: parameter_types.volume_id,\n                \u0027device\u0027: {\n                    \u0027type\u0027: [\u0027string\u0027, \u0027null\u0027],\n                    # NOTE: The validation pattern from match_device() in\n                    #       nova/block_device.py.\n                    \u0027pattern\u0027: \u0027(^/dev/x{0,1}[a-z]{0,1}d{0,1})([a-z]+)[0-9]*$\u0027\n                }\n                \u0027tag\u0027: parameter_types.tag,\n                \u0027delete_on_termination\u0027: parameter_types.boolean,\n                \u0027serverId\u0027: parameter_types.server_id,\n                \u0027id\u0027: parameter_types.attachment_id\n            },\n            \u0027required\u0027: [\u0027volumeId\u0027],\n            \u0027additionalProperties\u0027: False,\n        },\n    },\n    \u0027required\u0027: [\u0027volumeAttachment\u0027],\n    \u0027additionalProperties\u0027: False,\n}","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"a2de3ba7dd57a0dcea83e739bef760c778f313f7","unresolved":false,"context_lines":[{"line_number":95,"context_line":"del update_volume_attachment[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":96,"context_line":"    \u0027properties\u0027][\u0027device\u0027]"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"update_volume_attachment_v285 \u003d copy.deepcopy(update_volume_attachment)"},{"line_number":99,"context_line":"# Allow device, tag, and delete_on_termination to be specified for RESTfulness,"},{"line_number":100,"context_line":"# even though we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":102,"context_line":"    \u0027properties\u0027][\u0027device\u0027] \u003d copy.deepcopy(create_volume_attachment["},{"line_number":103,"context_line":"        \u0027properties\u0027][\u0027volumeAttachment\u0027][\u0027properties\u0027][\u0027device\u0027])"},{"line_number":104,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":105,"context_line":"                              \u0027properties\u0027][\u0027tag\u0027] \u003d parameter_types.tag"},{"line_number":106,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":107,"context_line":"    \u0027properties\u0027][\u0027delete_on_termination\u0027] \u003d parameter_types.boolean"},{"line_number":108,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":109,"context_line":"    \u0027properties\u0027][\u0027serverId\u0027] \u003d parameter_types.server_id"},{"line_number":110,"context_line":"update_volume_attachment_v285[\u0027properties\u0027][\u0027volumeAttachment\u0027]["},{"line_number":111,"context_line":"    \u0027properties\u0027][\u0027id\u0027] \u003d parameter_types.attachment_id"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"index_query \u003d {"},{"line_number":114,"context_line":"    \u0027type\u0027: \u0027object\u0027,"}],"source_content_type":"text/x-python","patch_set":23,"id":"df33271e_d8717a78","line":111,"range":{"start_line":98,"start_character":0,"end_line":111,"end_character":55},"in_reply_to":"df33271e_a4a7350b","updated":"2020-04-01 01:00:31.000000000","message":"Yes, use this list is easy to read, I will get your suggestion.","commit_id":"8f736524572022b7278c4b4094c7b00e3d344255"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b744cdc220cf0b82ce1d63469982c4571d01f79","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"# NOTE(brinzhang): Allow attachment_id, serverId, device, tag, and"},{"line_number":99,"context_line":"# delete_on_termination to be specified for RESTfulness, even though"},{"line_number":100,"context_line":"# we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285 \u003d {"},{"line_number":102,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":103,"context_line":"    \u0027properties\u0027: {"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_ec4f5b68","line":100,"updated":"2020-04-01 08:31:48.000000000","message":"nit:I would mention that this now follow the content of the GET response.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"9c0f041a6b62a61c78613a0fc132f7db94f5dc1e","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"# NOTE(brinzhang): Allow attachment_id, serverId, device, tag, and"},{"line_number":99,"context_line":"# delete_on_termination to be specified for RESTfulness, even though"},{"line_number":100,"context_line":"# we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285 \u003d {"},{"line_number":102,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":103,"context_line":"    \u0027properties\u0027: {"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_4b9829bf","line":100,"in_reply_to":"df33271e_ec4f5b68","updated":"2020-04-09 01:30:08.000000000","message":"Done","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4abdd8a5be63aad870b521304ede3200bd5d6bdc","unresolved":false,"context_lines":[{"line_number":98,"context_line":"# NOTE(brinzhang): Allow attachment_id, serverId, device, tag, and"},{"line_number":99,"context_line":"# delete_on_termination to be specified for RESTfulness, even though"},{"line_number":100,"context_line":"# we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285 \u003d {"},{"line_number":102,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":103,"context_line":"    \u0027properties\u0027: {"},{"line_number":104,"context_line":"        \u0027volumeAttachment\u0027: {"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_e56632ed","line":101,"updated":"2020-04-02 08:52:53.000000000","message":"still think we can copy the create schema...","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"752186bae4fe8f3db6883ac02c1e7be13080ca12","unresolved":false,"context_lines":[{"line_number":98,"context_line":"# NOTE(brinzhang): Allow attachment_id, serverId, device, tag, and"},{"line_number":99,"context_line":"# delete_on_termination to be specified for RESTfulness, even though"},{"line_number":100,"context_line":"# we will not allow updating all of them."},{"line_number":101,"context_line":"update_volume_attachment_v285 \u003d {"},{"line_number":102,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":103,"context_line":"    \u0027properties\u0027: {"},{"line_number":104,"context_line":"        \u0027volumeAttachment\u0027: {"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_a8851f09","line":101,"in_reply_to":"df33271e_e56632ed","updated":"2020-04-02 09:54:36.000000000","message":"From copy created schema, it\u0027s not easy to read, I prefre to use this way, they are same, but this way is better to the user.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b744cdc220cf0b82ce1d63469982c4571d01f79","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                \u0027delete_on_termination\u0027: parameter_types.boolean,"},{"line_number":116,"context_line":"                \u0027serverId\u0027: parameter_types.server_id,"},{"line_number":117,"context_line":"                \u0027id\u0027: parameter_types.attachment_id"},{"line_number":118,"context_line":"            },"},{"line_number":119,"context_line":"            \u0027required\u0027: [\u0027volumeId\u0027],"},{"line_number":120,"context_line":"            \u0027additionalProperties\u0027: False,"},{"line_number":121,"context_line":"        },"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_ec78fb90","line":118,"updated":"2020-04-01 08:31:48.000000000","message":"OK this now has the same field as the GET response.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"}],"nova/api/openstack/compute/volumes.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f4661e717881588fcb8aa741ca882b8a8ee5dc66","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    def update_volume(self, req, server_id, id, body):"},{"line_number":487,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":488,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":489,"context_line":"        # TODO(brinzhang): add a new policy"},{"line_number":490,"context_line":"        # \u0027os_compute_api:os-volumes-attachments:update_volume\u0027"},{"line_number":491,"context_line":"        # (PROJECT_MEMBER_OR_SYSTEM_ADMIN\u003d\u0027rule:system_admin_or_owner\u0027)"},{"line_number":492,"context_line":"        # role in volume attachments policy"},{"line_number":493,"context_line":"        # context.can(va_policies.POLICY_ROOT % \u0027update_volume\u0027,"},{"line_number":494,"context_line":"        #             target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        volume_id \u003d id"},{"line_number":497,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"df33271e_3f31cca3","line":494,"range":{"start_line":489,"start_character":0,"end_line":494,"end_character":65},"updated":"2020-03-26 16:51:51.000000000","message":"as we are going to use the PUT for this API now which has existing policy defaulted to admin-only. do we need a separate policy to control the delete_on_termination flag update? If owner is supposed to update it then we need it.","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"989959a961769b122437158fbe70336bde1fd35a","unresolved":false,"context_lines":[{"line_number":486,"context_line":"    def update_volume(self, req, server_id, id, body):"},{"line_number":487,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":488,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":489,"context_line":"        # TODO(brinzhang): add a new policy"},{"line_number":490,"context_line":"        # \u0027os_compute_api:os-volumes-attachments:update_volume\u0027"},{"line_number":491,"context_line":"        # (PROJECT_MEMBER_OR_SYSTEM_ADMIN\u003d\u0027rule:system_admin_or_owner\u0027)"},{"line_number":492,"context_line":"        # role in volume attachments policy"},{"line_number":493,"context_line":"        # context.can(va_policies.POLICY_ROOT % \u0027update_volume\u0027,"},{"line_number":494,"context_line":"        #             target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        volume_id \u003d id"},{"line_number":497,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":19,"id":"df33271e_ffdf64d3","line":494,"range":{"start_line":489,"start_character":0,"end_line":494,"end_character":65},"in_reply_to":"df33271e_3f31cca3","updated":"2020-03-26 17:16:48.000000000","message":"we discussed it on IRC, please check the consensus on - https://review.opendev.org/#/c/711194/7/nova/policies/volumes_attachments.py@93","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"897b90c5c42fffea7a6e117f3358e8cac33d13b9","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_ccbf45ce","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"updated":"2020-03-27 17:33:09.000000000","message":"should we make volumeId optional now ? for update other than swap, volumeId in request body will be duplicate with \u0027id\u0027 in url. making it optional can avoid redundant values.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c316964ae22ceeabc497232305595fe947bfede4","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_b24f9332","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_28703ddc","updated":"2020-03-31 01:43:47.000000000","message":"@Dan, I am sorry for typing this in morning on my cell phone (emm...hazy when just waking up).\n\nI mean, in the spec [1], we have discussed that if both \u0027volumeId\u0027 and \u0027delete_on_termination\u0027 are specified, it returns 400. But now \u0027volumeId\u0027 is not optional, IMO, we should support changing the \u0027delete_on_termination\u0027 property of the swapping volume (As the same as gmann\u0027s reply).\n\nIn the latest patch, I see you were already supported this and covered this scenario [2].\n\n[1]https://review.opendev.org/#/c/580336/32/specs/ussuri/approved/destroy-instance-with-datavolume.rst,unified@94\n\n[2]https://review.opendev.org/#/c/693828/21/nova/tests/unit/api/openstack/compute/test_volumes.py@1221","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"97fadbb11a09a8a21e7aa0dc94800a1d5ab1eec0","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_4c84d5b1","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_2ceb5179","updated":"2020-03-27 17:51:06.000000000","message":"See the note here:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/train/approved/support-delete-on-termination-in-server-attach-volume.html#rest-api-impact\n\nSeems the spec should be updated and get agreement on the redesign before making all of these changes.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b08d9493377bb88c4fb0f92185e0eab788d38726","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_4c09752f","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_4c84d5b1","updated":"2020-03-27 17:55:51.000000000","message":"As we said in IRC, we\u0027ll update the spec after we get this change made because we\u0027re under the gun timing-wise.\n\nI don\u0027t want the PUT body structure to differ from the GET in order to communicate intent. That\u0027s not RESTful. You could argue that in the new version we should honor changes to both the volume and the other properties in the same request if so authorized, I guess, but I\u0027m not sure how important that really is.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"33d2fd07b8ddbf3a8abe61d4c5ae99dc1a915520","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_a8a72da7","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_625ee644","updated":"2020-03-30 14:45:19.000000000","message":"ok for not making optional.\n\nWhat if delete_on_termination is present with different volume-id? I mean request for swap but delete_on_termination also present. we can accept both request in that case ? if requested new volume need to be adjusted for delete flag ?","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5a60a053064800c85d28848f7486c3aa7d098f0d","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_625ee644","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_8fe52aab","updated":"2020-03-30 13:47:21.000000000","message":"Brin, I\u0027m don\u0027t understand your question. Can you rephrase?","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"65ac1166fc58204b23063d50c33e42ec47c34ff9","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_28703ddc","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_a8a72da7","updated":"2020-03-30 14:51:07.000000000","message":"Ack.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"d9d70820949c93afe32ad37ee9f9135feb8bd87f","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_8fe52aab","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_cc2e25f9","updated":"2020-03-27 23:27:01.000000000","message":"If \u0027volumeId\u0027 is not optional, do we set \u0027delete_on_termination\u0027 at the same time as \u0027delete_on_termination\u0027 in the request body?","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_cc2e25f9","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_ccbf45ce","updated":"2020-03-27 17:49:15.000000000","message":"TBH, I\u0027d rather none of it be optional, since it\u0027s a PUT. I made the items below optional for cinder compatibility, but I think that it\u0027s better to err on the side of less optional-ness. Just MHO, of course.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5272e694f789bdf2e9f57fbe4455fe0f4db71cc5","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.84\u0027)"},{"line_number":447,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_2ceb5179","line":447,"range":{"start_line":447,"start_character":30,"end_line":447,"end_character":66},"in_reply_to":"df33271e_ccbf45ce","updated":"2020-03-27 17:49:49.000000000","message":"There was a time when the spec was proposing adding `delete_on_termination` to the PUT API and would be mutually exclusive with the volumeId for the existing swap operation, i.e. you could update the delete_on_termination flag *or* swap the volume but not both in the same operation, but it looks like that\u0027s not the case here?","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"897b90c5c42fffea7a6e117f3358e8cac33d13b9","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":459,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"        if attachment[\u0027volumeId\u0027] !\u003d volume_id:"},{"line_number":462,"context_line":"            raise exc.HTTPBadRequest(explanation\u003d\u0027The volumeId property is \u0027"},{"line_number":463,"context_line":"                                     \u0027not mutable\u0027)"},{"line_number":464,"context_line":"        try:"},{"line_number":465,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":466,"context_line":"                context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_cc08e5da","line":463,"range":{"start_line":461,"start_character":0,"end_line":463,"end_character":51},"updated":"2020-03-27 17:33:09.000000000","message":"this would hit anytime as we already check at L447","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":459,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"        if attachment[\u0027volumeId\u0027] !\u003d volume_id:"},{"line_number":462,"context_line":"            raise exc.HTTPBadRequest(explanation\u003d\u0027The volumeId property is \u0027"},{"line_number":463,"context_line":"                                     \u0027not mutable\u0027)"},{"line_number":464,"context_line":"        try:"},{"line_number":465,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":466,"context_line":"                context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_4c233506","line":463,"range":{"start_line":461,"start_character":0,"end_line":463,"end_character":51},"in_reply_to":"df33271e_cc08e5da","updated":"2020-03-27 17:49:15.000000000","message":"You\u0027re right, I added this before I combined the old and new update() methods to make sure I didn\u0027t allow non-admin to change the volume.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"897b90c5c42fffea7a6e117f3358e8cac33d13b9","unresolved":false,"context_lines":[{"line_number":468,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":469,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":470,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":471,"context_line":"            # FIXME(danms): Should we just allow tag here instead of requiring"},{"line_number":472,"context_line":"            # a new microversion later?"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_8c189d75","line":472,"range":{"start_line":471,"start_character":28,"end_line":472,"end_character":39},"updated":"2020-03-27 17:33:09.000000000","message":"+1, it make sense to update tag and doing that in this microversion is good opportunity.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"d9d70820949c93afe32ad37ee9f9135feb8bd87f","unresolved":false,"context_lines":[{"line_number":468,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":469,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":470,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":471,"context_line":"            # FIXME(danms): Should we just allow tag here instead of requiring"},{"line_number":472,"context_line":"            # a new microversion later?"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_cff0d26a","line":472,"range":{"start_line":471,"start_character":28,"end_line":472,"end_character":39},"in_reply_to":"df33271e_8c189d75","updated":"2020-03-27 23:27:01.000000000","message":"Make sense, this change is a surprise","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9abe6c3eba72d0f2f6e6e2209cc1166ade1fa1f5","unresolved":false,"context_lines":[{"line_number":468,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":469,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":470,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":471,"context_line":"            # FIXME(danms): Should we just allow tag here instead of requiring"},{"line_number":472,"context_line":"            # a new microversion later?"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_6e0ff636","line":472,"range":{"start_line":471,"start_character":28,"end_line":472,"end_character":39},"in_reply_to":"df33271e_cff0d26a","updated":"2020-03-30 10:21:43.000000000","message":"+1 on the general idea for completness but this might slow us down.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"897b90c5c42fffea7a6e117f3358e8cac33d13b9","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_4c1df50d","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"updated":"2020-03-27 17:33:09.000000000","message":"if \u0027delete_on_termination\u0027 is present then we do not need to update.\n\nshould we 400 if nothing to update ? means if \u0027\u0027delete_on_termination\u0027\u0027 (or tag if we make it updatable) is not present or with same value ? or just ignore the request and return 202.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"65ac1166fc58204b23063d50c33e42ec47c34ff9","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_486b01f0","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"in_reply_to":"df33271e_485ae182","updated":"2020-03-30 14:51:07.000000000","message":"I\u0027m not sure what the concern is. If delete_on_termination is not in attachment, then we will get None from the get, we won\u0027t pass the condition on 477 and thus will never run 478 right?","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"3500dcbaef0cf27e829effcbe9b612fd3ee94706","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_8826a9cd","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"in_reply_to":"df33271e_486b01f0","updated":"2020-03-30 14:53:29.000000000","message":"ah sorry, i overread the 477 condition its \u0027not in\u0027.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_4c685520","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"in_reply_to":"df33271e_4c1df50d","updated":"2020-03-27 17:49:15.000000000","message":"Technically if I put the same thing back that I just got from GET, that should be fine and I\u0027m not sure why someone would expect not to be able to do that. In the context of \"update\" it seems like an error if nothing is updated makes sense, but in the true sense of a PUT, I would expect no update. So I dunno, I tend to think the latter is better (no error), but happy to entertain arguments.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9abe6c3eba72d0f2f6e6e2209cc1166ade1fa1f5","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_ae1a9e6b","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"in_reply_to":"df33271e_4c685520","updated":"2020-03-30 10:21:43.000000000","message":"I\u0027m on the no-error side. It is like deleting something that is already gone. The end state of the system is the one the client requested so no error.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"33d2fd07b8ddbf3a8abe61d4c5ae99dc1a915520","unresolved":false,"context_lines":[{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_485ae182","line":477,"range":{"start_line":477,"start_character":20,"end_line":477,"end_character":24},"in_reply_to":"df33271e_ae1a9e6b","updated":"2020-03-30 14:45:19.000000000","message":"ok. i think no error is best option here. \n\nmy second question was for None value. if  \u0027delete_on_termination\u0027 is not present means L476 attachment.get(\u0027delete_on_termination\u0027) is None then we should not update or L478 will be KeyError on that case. we need to exclude the \u0027delete_on_termination\u0027 not present situation.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c316964ae22ceeabc497232305595fe947bfede4","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":468,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":471,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":472,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_cd4d56c5","line":470,"updated":"2020-03-31 01:43:47.000000000","message":"This seems a bit strange, when request microversion \u003e\u003d 2.85, we need to perform a swap volume. After the swap volume is completed, we need to update the attachment. At this time, if \u0027device\u0027 and/or \u0027tag\u0027 is in the body [\u0027volumeAttachment\u0027] The operator will receive HTTPBadRequest (400). In fact, the swap volume has been completed, but it has only updated the attributes it already contains, but our return information will mislead the operator, the request failed, right?\n\nLater: you are right, it\u0027s \u0027not in\u0027, it\u0027s ok.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":472,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_17911148","line":475,"range":{"start_line":474,"start_character":53,"end_line":475,"end_character":54},"updated":"2020-03-30 23:28:35.000000000","message":"we are not updating tag in this version? if we think someone will ask to do the same in fiture then, I think we should do to avoid multiple version bump.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":472,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_21904aed","line":475,"range":{"start_line":474,"start_character":53,"end_line":475,"end_character":54},"in_reply_to":"df33271e_17911148","updated":"2020-03-31 08:34:11.000000000","message":"wait for Dan reply on this, or directly update to the latest patch.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":447,"context_line":"        if only_swap or id !\u003d attachment[\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            self._update_volume_swap(req, server_id, id, body)"},{"line_number":451,"context_line":"            if only_swap:"},{"line_number":452,"context_line":"                # If the version is before other attributes were supported,"},{"line_number":453,"context_line":"                # then we return after the swap is done. If the version is"},{"line_number":454,"context_line":"                # new enough, we can fall through and continue to update"},{"line_number":455,"context_line":"                # other attributes if and after the swap completed."},{"line_number":456,"context_line":"                return"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        # FIXME(danms): Check the volume attachment upate policy here"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        try:"},{"line_number":461,"context_line":"            # Make sure the volume in path is valid"},{"line_number":462,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":463,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":464,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"        try:"},{"line_number":467,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":468,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":471,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":472,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"},{"line_number":481,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"    @wsgi.response(202)"},{"line_number":484,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_562431af","line":481,"range":{"start_line":450,"start_character":0,"end_line":481,"end_character":66},"updated":"2020-03-31 11:45:28.000000000","message":"_update_volume_swap will call swap_volume RPC, but that RPC is a cast. This causes that _update_volume_swap returns before the actual swap happened on the compute side. Then querying BDM, if fast enough, can fail as the swap did not happened yet and therefore the new volume is not assigned to the instance.\n\nThis wasn\u0027t the problem before this change as the RPC was the last step in the whole API handling code.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"757dcfbab8e77c6ebe1688e354f4be6112afcd75","unresolved":false,"context_lines":[{"line_number":447,"context_line":"        if only_swap or id !\u003d attachment[\u0027volumeId\u0027]:"},{"line_number":448,"context_line":"            # This is a swap operation"},{"line_number":449,"context_line":"            # FIXME(danms): Check the swap volume policy here"},{"line_number":450,"context_line":"            self._update_volume_swap(req, server_id, id, body)"},{"line_number":451,"context_line":"            if only_swap:"},{"line_number":452,"context_line":"                # If the version is before other attributes were supported,"},{"line_number":453,"context_line":"                # then we return after the swap is done. If the version is"},{"line_number":454,"context_line":"                # new enough, we can fall through and continue to update"},{"line_number":455,"context_line":"                # other attributes if and after the swap completed."},{"line_number":456,"context_line":"                return"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        # FIXME(danms): Check the volume attachment upate policy here"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        try:"},{"line_number":461,"context_line":"            # Make sure the volume in path is valid"},{"line_number":462,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":463,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":464,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"        try:"},{"line_number":467,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":468,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":471,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":472,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":473,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"},{"line_number":474,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The tag property is \u0027"},{"line_number":475,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":476,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":477,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":478,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":479,"context_line":"            bdm.save()"},{"line_number":480,"context_line":"        except exception.VolumeBDMNotFound as e:"},{"line_number":481,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"    @wsgi.response(202)"},{"line_number":484,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_610084ac","line":481,"range":{"start_line":450,"start_character":0,"end_line":481,"end_character":66},"in_reply_to":"df33271e_562431af","updated":"2020-03-31 16:19:33.000000000","message":"Oye, I should have checked the RPC call because I assumed that swap was synchronous given the cinder workflow. Obviously, as you mention, the functional test problem should have been the clue, but I was in whack-a-mole mode.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"dfdcd3313a6dbe07c5b5aa755219d097bd50bb0f","unresolved":false,"context_lines":[{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_932a3529","line":455,"range":{"start_line":450,"start_character":12,"end_line":455,"end_character":58},"updated":"2020-04-01 02:28:33.000000000","message":"Missing test case for \u0027id\u0027 and \u0027serverId\u0027 negative scenario.","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d63bcf1ef4e677e4cd2f1a7f0790aa323271baa3","unresolved":false,"context_lines":[{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_d3a03d93","line":455,"range":{"start_line":450,"start_character":12,"end_line":455,"end_character":58},"in_reply_to":"df33271e_932a3529","updated":"2020-04-01 02:47:30.000000000","message":"yeah, we can add those.","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b34e1573faa6f8c423e8e154235fa1de3fa639ed","unresolved":false,"context_lines":[{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_cef7f08a","line":455,"range":{"start_line":450,"start_character":12,"end_line":455,"end_character":58},"in_reply_to":"df33271e_d3a03d93","updated":"2020-04-01 05:00:39.000000000","message":"Done","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"dfdcd3313a6dbe07c5b5aa755219d097bd50bb0f","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_b325b95e","line":494,"updated":"2020-04-01 02:28:33.000000000","message":"If microversion \u003e 2.85 the user want to do update + swap for a new volume (instance_uuid is None, not attached to the instance), the bdm will be always failed, right?\nBecause L446 will be raised VolumeBDMNotFound [1]\n\n[1]https://opendev.org/openstack/nova/src/branch/master/nova/objects/block_device.py#L281-L282","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d63bcf1ef4e677e4cd2f1a7f0790aa323271baa3","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_f3d101fd","line":494,"in_reply_to":"df33271e_b325b95e","updated":"2020-04-01 02:47:30.000000000","message":"there is no difference in this exception becasue swap operation also get the bdm with old volume id. failing it on during updating or during swap is no difference. I mean we are not doing any extra step in update which we do not do in swap.\n\n- https://opendev.org/openstack/nova/src/commit/c31de903dd73de1c0a41bcfe4c867daa6eb10f85/nova/compute/api.py#L4777","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b744cdc220cf0b82ce1d63469982c4571d01f79","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_cc36d703","line":494,"in_reply_to":"df33271e_c939aad7","updated":"2020-04-01 08:31:48.000000000","message":"A L446 the volume_id is the id from the URI, so it is the old volume, so the query will find the BDM.","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b028c2c7e96a7cdcded51a02344ffc0de5c8822f","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_6f24dd5e","line":494,"in_reply_to":"df33271e_cc36d703","updated":"2020-04-01 08:45:06.000000000","message":"Oh yes, at this moment the brain was blocked ...","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b34e1573faa6f8c423e8e154235fa1de3fa639ed","unresolved":false,"context_lines":[{"line_number":491,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_c939aad7","line":494,"in_reply_to":"df33271e_f3d101fd","updated":"2020-04-01 05:00:39.000000000","message":"yes, but I also have some trepidation if the user used a new volume to swap. That because the \u0027update\u0027 operation is abnormal, swap cannot be executed normally.","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"dfdcd3313a6dbe07c5b5aa755219d097bd50bb0f","unresolved":false,"context_lines":[{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"},{"line_number":499,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_53342d93","line":496,"updated":"2020-04-01 02:28:33.000000000","message":"\u0027update\u0027 will be used admin_or_owner role, but \u0027swap\u0027 will be used admin role. If the user is an admin_or_owner role request \u0027volumeId\u0027(!\u003d \u0027id\u0027) to update the volume, we will update the volume_id in PUT path, ignored the \u0027volumeId\u0027, but that will do swap action, so we also need to check policy after \"if id !\u003d volume_id:\" condition, right?","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d63bcf1ef4e677e4cd2f1a7f0790aa323271baa3","unresolved":false,"context_lines":[{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"},{"line_number":499,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_b3fdb96f","line":496,"in_reply_to":"df33271e_53342d93","updated":"2020-04-01 02:47:30.000000000","message":"that is done in next patch, though that need to be rebase on this. there we can check both policy in starting itself. - https://review.opendev.org/#/c/711194/10/nova/api/openstack/compute/volumes.py@456","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"b34e1573faa6f8c423e8e154235fa1de3fa639ed","unresolved":false,"context_lines":[{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"},{"line_number":499,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":24,"id":"df33271e_4e2000ad","line":496,"in_reply_to":"df33271e_b3fdb96f","updated":"2020-04-01 05:00:39.000000000","message":"I rebased and update https://review.opendev.org/#/c/711194/11.","commit_id":"3078978af9da0947b894e8cdfcebcd84195a4e47"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4abdd8a5be63aad870b521304ede3200bd5d6bdc","unresolved":false,"context_lines":[{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _update_volume_regular(self, req, server_id, id, body):"},{"line_number":431,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":432,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":433,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":434,"context_line":"        # NOTE(danms): We may be doing an update of regular parameters in"},{"line_number":435,"context_line":"        # the midst of a swap operation, so to find the original BDM, we need"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_6567c24e","line":432,"updated":"2020-04-02 08:52:53.000000000","message":"this is wasteful, you already get instance at line 476","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"752186bae4fe8f3db6883ac02c1e7be13080ca12","unresolved":false,"context_lines":[{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _update_volume_regular(self, req, server_id, id, body):"},{"line_number":431,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":432,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":433,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":434,"context_line":"        # NOTE(danms): We may be doing an update of regular parameters in"},{"line_number":435,"context_line":"        # the midst of a swap operation, so to find the original BDM, we need"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_eb0f212b","line":432,"in_reply_to":"df33271e_6567c24e","updated":"2020-04-02 09:54:36.000000000","message":"I think we can access this, if you insist on not using it, I can optimize it in the comments of follow-up gibi and gmann and pass the instance in the update API (L475) as a parameter.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e10e5cebdb3fc0d726bda34fc8bc839ea7b93558","unresolved":false,"context_lines":[{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _update_volume_regular(self, req, server_id, id, body):"},{"line_number":431,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":432,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":433,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":434,"context_line":"        # NOTE(danms): We may be doing an update of regular parameters in"},{"line_number":435,"context_line":"        # the midst of a swap operation, so to find the original BDM, we need"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_d1d92b83","line":432,"in_reply_to":"df33271e_6567c24e","updated":"2020-04-02 14:38:01.000000000","message":"This is duplicated for convenience but is collapsed in the next patch (which needs updating) after the policy checks get fixed.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d94a2b32bc233a445b54ed4f7d11d80224ad86aa","unresolved":false,"context_lines":[{"line_number":429,"context_line":""},{"line_number":430,"context_line":"    def _update_volume_regular(self, req, server_id, id, body):"},{"line_number":431,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":432,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":433,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":434,"context_line":"        # NOTE(danms): We may be doing an update of regular parameters in"},{"line_number":435,"context_line":"        # the midst of a swap operation, so to find the original BDM, we need"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_39ede3b6","line":432,"in_reply_to":"df33271e_eb0f212b","updated":"2020-04-02 11:55:56.000000000","message":"I don\u0027t think it makes sense to access the DB twices.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4abdd8a5be63aad870b521304ede3200bd5d6bdc","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        try:"},{"line_number":440,"context_line":"            # Make sure the volume in path is valid"},{"line_number":441,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":442,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":443,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_25701a08","line":441,"range":{"start_line":441,"start_character":12,"end_line":441,"end_character":51},"updated":"2020-04-02 08:52:53.000000000","message":"not sure we need this. Since we can get volume id from BDM.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cc9a8a89fd0b3db7f6a83603be58f476aaf9a93","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        try:"},{"line_number":440,"context_line":"            # Make sure the volume in path is valid"},{"line_number":441,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":442,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":443,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_e4b3cb96","line":441,"range":{"start_line":441,"start_character":12,"end_line":441,"end_character":51},"in_reply_to":"df33271e_08624b0a","updated":"2020-04-02 15:20:52.000000000","message":"I included it because we previously did this in the swap path to make sure the volume was correct. However, you\u0027re probably right that if we\u0027re not doing a swap, we\u0027re really just updating the BDM and thus checking the volume in cinder is not strictly necessary.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"752186bae4fe8f3db6883ac02c1e7be13080ca12","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        try:"},{"line_number":440,"context_line":"            # Make sure the volume in path is valid"},{"line_number":441,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":442,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":443,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_08624b0a","line":441,"range":{"start_line":441,"start_character":12,"end_line":441,"end_character":51},"in_reply_to":"df33271e_25701a08","updated":"2020-04-02 09:54:36.000000000","message":"Just check the volume is valid in path.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b744cdc220cf0b82ce1d63469982c4571d01f79","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_4cc3e789","line":449,"updated":"2020-04-01 08:31:48.000000000","message":"Correct based on how the GET works. There is an attachment_id in the BDM as well but that is, interestingly, not used here.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d507f854beaf01498379a6634133bf7ccadb73c9","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_5727e764","line":449,"in_reply_to":"df33271e_4cc3e789","updated":"2020-04-01 16:48:16.000000000","message":"true, basically this- https://github.com/openstack/nova/blob/15974f476f524afbe3d7aef27077b1bbbfc4b9b0/nova/api/openstack/compute/volumes.py#L239","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"ecd4e9fccc377df752eb407f37145427b6e76a2c","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":447,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"            # NOTE(danms): The attachment id is just the (current) volume id"},{"line_number":450,"context_line":"            if attachment.get(\u0027id\u0027) not in (None, volume_id):"},{"line_number":451,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The id property is \u0027"},{"line_number":452,"context_line":"                                         \u0027not mutable\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_c3c32fec","line":449,"in_reply_to":"df33271e_5727e764","updated":"2020-04-02 03:10:35.000000000","message":"IMO, in next cleanup API, maybe we can remove this, or instead of attachment_id, wo show \u0027is\u0027 but cannot easy know who is attachment_id or volume_id.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"cfc6ddb7095fa4b6ac5afc961a4d79cca15e5b64","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":459,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_28e0ef25","line":456,"range":{"start_line":456,"start_character":48,"end_line":456,"end_character":52},"updated":"2020-04-02 09:14:35.000000000","message":"device field support None value.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"752186bae4fe8f3db6883ac02c1e7be13080ca12","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":459,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_681f976b","line":456,"range":{"start_line":456,"start_character":48,"end_line":456,"end_character":52},"in_reply_to":"df33271e_28e0ef25","updated":"2020-04-02 09:54:36.000000000","message":"It\u0027s condition is \"not in\", only when the device value changes and at the same we want to update the volume, that will raise 400, otherwise we do nothing.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d94a2b32bc233a445b54ed4f7d11d80224ad86aa","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":459,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_f9cd3b5d","line":456,"range":{"start_line":456,"start_character":48,"end_line":456,"end_character":52},"in_reply_to":"df33271e_681f976b","updated":"2020-04-02 11:55:56.000000000","message":"So...If the None is valid value. That means you can change device value to None, so you should raise 400 for that case.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d967b451ecf2be7f01a0c7488c3dfd4b0b09a3c3","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":459,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_c220379e","line":456,"range":{"start_line":456,"start_character":48,"end_line":456,"end_character":52},"in_reply_to":"df33271e_e7faadba","updated":"2020-04-02 15:57:33.000000000","message":"NOTE: any other field restricts None as value in the schema but \u0027device\u0027 allows None as a value so we need to check hasattr. Adding simple tests for those also will be good","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"338bf6bef9a9cbd21eb0e32387aedf8f65778750","unresolved":false,"context_lines":[{"line_number":453,"context_line":"            if attachment.get(\u0027serverId\u0027) not in (None, server_id):"},{"line_number":454,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The serverId property \u0027"},{"line_number":455,"context_line":"                                         \u0027is not mutable\u0027)"},{"line_number":456,"context_line":"            if attachment.get(\u0027device\u0027) not in (None, bdm.device_name):"},{"line_number":457,"context_line":"                raise exc.HTTPBadRequest(explanation\u003d\u0027The device property is \u0027"},{"line_number":458,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":459,"context_line":"            if attachment.get(\u0027tag\u0027) not in (None, bdm.tag):"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_e7faadba","line":456,"range":{"start_line":456,"start_character":48,"end_line":456,"end_character":52},"in_reply_to":"df33271e_f9cd3b5d","updated":"2020-04-02 15:40:49.000000000","message":"yeah, we did not think on this. we should check hasattr here not None","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b35f87525c296b3ab788e9490e94a8ed23363de9","unresolved":false,"context_lines":[{"line_number":471,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment, \u00272.0\u0027, \u00272.84\u0027)"},{"line_number":472,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment_v285,"},{"line_number":473,"context_line":"                       min_version\u003d\u00272.85\u0027)"},{"line_number":474,"context_line":"    def update(self, req, server_id, id, body):"},{"line_number":475,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":476,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":477,"context_line":"        # TODO(danms): For now, use the existing admin-only policy for update."}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_d45abe80","line":474,"range":{"start_line":474,"start_character":37,"end_line":474,"end_character":39},"updated":"2020-04-02 12:40:43.000000000","message":"id is a built in function in python\n\nHelp on built-in function id in module __builtin__:\n\nid(...)\n    id(object) -\u003e integer\n    \n    Return the identity of an object.  This is guaranteed to be unique among\n    simultaneously existing objects.  (Hint: it\u0027s the object\u0027s memory address.)\n\nso you should not alias it by using it as a variable\n\nthat is why its is being syntax hilighted in blue","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"338bf6bef9a9cbd21eb0e32387aedf8f65778750","unresolved":false,"context_lines":[{"line_number":471,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment, \u00272.0\u0027, \u00272.84\u0027)"},{"line_number":472,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment_v285,"},{"line_number":473,"context_line":"                       min_version\u003d\u00272.85\u0027)"},{"line_number":474,"context_line":"    def update(self, req, server_id, id, body):"},{"line_number":475,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":476,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":477,"context_line":"        # TODO(danms): For now, use the existing admin-only policy for update."}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_c79fa94d","line":474,"range":{"start_line":474,"start_character":37,"end_line":474,"end_character":39},"in_reply_to":"df33271e_bfeb9375","updated":"2020-04-02 15:40:49.000000000","message":"yeah that is problem in many API methods.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"cc3e8de9b65d80bbd10cc25da2900bd399a08a0f","unresolved":false,"context_lines":[{"line_number":471,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment, \u00272.0\u0027, \u00272.84\u0027)"},{"line_number":472,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment_v285,"},{"line_number":473,"context_line":"                       min_version\u003d\u00272.85\u0027)"},{"line_number":474,"context_line":"    def update(self, req, server_id, id, body):"},{"line_number":475,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":476,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":477,"context_line":"        # TODO(danms): For now, use the existing admin-only policy for update."}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_820c6694","line":474,"range":{"start_line":474,"start_character":37,"end_line":474,"end_character":39},"in_reply_to":"df33271e_c79fa94d","updated":"2020-04-03 00:52:17.000000000","message":"maybe, we can try to fix this later, we should avoid using python keywords. I will conduct a review of all api entries later. I will list api interfaces that use \u0027id\u0027.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b9aa02bff65a2a52995b40d4295af64062bb5bb4","unresolved":false,"context_lines":[{"line_number":471,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment, \u00272.0\u0027, \u00272.84\u0027)"},{"line_number":472,"context_line":"    @validation.schema(volumes_schema.update_volume_attachment_v285,"},{"line_number":473,"context_line":"                       min_version\u003d\u00272.85\u0027)"},{"line_number":474,"context_line":"    def update(self, req, server_id, id, body):"},{"line_number":475,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":476,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, server_id)"},{"line_number":477,"context_line":"        # TODO(danms): For now, use the existing admin-only policy for update."}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_bfeb9375","line":474,"range":{"start_line":474,"start_character":37,"end_line":474,"end_character":39},"in_reply_to":"df33271e_d45abe80","updated":"2020-04-02 13:11:47.000000000","message":"in case you are wondering why this is bad\nif you do \n\npython3 -c \"id\u003ddir; help(id)\"\n\nit will print the help text for dir as it will do a replacement of the id fucntion in the context wehere it is redefined.\n\nso if you ever did this at module scope it would do it globally.\n\nthat said i talked about this with alex on irc and this is an existing issue so im going to remvoe the -1 \n\nbut we shoudl remove id from all the rotes defiend in\n\nhttps://github.com/openstack/nova/blob/master/nova/api/openstack/compute/routes.py#L820\n\nat some point","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4abdd8a5be63aad870b521304ede3200bd5d6bdc","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_052a36d9","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"updated":"2020-04-02 08:52:53.000000000","message":"I\u0027m curious why we still do the swap when volume id are same","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"dfea53b8a1bb7ee95c8b93386a0939452e57eb12","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_a5470a77","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_052a36d9","updated":"2020-04-02 08:59:56.000000000","message":"correct my words....I\u0027m curious that in the before why we still do the swap when volume ids are same.\n\nI want to know is there any syntax we changed at here.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d94a2b32bc233a445b54ed4f7d11d80224ad86aa","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_59c42740","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_0879abc6","updated":"2020-04-02 11:55:56.000000000","message":"Emm...that doesn\u0027t answer my question I think.\n\nI mean why we can swap the volume with same volume. Is it bug? or something useless. I don\u0027t want to lose any feature we have before.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"76983a487f4a89cac62ace6135e6d780865072ca","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_42789e8e","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_31f2179b","updated":"2020-04-03 00:22:52.000000000","message":"After checking more, I think this will fail at the check https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4762, so same volume, the status will be attached.\n\nSo we change a HTTPBadRequest 400 to silence here.\n\nSince this is new microversion, so it is ok we change this behavior.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f6b25941e50a1958cbf3db3d2154b7872f8ac45e","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_820d86e9","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_42789e8e","updated":"2020-04-03 00:32:20.000000000","message":"yeah, for delete flag also if value is same then we do not error and just return success. keeping the same behaviour for volumeId also make consistent. But we can mention this in reno/doc for interoperability.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e10e5cebdb3fc0d726bda34fc8bc839ea7b93558","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_31f2179b","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_59c42740","updated":"2020-04-02 14:38:01.000000000","message":"As far as I can tell the old code would allow swapping the same volume, but I can\u0027t see how it would actually work from the swap code, so I think that probably generated a very strange error if you tried it. Even still, it\u0027s very un-RESTful to take a PUT of the same value and trigger some change, so I would expect we need a very compelling reason to support that if so.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"75edd512ab9dfdc99e8c1229bccef705c0802937","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_e2a4524d","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_820d86e9","updated":"2020-04-03 01:06:20.000000000","message":"\u003e yeah, for delete flag also if value is same then we do not error\n \u003e and just return success. keeping the same behaviour for volumeId\n \u003e also make consistent. But we can mention this in reno/doc for\n \u003e interoperability.\n\nThanks, that make sense","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"cc3e8de9b65d80bbd10cc25da2900bd399a08a0f","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_422d3efc","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_820d86e9","updated":"2020-04-03 00:52:17.000000000","message":"IMHO, swap with the same volume and return 400, this is really a useless use case.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"752186bae4fe8f3db6883ac02c1e7be13080ca12","unresolved":false,"context_lines":[{"line_number":492,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":493,"context_line":"            # FIXME(danms): Check the volume attachment update policy here"},{"line_number":494,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":495,"context_line":"            if id !\u003d volume_id:"},{"line_number":496,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_0879abc6","line":495,"range":{"start_line":495,"start_character":12,"end_line":495,"end_character":31},"in_reply_to":"df33271e_a5470a77","updated":"2020-04-02 09:54:36.000000000","message":"We moved this to https://review.opendev.org/#/c/711194/12/nova/api/openstack/compute/volumes.py@503, change this condition to authorized check. And we disscussed the policy\u0027s scenarios in https://review.opendev.org/#/c/711194/11/nova/api/openstack/compute/volumes.py@480","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b1a0463b1b1e994693e6dad26b5bf481a54c193b","unresolved":false,"context_lines":[{"line_number":436,"context_line":""},{"line_number":437,"context_line":"        try:"},{"line_number":438,"context_line":"            # Make sure the volume in path is valid"},{"line_number":439,"context_line":"            self.volume_api.get(context, volume_id)"},{"line_number":440,"context_line":"        except exception.VolumeNotFound as e:"},{"line_number":441,"context_line":"            raise exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":442,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"df33271e_224e4337","line":439,"updated":"2020-04-02 16:14:04.000000000","message":"I meant to remove this and forgot.","commit_id":"45232036307ff7ff4c2064722f7e96b1b174514d"}],"nova/api/openstack/wsgi.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8335044a5849c20e3fdd2120b53d753b456cd91","unresolved":false,"context_lines":[{"line_number":51,"context_line":"_METHODS_WITH_BODY \u003d ["},{"line_number":52,"context_line":"    \u0027POST\u0027,"},{"line_number":53,"context_line":"    \u0027PUT\u0027,"},{"line_number":54,"context_line":"    \u0027PATCH\u0027,"},{"line_number":55,"context_line":"]"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# The default api version request if none is requested in the headers"}],"source_content_type":"text/x-python","patch_set":19,"id":"1fa4df85_03146a4d","line":54,"updated":"2020-03-17 20:30:44.000000000","message":"This is a big move, our first PATCH that I know of.\n\nI apologize if this was discussed at length before, but why is this not a PUT operation?","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"706d36c24c4517f2ee6608e5aefb69bd143c9e1a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"_METHODS_WITH_BODY \u003d ["},{"line_number":52,"context_line":"    \u0027POST\u0027,"},{"line_number":53,"context_line":"    \u0027PUT\u0027,"},{"line_number":54,"context_line":"    \u0027PATCH\u0027,"},{"line_number":55,"context_line":"]"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# The default api version request if none is requested in the headers"}],"source_content_type":"text/x-python","patch_set":19,"id":"1fa4df85_a4023c0a","line":54,"in_reply_to":"1fa4df85_03146a4d","updated":"2020-03-18 02:34:05.000000000","message":"SPEC was originally implemented using the PUT API [1], but the original PUT API was changed a lot. In addition, it is restricted by the Policy. The PUT API is for the administrative role but this will be allowed admin_or_owner role. From the IRC meeting discussed [2] and finally adopted Sean\u0027s alternative suggestion [3], and adopted PATCH to update the attached volume. If you want to get more details of this that you can review the latest merged spec [4]\n\n[1]https://review.opendev.org/#/c/580336/32/specs/ussuri/approved/destroy-instance-with-datavolume.rst\n\n[2]http://eavesdrop.openstack.org/meetings/nova/2020/nova.2020-02-20-14.00.log.html#l-82\n\n[3]https://review.opendev.org/#/c/580336/32/specs/ussuri/approved/destroy-instance-with-datavolume.rst@51\n\n[4]https://review.opendev.org/#/c/580336/35","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"961f17b24e4946209be905094525b3e5f20252a1","unresolved":false,"context_lines":[{"line_number":51,"context_line":"_METHODS_WITH_BODY \u003d ["},{"line_number":52,"context_line":"    \u0027POST\u0027,"},{"line_number":53,"context_line":"    \u0027PUT\u0027,"},{"line_number":54,"context_line":"    \u0027PATCH\u0027,"},{"line_number":55,"context_line":"]"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"# The default api version request if none is requested in the headers"}],"source_content_type":"text/x-python","patch_set":19,"id":"df33271e_5ba2ed4c","line":54,"in_reply_to":"1fa4df85_a4023c0a","updated":"2020-03-26 10:22:31.000000000","message":"Yeah. This was really back and forth discussed in the spec. Unfortunately the current PUT /servers/{server_id}/os-volume_attachments/{volume_id} [1] is used for the admin-only, swap volume operation. That is a fairly obscure and special API now where we cannot easily add a user facing update possibility on the attachment paramters. Adding a PATCH method would separate the swap volume from updating fields on volume attachments nicely.\n\n[1]https://docs.openstack.org/api-ref/compute/#update-a-volume-attachment","commit_id":"cde83a501f341cba316cd24c829222323d40010a"}],"nova/compute/manager.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"6d043f12eb194ea1aaa9c5a868a2c4f5799e64b0","unresolved":false,"context_lines":[{"line_number":6926,"context_line":"                  \"%(updates)s\", {\u0027volume_id\u0027: bdm.volume_id,"},{"line_number":6927,"context_line":"                                  \u0027updates\u0027: values},"},{"line_number":6928,"context_line":"                  instance\u003dinstance)"},{"line_number":6929,"context_line":"        bdm.update(values)"},{"line_number":6930,"context_line":"        bdm.save()"},{"line_number":6931,"context_line":""},{"line_number":6932,"context_line":"        compute_utils.notify_about_volume_swap("},{"line_number":6933,"context_line":"            context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_46fb6e47","line":6930,"range":{"start_line":6929,"start_character":8,"end_line":6930,"end_character":18},"updated":"2019-12-02 13:02:52.000000000","message":"why not update existing BDM value?","commit_id":"0f7ec388e0931a38d12bf81d16a0e94b128a2172"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c96ea96add08710f42a660f7260a2b2d8c7e5a36","unresolved":false,"context_lines":[{"line_number":6926,"context_line":"                  \"%(updates)s\", {\u0027volume_id\u0027: bdm.volume_id,"},{"line_number":6927,"context_line":"                                  \u0027updates\u0027: values},"},{"line_number":6928,"context_line":"                  instance\u003dinstance)"},{"line_number":6929,"context_line":"        bdm.update(values)"},{"line_number":6930,"context_line":"        bdm.save()"},{"line_number":6931,"context_line":""},{"line_number":6932,"context_line":"        compute_utils.notify_about_volume_swap("},{"line_number":6933,"context_line":"            context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_f7b543ee","line":6930,"range":{"start_line":6929,"start_character":8,"end_line":6930,"end_character":18},"in_reply_to":"3fa7e38b_12ae0281","updated":"2019-12-04 02:45:45.000000000","message":"Emm...I think I know what do you point, the old volume and the new volume has the same BDM, so I don\u0027t need to get the new volume BDM in line6917, I will update it.","commit_id":"0f7ec388e0931a38d12bf81d16a0e94b128a2172"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"ce241b2e35105f6a40654f8cc2efe44e3c533312","unresolved":false,"context_lines":[{"line_number":6926,"context_line":"                  \"%(updates)s\", {\u0027volume_id\u0027: bdm.volume_id,"},{"line_number":6927,"context_line":"                                  \u0027updates\u0027: values},"},{"line_number":6928,"context_line":"                  instance\u003dinstance)"},{"line_number":6929,"context_line":"        bdm.update(values)"},{"line_number":6930,"context_line":"        bdm.save()"},{"line_number":6931,"context_line":""},{"line_number":6932,"context_line":"        compute_utils.notify_about_volume_swap("},{"line_number":6933,"context_line":"            context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_e5759034","line":6930,"range":{"start_line":6929,"start_character":8,"end_line":6930,"end_character":18},"in_reply_to":"3fa7e38b_12ae0281","updated":"2019-12-04 00:57:27.000000000","message":"No, we just get the instance\u0027s BDM by the new volume_id https://review.opendev.org/#/c/693828/5/nova/compute/manager.py@6918, then update the \"delete_on_termination\" of the new volume.","commit_id":"0f7ec388e0931a38d12bf81d16a0e94b128a2172"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"859404a9a770108c56191fa30cfc3fcc69e043b6","unresolved":false,"context_lines":[{"line_number":6926,"context_line":"                  \"%(updates)s\", {\u0027volume_id\u0027: bdm.volume_id,"},{"line_number":6927,"context_line":"                                  \u0027updates\u0027: values},"},{"line_number":6928,"context_line":"                  instance\u003dinstance)"},{"line_number":6929,"context_line":"        bdm.update(values)"},{"line_number":6930,"context_line":"        bdm.save()"},{"line_number":6931,"context_line":""},{"line_number":6932,"context_line":"        compute_utils.notify_about_volume_swap("},{"line_number":6933,"context_line":"            context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_642d38ad","line":6930,"range":{"start_line":6929,"start_character":8,"end_line":6930,"end_character":18},"in_reply_to":"3fa7e38b_46fb6e47","updated":"2019-12-03 00:49:41.000000000","message":"That is the old volume\u0027s BDM, that I was updated this in update attachment volume API. Here https://review.opendev.org/#/c/693828/5/nova/api/openstack/compute/volumes.py@424~427","commit_id":"0f7ec388e0931a38d12bf81d16a0e94b128a2172"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"52f57a56e8b0005f35b5a3c8a334a12e762f0885","unresolved":false,"context_lines":[{"line_number":6926,"context_line":"                  \"%(updates)s\", {\u0027volume_id\u0027: bdm.volume_id,"},{"line_number":6927,"context_line":"                                  \u0027updates\u0027: values},"},{"line_number":6928,"context_line":"                  instance\u003dinstance)"},{"line_number":6929,"context_line":"        bdm.update(values)"},{"line_number":6930,"context_line":"        bdm.save()"},{"line_number":6931,"context_line":""},{"line_number":6932,"context_line":"        compute_utils.notify_about_volume_swap("},{"line_number":6933,"context_line":"            context, instance, self.host,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_12ae0281","line":6930,"range":{"start_line":6929,"start_character":8,"end_line":6930,"end_character":18},"in_reply_to":"3fa7e38b_642d38ad","updated":"2019-12-03 05:52:42.000000000","message":"We create an new BDM for the new volume?","commit_id":"0f7ec388e0931a38d12bf81d16a0e94b128a2172"}],"nova/tests/functional/api_sample_tests/test_volumes.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":302,"context_line":"    def test_volume_attachment_update(self):"},{"line_number":303,"context_line":"        subs \u003d self.test_attach_volume_to_server()"},{"line_number":304,"context_line":"        attached_volume_id \u003d subs[\u0027volume_id\u0027]"},{"line_number":305,"context_line":"        subs[\u0027delete_on_termination\u0027] \u003d \u0027False\u0027"},{"line_number":306,"context_line":"        response \u003d self._do_put(\u0027servers/%s/os-volume_attachments/%s\u0027"},{"line_number":307,"context_line":"                                % (self.server_id, attached_volume_id),"},{"line_number":308,"context_line":"                                \u0027update-volume-attachment-delete-flag-req\u0027,"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_47ef5e1d","line":305,"updated":"2020-03-31 11:45:28.000000000","message":"I tried to change this and the test still passes. I think the update-volume-attachment-delete-flag-req.json.tpl does not have the delete_on_termination as something you need to substitute. So I think this subs can be deleted or change the .tpl file to make delete_on_termination substitutable.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6e0acbf9afeaa1323db8fcee1017690aed0b0a65","unresolved":false,"context_lines":[{"line_number":1524,"context_line":"        # will fail to find the updated BDM after a swap for the remaining"},{"line_number":1525,"context_line":"        # properties to be updated (if applicable). Since that is a larger"},{"line_number":1526,"context_line":"        # change to the base fixture, just call swap_volume at 2.83 to"},{"line_number":1527,"context_line":"        # avoid a big refactor blocking 2.85."},{"line_number":1528,"context_line":"        orig_ver \u003d self.api.microversion"},{"line_number":1529,"context_line":"        try:"},{"line_number":1530,"context_line":"            self.api.microversion \u003d \u00272.83\u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"df33271e_2205e814","line":1527,"updated":"2020-03-31 11:45:28.000000000","message":"The fact that it only works for 2.83 shows an error in the change. See my comment in the api code.\n\nThe sample tests run with a normal compute manager just the virt driver and cinder interface is faked.","commit_id":"4c5cf0205564594973e1ea9384f2ebfb76f19bfd"}],"nova/tests/unit/api/openstack/compute/test_volumes.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d19377d7e12bcd445d53d5aa45d6a3a3acbe1c3a","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"class UpdateVolumeAttachTests(VolumeAttachTestsV279):"},{"line_number":1154,"context_line":"    microversion \u003d \u00272.83\u0027"},{"line_number":1155,"context_line":""},{"line_number":1156,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":1157,"context_line":"    def test_update_volume(self, mock_bdm_save):"},{"line_number":1158,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"},{"line_number":1159,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027delete_on_termination\u0027: True}}"},{"line_number":1160,"context_line":"        self.attachments.update_volume(self.req, FAKE_UUID,"},{"line_number":1161,"context_line":"                                       FAKE_UUID_A, body\u003dbody)"},{"line_number":1162,"context_line":"        mock_bdm_save.assert_called_once()"},{"line_number":1163,"context_line":""},{"line_number":1164,"context_line":"    def test_update_volume_without_delete_on_termionation(self):"},{"line_number":1165,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"1fa4df85_9c9d22bd","line":1162,"range":{"start_line":1156,"start_character":0,"end_line":1162,"end_character":42},"updated":"2020-03-17 12:42:22.000000000","message":"Could you mock out get_by_volume_and_instance, return a mock\u0027d bdm and assert that delete_on_termination is updated and .save() is called?","commit_id":"d90d0106bc2bcaf9830662ff6a46cd8cd177b27c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"ec3098002762daf9f285a3c0f93d28b091202992","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"class UpdateVolumeAttachTests(VolumeAttachTestsV279):"},{"line_number":1154,"context_line":"    microversion \u003d \u00272.83\u0027"},{"line_number":1155,"context_line":""},{"line_number":1156,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":1157,"context_line":"    def test_update_volume(self, mock_bdm_save):"},{"line_number":1158,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"},{"line_number":1159,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027delete_on_termination\u0027: True}}"},{"line_number":1160,"context_line":"        self.attachments.update_volume(self.req, FAKE_UUID,"},{"line_number":1161,"context_line":"                                       FAKE_UUID_A, body\u003dbody)"},{"line_number":1162,"context_line":"        mock_bdm_save.assert_called_once()"},{"line_number":1163,"context_line":""},{"line_number":1164,"context_line":"    def test_update_volume_without_delete_on_termionation(self):"},{"line_number":1165,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"}],"source_content_type":"text/x-python","patch_set":18,"id":"1fa4df85_3fb74089","line":1162,"range":{"start_line":1156,"start_character":0,"end_line":1162,"end_character":42},"in_reply_to":"1fa4df85_9c9d22bd","updated":"2020-03-17 14:28:29.000000000","message":"Done","commit_id":"d90d0106bc2bcaf9830662ff6a46cd8cd177b27c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a5b358b6e5958f56e700687c62a4f9672561bb7c","unresolved":false,"context_lines":[{"line_number":1157,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1158,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":1159,"context_line":"    def test_update_volume(self, mock_bdm_save, mock_get_vol_and_inst):"},{"line_number":1160,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1161,"context_line":"            self.context,"},{"line_number":1162,"context_line":"            id\u003d1,"},{"line_number":1163,"context_line":"            instance_uuid\u003dFAKE_UUID,"},{"line_number":1164,"context_line":"            volume_id\u003dFAKE_UUID_A,"},{"line_number":1165,"context_line":"            source_type\u003d\u0027volume\u0027,"},{"line_number":1166,"context_line":"            destination_type\u003d\u0027volume\u0027,"},{"line_number":1167,"context_line":"            delete_on_termination\u003dFalse,"},{"line_number":1168,"context_line":"            connection_info\u003dNone,"},{"line_number":1169,"context_line":"            tag\u003d\u0027fake-tag\u0027,"},{"line_number":1170,"context_line":"            device_name\u003d\u0027/dev/fake0\u0027,"},{"line_number":1171,"context_line":"            attachment_id\u003duuids.attachment_id)"},{"line_number":1172,"context_line":"        mock_get_vol_and_inst.return_value \u003d vol_bdm"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"1fa4df85_55869d15","line":1171,"range":{"start_line":1160,"start_character":0,"end_line":1171,"end_character":46},"updated":"2020-03-17 14:40:10.000000000","message":"FWIW I was thinking more of something like:\n    \n    mock_bdm \u003d mock.MagicMock(spec\u003dobjects.BlockDeviceMapping)\n    mock_bdm[\u0027delete_on_terminiation\u0027] \u003d False\n    mock_get_vol_inst.return_value \u003d mock_bdm\n\n    [..]\n\n    self.assertTrue(mock_bdm[\u0027delete_on_termination\u0027])\n    mock_bdm.save.assert_called_once()\n\nThis also works so it\u0027s just a nit for me.","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"95a9e58b5480aa98c06ff254a770157ac162ca15","unresolved":false,"context_lines":[{"line_number":1157,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1158,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":1159,"context_line":"    def test_update_volume(self, mock_bdm_save, mock_get_vol_and_inst):"},{"line_number":1160,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1161,"context_line":"            self.context,"},{"line_number":1162,"context_line":"            id\u003d1,"},{"line_number":1163,"context_line":"            instance_uuid\u003dFAKE_UUID,"},{"line_number":1164,"context_line":"            volume_id\u003dFAKE_UUID_A,"},{"line_number":1165,"context_line":"            source_type\u003d\u0027volume\u0027,"},{"line_number":1166,"context_line":"            destination_type\u003d\u0027volume\u0027,"},{"line_number":1167,"context_line":"            delete_on_termination\u003dFalse,"},{"line_number":1168,"context_line":"            connection_info\u003dNone,"},{"line_number":1169,"context_line":"            tag\u003d\u0027fake-tag\u0027,"},{"line_number":1170,"context_line":"            device_name\u003d\u0027/dev/fake0\u0027,"},{"line_number":1171,"context_line":"            attachment_id\u003duuids.attachment_id)"},{"line_number":1172,"context_line":"        mock_get_vol_and_inst.return_value \u003d vol_bdm"},{"line_number":1173,"context_line":""},{"line_number":1174,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"1fa4df85_306f6fc6","line":1171,"range":{"start_line":1160,"start_character":0,"end_line":1171,"end_character":46},"in_reply_to":"1fa4df85_55869d15","updated":"2020-03-17 15:04:47.000000000","message":"It looks very fresh, if it needs to be updated again, it will be replaced in the next patch.","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7cb5539f81734b2d872e02b402e6ebabfe4bcdba","unresolved":false,"context_lines":[{"line_number":1178,"context_line":"        mock_bdm_save.assert_called_once()"},{"line_number":1179,"context_line":"        self.assertTrue(vol_bdm[\u0027delete_on_termination\u0027])"},{"line_number":1180,"context_line":""},{"line_number":1181,"context_line":"    def test_update_volume_without_delete_on_termionation(self):"},{"line_number":1182,"context_line":"        self.req.method \u003d \u0027PATCH\u0027"},{"line_number":1183,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027foo\u0027: \u0027foo_value\u0027}}"},{"line_number":1184,"context_line":"        self.assertRaises(self.validation_error,"}],"source_content_type":"text/x-python","patch_set":19,"id":"df33271e_a35015d6","line":1181,"range":{"start_line":1181,"start_character":45,"end_line":1181,"end_character":57},"updated":"2020-03-27 14:34:09.000000000","message":"termination","commit_id":"cde83a501f341cba316cd24c829222323d40010a"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"d9d70820949c93afe32ad37ee9f9135feb8bd87f","unresolved":false,"context_lines":[{"line_number":1157,"context_line":"        # NOTE(danms): Override this from parent because now device"},{"line_number":1158,"context_line":"        # is checked for unchanged-ness."},{"line_number":1159,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_A,"},{"line_number":1160,"context_line":"                                    \u0027device\u0027: \u0027/dev/fake0\u0027,"},{"line_number":1161,"context_line":"                                     \u0027notathing\u0027: \u0027foo\u0027}}"},{"line_number":1162,"context_line":""},{"line_number":1163,"context_line":"        self.assertRaises(self.validation_error,"}],"source_content_type":"text/x-python","patch_set":20,"id":"df33271e_2f3a1e33","line":1160,"updated":"2020-03-27 23:27:01.000000000","message":"nit: Indentation","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {"},{"line_number":1208,"context_line":"            \u0027volumeId\u0027: FAKE_UUID_A,"},{"line_number":1209,"context_line":"            \u0027tag\u0027: \u0027fake-tag\u0027,"},{"line_number":1210,"context_line":"            \u0027delete_on_termination\u0027: True,"},{"line_number":1211,"context_line":"        }}"},{"line_number":1212,"context_line":"        self.attachments.update(self.req, FAKE_UUID,"},{"line_number":1213,"context_line":"                                FAKE_UUID_A, body\u003dbody)"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_77d97df1","line":1210,"range":{"start_line":1210,"start_character":41,"end_line":1210,"end_character":42},"updated":"2020-03-30 23:28:35.000000000","message":"can we add \u0027device\u0027 also here to check it is valid in request and ignore for same value.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {"},{"line_number":1208,"context_line":"            \u0027volumeId\u0027: FAKE_UUID_A,"},{"line_number":1209,"context_line":"            \u0027tag\u0027: \u0027fake-tag\u0027,"},{"line_number":1210,"context_line":"            \u0027delete_on_termination\u0027: True,"},{"line_number":1211,"context_line":"        }}"},{"line_number":1212,"context_line":"        self.attachments.update(self.req, FAKE_UUID,"},{"line_number":1213,"context_line":"                                FAKE_UUID_A, body\u003dbody)"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_3369c4f8","line":1210,"range":{"start_line":1210,"start_character":41,"end_line":1210,"end_character":42},"in_reply_to":"df33271e_77d97df1","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"                                FAKE_UUID_A, body\u003dbody)"},{"line_number":1214,"context_line":"        mock_bdm_save.assert_called_once()"},{"line_number":1215,"context_line":"        self.assertTrue(vol_bdm[\u0027delete_on_termination\u0027])"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    @mock.patch.object(compute_api.API, \u0027swap_volume\u0027)"},{"line_number":1218,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1219,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_570c3993","line":1216,"range":{"start_line":1216,"start_character":0,"end_line":1216,"end_character":0},"updated":"2020-03-30 23:28:35.000000000","message":"in this case where old and new volume id is same, can you check compute.api.swap_volume is not called","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":1213,"context_line":"                                FAKE_UUID_A, body\u003dbody)"},{"line_number":1214,"context_line":"        mock_bdm_save.assert_called_once()"},{"line_number":1215,"context_line":"        self.assertTrue(vol_bdm[\u0027delete_on_termination\u0027])"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    @mock.patch.object(compute_api.API, \u0027swap_volume\u0027)"},{"line_number":1218,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1219,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_b3af9428","line":1216,"range":{"start_line":1216,"start_character":0,"end_line":1216,"end_character":0},"in_reply_to":"df33271e_570c3993","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fe6439ca582546e57ba1cc4e7d8c318f0897dd29","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"                          self.attachments.update,"},{"line_number":1313,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1314,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":""},{"line_number":1317,"context_line":"class SwapVolumeMultiattachTestCase(test.NoDBTestCase):"},{"line_number":1318,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_b7df85d4","line":1315,"range":{"start_line":1315,"start_character":0,"end_line":1315,"end_character":0},"updated":"2020-03-30 23:28:35.000000000","message":"please add tests to test the immediate previous version for this change. \n\n- request with 2.84 version and check \u0027delete_on_termination\u0027, \u0027tag\u0027, \u0027device\u0027 are Valdiation error.\n\nalso to check swap only operation but all the above fields are present with same value and save not called. separate test or adjusted in one of above test.","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":1312,"context_line":"                          self.attachments.update,"},{"line_number":1313,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1314,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":""},{"line_number":1317,"context_line":"class SwapVolumeMultiattachTestCase(test.NoDBTestCase):"},{"line_number":1318,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"df33271e_214deac3","line":1315,"range":{"start_line":1315,"start_character":0,"end_line":1315,"end_character":0},"in_reply_to":"df33271e_b7df85d4","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"d507f854beaf01498379a6634133bf7ccadb73c9","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                          self.attachments.update,"},{"line_number":1336,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1337,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1340,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1341,"context_line":"    def test_update_volume_with_changed_serverId(self,"},{"line_number":1342,"context_line":"                                                 mock_get_vol_and_inst):"},{"line_number":1343,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1344,"context_line":"            self.context,"},{"line_number":1345,"context_line":"            id\u003d1,"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_d77136aa","line":1342,"range":{"start_line":1338,"start_character":0,"end_line":1342,"end_character":66},"updated":"2020-04-01 16:48:16.000000000","message":"can we add test of previous microverison for all these 3 new field also.\n\nwe had regressions in past of putting new field in older version so having tests really help here.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"2176c2d6fc8d879bccd5d629a91760a7e144b1f1","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                          self.attachments.update,"},{"line_number":1336,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1337,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1340,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1341,"context_line":"    def test_update_volume_with_changed_serverId(self,"},{"line_number":1342,"context_line":"                                                 mock_get_vol_and_inst):"},{"line_number":1343,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1344,"context_line":"            self.context,"},{"line_number":1345,"context_line":"            id\u003d1,"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_c74cc043","line":1342,"range":{"start_line":1338,"start_character":0,"end_line":1342,"end_character":66},"in_reply_to":"df33271e_27c4953f","updated":"2020-04-02 23:16:43.000000000","message":"ok, let\u0027s add in followup.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"e803fb758a1c9cbd5f0d8c3d54eb7fceb55e69cf","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                          self.attachments.update,"},{"line_number":1336,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1337,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1340,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1341,"context_line":"    def test_update_volume_with_changed_serverId(self,"},{"line_number":1342,"context_line":"                                                 mock_get_vol_and_inst):"},{"line_number":1343,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1344,"context_line":"            self.context,"},{"line_number":1345,"context_line":"            id\u003d1,"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_27c4953f","line":1342,"range":{"start_line":1338,"start_character":0,"end_line":1342,"end_character":66},"in_reply_to":"df33271e_a5670b4d","updated":"2020-04-02 15:41:59.000000000","message":"ok as you might need to respin. can you add in this itself.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"9c0f041a6b62a61c78613a0fc132f7db94f5dc1e","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                          self.attachments.update,"},{"line_number":1336,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1337,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1340,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1341,"context_line":"    def test_update_volume_with_changed_serverId(self,"},{"line_number":1342,"context_line":"                                                 mock_get_vol_and_inst):"},{"line_number":1343,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1344,"context_line":"            self.context,"},{"line_number":1345,"context_line":"            id\u003d1,"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_4be78913","line":1342,"range":{"start_line":1338,"start_character":0,"end_line":1342,"end_character":66},"in_reply_to":"df33271e_c74cc043","updated":"2020-04-09 01:30:08.000000000","message":"The follow up patch https://review.opendev.org/#/c/718589/","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"432e10a321db5af473c14ff2129f11d9d57e67a6","unresolved":false,"context_lines":[{"line_number":1335,"context_line":"                          self.attachments.update,"},{"line_number":1336,"context_line":"                          self.req, FAKE_UUID,"},{"line_number":1337,"context_line":"                          FAKE_UUID_A, body\u003dbody)"},{"line_number":1338,"context_line":""},{"line_number":1339,"context_line":"    @mock.patch.object(objects.BlockDeviceMapping,"},{"line_number":1340,"context_line":"                       \u0027get_by_volume_and_instance\u0027)"},{"line_number":1341,"context_line":"    def test_update_volume_with_changed_serverId(self,"},{"line_number":1342,"context_line":"                                                 mock_get_vol_and_inst):"},{"line_number":1343,"context_line":"        vol_bdm \u003d objects.BlockDeviceMapping("},{"line_number":1344,"context_line":"            self.context,"},{"line_number":1345,"context_line":"            id\u003d1,"}],"source_content_type":"text/x-python","patch_set":25,"id":"df33271e_a5670b4d","line":1342,"range":{"start_line":1338,"start_character":0,"end_line":1342,"end_character":66},"in_reply_to":"df33271e_d77136aa","updated":"2020-04-02 08:01:08.000000000","message":"Yeah, that will be completed by follow up.","commit_id":"092aef1b516cb1e56a59198f4fa1d41a09bfd07c"}],"releasenotes/notes/bp-destroy-instance-with-datavolume-4c71b12e005832b0.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10b06c72638e4bb2bb9d096a6f0fe4be42b800b5","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.84 add new API"},{"line_number":5,"context_line":"    ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}`` which"},{"line_number":6,"context_line":"    support for specifying ``delete_on_termination`` field in the request"},{"line_number":7,"context_line":"    body to re-config the attached volume whether to delete when the instance"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"df33271e_2c36b153","line":4,"updated":"2020-03-27 17:46:02.000000000","message":"Shouldn\u0027t this also be updated to mention the changes to the PUT API?\n\nAnd if you\u0027re going to allow this with PUT (which is what I think the agreement in the spec was earlier, before I left anyway), why even add PATCH? This would be the only PATCH API in nova but if you can also do the thing with the existing PUT API why add another route? It\u0027s just more maintenance.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4729642674ad6f0bb37ca67ff289431ff3e82541","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.84 add new API"},{"line_number":5,"context_line":"    ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}`` which"},{"line_number":6,"context_line":"    support for specifying ``delete_on_termination`` field in the request"},{"line_number":7,"context_line":"    body to re-config the attached volume whether to delete when the instance"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"df33271e_ecc7a911","line":4,"in_reply_to":"df33271e_2c36b153","updated":"2020-03-27 17:49:15.000000000","message":"This is just not updated. We\u0027re not adding PATCH, as you can see I removed all that stuff.","commit_id":"104006f57794d332179266d8048dfdf8a6803c6f"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c316964ae22ceeabc497232305595fe947bfede4","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85 add new API"},{"line_number":5,"context_line":"    ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}`` which"},{"line_number":6,"context_line":"    support for specifying ``delete_on_termination`` field in the request"},{"line_number":7,"context_line":"    body to re-config the attached volume whether to delete when the instance"},{"line_number":8,"context_line":"    is deleted."}],"source_content_type":"text/x-yaml","patch_set":21,"id":"df33271e_0d08de8f","line":5,"range":{"start_line":5,"start_character":6,"end_line":5,"end_character":11},"updated":"2020-03-31 01:43:47.000000000","message":"s/PATCH/PUT","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"738300f11a59b438666f639f1a525fc5971f709f","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85 add new API"},{"line_number":5,"context_line":"    ``PATCH /servers/{server_id}/os-volume_attachments/{volume_id}`` which"},{"line_number":6,"context_line":"    support for specifying ``delete_on_termination`` field in the request"},{"line_number":7,"context_line":"    body to re-config the attached volume whether to delete when the instance"},{"line_number":8,"context_line":"    is deleted."}],"source_content_type":"text/x-yaml","patch_set":21,"id":"df33271e_21fb2a47","line":5,"range":{"start_line":5,"start_character":6,"end_line":5,"end_character":11},"in_reply_to":"df33271e_0d08de8f","updated":"2020-03-31 08:34:11.000000000","message":"Done","commit_id":"7a0e5585c499e7b92c41b1da1fe625d9d2710df3"}]}