)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c6018cb27b12226c9390126b3a8eceeb09432a27","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8a17503a_0f909966","updated":"2025-02-06 22:16:41.000000000","message":"The -1 is to get your attention for something to check in cinder/volume/manager.py\n\nOtherwise, I think this is OK.  AFAIK, there isn\u0027t a contract for notification content, it\u0027s just expected to be a JSON blob, so it should be OK to add a new field.  And since you aren\u0027t changing any of the other fields, it shouldn\u0027t break any notification consumer.  Also, since you\u0027re passing CI, there aren\u0027t any tempest tests that are failing, which is more evidence that this is an OK change.","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"45031fb33e54b9ae26d456dcf4f406f41d2fd475","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"92e7f5a9_6f116cef","updated":"2025-02-05 05:55:36.000000000","message":"recheck transient tempest-slow-py3 failure","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2934968be04ca4178ee10ff75996083577df05e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c715980b_afae2bea","updated":"2025-02-20 00:26:17.000000000","message":"-1: See my comment in manager.py about not moving the place where the volume type is retrieved.","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"f34e4cfd65cb5d3e87471bd8cb1c9a7f1d8b4f52","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"dee40d43_beb8cfe8","updated":"2025-02-07 06:51:13.000000000","message":"recheck transient devstack-plugin-nfs-tempest-full timeout","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"9e6de5e57ac80a1de49f2c2e08e34ad9915e2ffa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4778e09d_2427cff8","updated":"2025-02-21 07:41:39.000000000","message":"recheck cinder-plugin-ceph-tempest intermittent failure","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"8cd0339e500db94422e6ff45192d07f180800308","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e696a9d4_11834344","updated":"2025-02-21 11:02:23.000000000","message":"recheck cinder-plugin-ceph-tempest intermittent failure","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"8e299fff724e4adbbfe4fc4cb883b3b355858dc0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"bd6aa153_481dd617","updated":"2025-02-20 10:55:56.000000000","message":"recheck cinder-plugin-ceph-tempest transient failure","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"f16e579feedb6cff8832fef8d24973967c4117e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"27b9411e_bc012a01","updated":"2025-02-21 21:31:10.000000000","message":"recheck devstack-plugin-nfs-tempest-full POST_FAILURE","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"282be8eb36524885c871202f79f64c243b91b9db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b1dbf808_fc435a3e","updated":"2025-02-20 17:57:19.000000000","message":"recheck devstack-plugin-nfs-tempest-full POST_FAILURE","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"39e8902eb3deecd4c5c15fc2e7ec4a03e82d1fbe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"aa3bca31_9cb50680","updated":"2025-02-20 22:32:09.000000000","message":"recheck devstack-plugin-nfs-tempest-full intermittent failure","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"51d20d1f154c54d53b6bc23c33331a0e6a0990e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9321b4c1_b3ebf1d9","updated":"2025-02-21 03:01:54.000000000","message":"recheck tempest-integrated-storage-ubuntu-jammy intermittent failure","commit_id":"35b07a13c2f1dc0407098caff944505e328e704c"}],"cinder/tests/unit/utils.py":[{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"3b736f5aa38e32268f63708254befd62707518d5","unresolved":true,"context_lines":[{"line_number":81,"context_line":"    # not provided a pre-existing volume type to use."},{"line_number":82,"context_line":"    # Set volume_type_id to a well-defined value,"},{"line_number":83,"context_line":"    # and create a volume type with that ID and a unique name"},{"line_number":84,"context_line":"    # (if it doesn\u0027t exist) so that references to it work."},{"line_number":85,"context_line":"    if not volume_type_id:"},{"line_number":86,"context_line":"        volume_type_id \u003d fake.VOLUME_TYPE6_ID"},{"line_number":87,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"5ffdc000_8484d0a4","line":84,"updated":"2025-02-06 22:13:43.000000000","message":"When a volume type is not passed in to a volume creation request, the default volume type is used.  We shouldn\u0027t be forcing it to a new type here.","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"7c65539261ffcfa9731f91b9319b5ac224921ce1","unresolved":true,"context_lines":[{"line_number":81,"context_line":"    # not provided a pre-existing volume type to use."},{"line_number":82,"context_line":"    # Set volume_type_id to a well-defined value,"},{"line_number":83,"context_line":"    # and create a volume type with that ID and a unique name"},{"line_number":84,"context_line":"    # (if it doesn\u0027t exist) so that references to it work."},{"line_number":85,"context_line":"    if not volume_type_id:"},{"line_number":86,"context_line":"        volume_type_id \u003d fake.VOLUME_TYPE6_ID"},{"line_number":87,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ee608846_00dce56d","line":84,"in_reply_to":"5ffdc000_8484d0a4","updated":"2025-02-06 22:23:01.000000000","message":"Pre-this change this function sets the volume type ID to `fake.VOLUME_TYPE2_ID` if no override is set (I\u0027m not sure if this is supposed to be the default volume type or not).\n\nThe issue is for most tests is the previously set default volume type, `fake.VOLUME_TYPE2_ID`, doesn\u0027t actually exist unless the test creates it.\n\nSince `fake.VOLUME_TYPE2_ID` is referenced in other tests, I made a new volume type ID, and made sure that a volume type with that ID always exists so the code runs correctly.\n\nDo you have any suggestion for restoring the previous behaviour in a way that volume type references will work correctly, without increasing the size of the diffs too much?","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"}],"cinder/volume/manager.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c6018cb27b12226c9390126b3a8eceeb09432a27","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id})"},{"line_number":3160,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3161,"context_line":"        LOG.info(\"Retype volume completed successfully.\","},{"line_number":3162,"context_line":"                 resource\u003dvolume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"43de6769_8a294f91","line":3159,"updated":"2025-02-06 22:16:41.000000000","message":"Please take a look at this.  I think the idea of passing the new_type_id here is that the volume object would be holding the old type id, in which case you\u0027ll have a name/id mismatch in your notification (because the extra_usage_info is processed after the volume info has been loaded).  However, this notification only happens when the retype is successful, which should mean that the volume object has been updated and volume.save() called before we hit this line, which would make passing this extra_usage_info unnecessary.  But it is here, and it post-dates the transition to oslo versioned objects, so maybe it really is necessary?  See what you think, because if it is necessary, then we need to add the volume_type_name here too.  (But if it\u0027s not necessary, we should remove it in a follow up patch, so our successors don\u0027t waste brain cycles trying to figure this out again.)","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"6ed38e2f0cbd401d2b361d4b4494986ba4c9612d","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id})"},{"line_number":3160,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3161,"context_line":"        LOG.info(\"Retype volume completed successfully.\","},{"line_number":3162,"context_line":"                 resource\u003dvolume)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f790b06d_2f1ae5f3","line":3159,"in_reply_to":"43de6769_8a294f91","updated":"2025-02-07 00:14:17.000000000","message":"Thank you for bringing this to my attention. I believe that yes, this extra usage info should be updated as well, so I have updated the patch to do so (reusing the already fetched `new_type` object to avoid adding more queries than necessary). I have also updated `_usage_from_volume` to not perform the volume type query if `volume_type_name` is overridden by the caller.","commit_id":"0babccb0a0ddc9d89956ab0ae5336509399c7322"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2934968be04ca4178ee10ff75996083577df05e3","unresolved":true,"context_lines":[{"line_number":3066,"context_line":""},{"line_number":3067,"context_line":"        # We assume that those that support pools do this internally"},{"line_number":3068,"context_line":"        # so we strip off the pools designation"},{"line_number":3069,"context_line":""},{"line_number":3070,"context_line":"        new_type \u003d objects.VolumeType.get_by_id(context.elevated(),"},{"line_number":3071,"context_line":"                                                new_type_id)"},{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"        if (not retyped and"},{"line_number":3074,"context_line":"                not diff.get(\u0027encryption\u0027) and"}],"source_content_type":"text/x-python","patch_set":5,"id":"7932914a_51681d45","line":3071,"range":{"start_line":3069,"start_character":0,"end_line":3071,"end_character":60},"updated":"2025-02-20 00:26:17.000000000","message":"I don\u0027t think you should move this outside of the try block.  If the volume type has disappeared, we need to make sure the quota reservations are rolled back.  That\u0027s accomplished in the original code by catching the exception and setting retyped \u003d False, and then when we eventually make the call at line 3131 (note that we pass the type ID, not the type object), that will raise, and then in the catch, _retype_error() is called, which will handle the quotas.","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"e38fade19de3da85979b821fbfdf3adf05999e04","unresolved":true,"context_lines":[{"line_number":3066,"context_line":""},{"line_number":3067,"context_line":"        # We assume that those that support pools do this internally"},{"line_number":3068,"context_line":"        # so we strip off the pools designation"},{"line_number":3069,"context_line":""},{"line_number":3070,"context_line":"        new_type \u003d objects.VolumeType.get_by_id(context.elevated(),"},{"line_number":3071,"context_line":"                                                new_type_id)"},{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"        if (not retyped and"},{"line_number":3074,"context_line":"                not diff.get(\u0027encryption\u0027) and"}],"source_content_type":"text/x-python","patch_set":5,"id":"f8fa0b80_a31f961e","line":3071,"range":{"start_line":3069,"start_character":0,"end_line":3071,"end_character":60},"in_reply_to":"7932914a_51681d45","updated":"2025-02-20 03:41:35.000000000","message":"The reason why I moved this out of the try block is that `new_type` is only set here in this function, and I wanted to pass `new_type` to the notify call to save an extraneous database query. I\u0027ve added support for getting the volume object by the passed ID in `_notify_about_volume_usage` though, I think this is no longer necessary and we can just remove setting `volume_type_name` in `extra_usage_info` to reduce the complexity of this.","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2934968be04ca4178ee10ff75996083577df05e3","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id,"},{"line_number":3160,"context_line":"                              \u0027volume_type_name\u0027: new_type[\u0027name\u0027]})"},{"line_number":3161,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3162,"context_line":"        LOG.info(\"Retype volume completed successfully.\","}],"source_content_type":"text/x-python","patch_set":5,"id":"40aa2ac9_e2daa53e","line":3159,"updated":"2025-02-20 00:26:17.000000000","message":"I really think volume.save() is being called in all the right places before we hit this notify call, so I don\u0027t think you need the extra_usage_info at all (but correct me if I\u0027m wrong).","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"65f3ab44c12fab87feda94195ec04030e4dfa5d9","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id,"},{"line_number":3160,"context_line":"                              \u0027volume_type_name\u0027: new_type[\u0027name\u0027]})"},{"line_number":3161,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3162,"context_line":"        LOG.info(\"Retype volume completed successfully.\","}],"source_content_type":"text/x-python","patch_set":5,"id":"33d8f982_cc034d3a","line":3159,"in_reply_to":"16079bd0_3e134fec","updated":"2025-02-20 13:56:34.000000000","message":"OK, thanks for checking.  At worst this is redundant; it\u0027s not going to introduce an error, so I\u0027m fine with leaving it in.\n\nYou\u0027re right about the troubleshooting, the UT issue may be a difference between using the fake object vs. a real object, which would need some real digging to figure out.  (And it could turn out that you\u0027re right in the first place, that this is necessary!)","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"29e0db89c6ae2d8eadff0976a9f7ec901d019a92","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id,"},{"line_number":3160,"context_line":"                              \u0027volume_type_name\u0027: new_type[\u0027name\u0027]})"},{"line_number":3161,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3162,"context_line":"        LOG.info(\"Retype volume completed successfully.\","}],"source_content_type":"text/x-python","patch_set":5,"id":"c43da13d_861b76f6","line":3159,"in_reply_to":"33d8f982_cc034d3a","updated":"2025-02-20 18:00:23.000000000","message":"Thanks, sounds good to me. Anything else you\u0027d like me to address in this area? At this point I think some of these comments can be resolved.","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"},{"author":{"_account_id":36393,"name":"Callum Dickinson","email":"callum.dickinson@catalystcloud.nz","username":"Callum027","status":"Catalyst Cloud"},"change_message_id":"e38fade19de3da85979b821fbfdf3adf05999e04","unresolved":true,"context_lines":[{"line_number":3156,"context_line":"            QUOTAS.commit(context, new_reservations, project_id\u003dproject_id)"},{"line_number":3157,"context_line":"        self._notify_about_volume_usage("},{"line_number":3158,"context_line":"            context, volume, \"retype\","},{"line_number":3159,"context_line":"            extra_usage_info\u003d{\u0027volume_type\u0027: new_type_id,"},{"line_number":3160,"context_line":"                              \u0027volume_type_name\u0027: new_type[\u0027name\u0027]})"},{"line_number":3161,"context_line":"        self.publish_service_capabilities(context)"},{"line_number":3162,"context_line":"        LOG.info(\"Retype volume completed successfully.\","}],"source_content_type":"text/x-python","patch_set":5,"id":"16079bd0_3e134fec","line":3159,"in_reply_to":"40aa2ac9_e2daa53e","updated":"2025-02-20 03:41:35.000000000","message":"I changed the unit test where this is checked to also check the returned usage info from `_usage_from_volume`, and it looks like `extra_usage_info` is needed for the test to pass, at least at the moment. So I\u0027m leaving it in, as I don\u0027t think I quite have the knowledge to troubleshoot that.","commit_id":"9f504d4eb88ea1f4a23ffd0d4a2583e9921dd5ad"}]}