)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ab30d7873fdc3a0408e0a1ade7e0ef1dec1a7522","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4e92b1ef_9135445e","updated":"2022-06-14 22:06:35.000000000","message":"This change makes sense and contains good test coverage for all the cases, release note rendering looks good. LGTM","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1246219ef450a624ba4efece4dd1d4ef94cfd872","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"fdf8659c_bf7d79da","updated":"2022-07-08 10:55:58.000000000","message":"This would make a good backport","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"}],"nova/volume/cinder.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"be178e9cb9d72dc29270d47ccab809a4eb873208","unresolved":true,"context_lines":[{"line_number":884,"context_line":"                           \u0027msg\u0027: str(ex),"},{"line_number":885,"context_line":"                           \u0027code\u0027: getattr(ex, \u0027code\u0027, None)})"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"    @translate_attachment_exception"},{"line_number":888,"context_line":"    @retrying.retry(stop_max_attempt_number\u003d5,"},{"line_number":889,"context_line":"                    retry_on_exception\u003dlambda e:"},{"line_number":890,"context_line":"                    (isinstance(e, cinder_exception.ClientException) and"}],"source_content_type":"text/x-python","patch_set":6,"id":"4fc4c244_dee62037","line":887,"updated":"2022-06-14 22:16:38.000000000","message":"(later) I just realized this is how we mean to raise nova exceptions instead of cinderclient ones ... but currently translate_attachment_exception only translates cinder_exception.NotFound.","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ab30d7873fdc3a0408e0a1ade7e0ef1dec1a7522","unresolved":true,"context_lines":[{"line_number":897,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":898,"context_line":"            if ex.code \u003d\u003d 404:"},{"line_number":899,"context_line":"                LOG.warning(\u0027Attachment %(id)s does not exist. Ignoring.\u0027,"},{"line_number":900,"context_line":"                            {\u0027id\u0027: attachment_id})"},{"line_number":901,"context_line":"            else:"},{"line_number":902,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":903,"context_line":"                    LOG.error(\u0027Delete attachment failed for attachment \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"9645049f_c71e782c","line":900,"updated":"2022-06-14 22:06:35.000000000","message":"I note that while this is a change in behavior, we generally use the pattern of not treating NotFound as a failure during a detach or delete operation. I think this is an improvement that makes attachment_delete more consistent with similar delete $thing code in the codebase and it also makes this call idempotent.","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"be178e9cb9d72dc29270d47ccab809a4eb873208","unresolved":true,"context_lines":[{"line_number":897,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":898,"context_line":"            if ex.code \u003d\u003d 404:"},{"line_number":899,"context_line":"                LOG.warning(\u0027Attachment %(id)s does not exist. Ignoring.\u0027,"},{"line_number":900,"context_line":"                            {\u0027id\u0027: attachment_id})"},{"line_number":901,"context_line":"            else:"},{"line_number":902,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":903,"context_line":"                    LOG.error(\u0027Delete attachment failed for attachment \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"da71dc8a_7b2410e5","line":900,"in_reply_to":"9645049f_c71e782c","updated":"2022-06-14 22:16:38.000000000","message":"I also note that logging at level warning is consistent with how 409 (already activated) is treated for port binding activation in nova/network/neutron.py:\nhttps://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/network/neutron.py#L3168-L3172","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ab30d7873fdc3a0408e0a1ade7e0ef1dec1a7522","unresolved":true,"context_lines":[{"line_number":899,"context_line":"                LOG.warning(\u0027Attachment %(id)s does not exist. Ignoring.\u0027,"},{"line_number":900,"context_line":"                            {\u0027id\u0027: attachment_id})"},{"line_number":901,"context_line":"            else:"},{"line_number":902,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":903,"context_line":"                    LOG.error(\u0027Delete attachment failed for attachment \u0027"},{"line_number":904,"context_line":"                              \u0027%(id)s. Error: %(msg)s Code: %(code)s\u0027,"},{"line_number":905,"context_line":"                              {\u0027id\u0027: attachment_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3be1908a_a8b7c11a","line":902,"updated":"2022-06-14 22:06:35.000000000","message":"This should really be raising a nova exception IMHO and not letting a cinderclient exception propagate up but that is out of scope and unrelated to this fix.","commit_id":"8f4b740ca5292556f8e953a30f2a11ed4fbc2945"}]}
