)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"59443c1df7576aff495c094b6fa852aa6c767056","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[SVF] Fix issue to get volume relationship details"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"[Spectrum Virtualize Family] Fix the issue while fetching"},{"line_number":10,"context_line":"relationship details of a volume with replication enabled."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1926286"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"567e4351_cd3085c7","line":9,"range":{"start_line":9,"start_character":29,"end_line":9,"end_character":42},"updated":"2021-08-12 17:16:18.000000000","message":"It\u0027s OK to use a vague phrase like this in the subject line, but here in the body you need to elaborate.\n- What is the issue you\u0027re fixing?\n- What is the nature of the fix?","commit_id":"c7f39ad162770dedf11f946e7c23a2c97c1165bc"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"6ec351cc8dc6781c4f4b515961911f6b24970880","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[SVF] Fix issue to get volume relationship details"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"[Spectrum Virtualize Family] Fix the issue while fetching"},{"line_number":10,"context_line":"relationship details of a volume with replication enabled."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1926286"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"41dcbece_fb4e76ef","line":9,"range":{"start_line":9,"start_character":29,"end_line":9,"end_character":42},"in_reply_to":"567e4351_cd3085c7","updated":"2021-08-13 15:47:59.000000000","message":"Done","commit_id":"c7f39ad162770dedf11f946e7c23a2c97c1165bc"}],"cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"2c7911640b73f76d9b42297acb5e42f387e5eb4d","unresolved":true,"context_lines":[{"line_number":3355,"context_line":"                      \u0027relationship details for the volume %(volume)s. \u0027"},{"line_number":3356,"context_line":"                      \u0027Exception: %(err)s.\u0027,"},{"line_number":3357,"context_line":"                      {\u0027volume\u0027: volume[\u0027name\u0027], \u0027err\u0027: ex})"},{"line_number":3358,"context_line":"            model_update[\u0027status\u0027] \u003d fields.VolumeStatus.ERROR"},{"line_number":3359,"context_line":"            return model_update"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"        rep_properties \u003d {"}],"source_content_type":"text/x-python","patch_set":4,"id":"5e9c6f44_357b74a3","line":3358,"updated":"2021-07-29 15:50:20.000000000","message":"I\u0027m not sure how this fully addresses the issue. The function is invoked during multiple operations, and handling the exception this way essentially prevents the higher layers from knowing something went wrong. Consider the volume creation flow: it will have no idea there\u0027s a problem, and will proceed to completion, and may actually overwrite the volume status again.\n\nI think you need to consider handling this exception at a higher layer.\n\nI don\u0027t know much about cinder\u0027s replication feature, but there\u0027s a replication_status field that would seem to be better suited than setting the volume status.","commit_id":"a08328364699afd21187e958baf7df69f5e8c348"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4c8c3cc2e0f9cfd9c309a754762457acdaaeba9b","unresolved":false,"context_lines":[{"line_number":3355,"context_line":"                      \u0027relationship details for the volume %(volume)s. \u0027"},{"line_number":3356,"context_line":"                      \u0027Exception: %(err)s.\u0027,"},{"line_number":3357,"context_line":"                      {\u0027volume\u0027: volume[\u0027name\u0027], \u0027err\u0027: ex})"},{"line_number":3358,"context_line":"            model_update[\u0027status\u0027] \u003d fields.VolumeStatus.ERROR"},{"line_number":3359,"context_line":"            return model_update"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"        rep_properties \u003d {"}],"source_content_type":"text/x-python","patch_set":4,"id":"fbc42d2c_4e9eac77","line":3358,"in_reply_to":"40bfbbd3_fb8a0a8f","updated":"2021-08-06 12:53:28.000000000","message":"Your response doesn\u0027t address the points I raised. I know what the code is doing (logging the failure and setting the volume status). That is not in question. My point is the upper layers of code have no knowledge that something failed, and operations like creating a volume will proceed as if everything is fine. That doesn\u0027t seem like a good approach.\n\nYou state, \"this issue is observed mainly during bulk operations.\" Have you considered adding retry logic to the part of the code? Other drivers do that. The goal is to prevent a soft failure (often a timeout due to something being temporarily overloaded) into a hard failure (volume in ERROR state).","commit_id":"a08328364699afd21187e958baf7df69f5e8c348"},{"author":{"_account_id":32266,"name":"Venkata krishna Thumu","display_name":"VenkataKrishna","email":"venkata.krishna.reddy@ibm.com","username":"venkatakrishnathumu","status":"Active"},"change_message_id":"c25965e2cc19e70923fb453bf37f98a59d52396e","unresolved":false,"context_lines":[{"line_number":3355,"context_line":"                      \u0027relationship details for the volume %(volume)s. \u0027"},{"line_number":3356,"context_line":"                      \u0027Exception: %(err)s.\u0027,"},{"line_number":3357,"context_line":"                      {\u0027volume\u0027: volume[\u0027name\u0027], \u0027err\u0027: ex})"},{"line_number":3358,"context_line":"            model_update[\u0027status\u0027] \u003d fields.VolumeStatus.ERROR"},{"line_number":3359,"context_line":"            return model_update"},{"line_number":3360,"context_line":""},{"line_number":3361,"context_line":"        rep_properties \u003d {"}],"source_content_type":"text/x-python","patch_set":4,"id":"40bfbbd3_fb8a0a8f","line":3358,"in_reply_to":"5e9c6f44_357b74a3","updated":"2021-08-02 12:01:42.000000000","message":"In the current exception handling,\n1. We display the error log saying that it failed to fetch relationship details.\n2. We update the metadata for volume \u0027status\u0027 as ERROR. With this user will know why and where the operation is failed. \n\nThis issue is observed mainly during bulk operations on volume, sometimes get_relationship_info command failed on the storage, so the volume metadata is not updated properly and we leave the volume is in an \u0027available\u0027 state. Users will never know that volume metadata is partially updated or not updated properly.\n\nHandling this exception for \u0027get_relationship_info\u0027 as it is the only backend call from the function \u0027_update_replication_properties\u0027 by forcefully updating the volume status as \u0027error\u0027.","commit_id":"a08328364699afd21187e958baf7df69f5e8c348"}]}