)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"2081f9d7c05383abb2715442b5e1b76a2aa2aad1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3fff1858_1f8757da","updated":"2025-08-28 07:25:53.000000000","message":"Rebased on the py39 fix","commit_id":"c06fe27d17a962aca9f0f9e75bdcf0f9145fe932"}],"magnum_capi_helm/common/app_creds.py":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"de1bab7b0d7485987a13b21c978bf307b3dc18cb","unresolved":true,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def delete_app_cred(cluster, app_cred_id):"},{"line_number":97,"context_line":"    # admin privileges needed to delete app creds outside requestor scope"},{"line_number":98,"context_line":"    context \u003d ctx.make_admin_context()"},{"line_number":99,"context_line":"    osc \u003d clients.OpenStackClients(context)"},{"line_number":100,"context_line":"    kst \u003d osc.keystone()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ad8183eb_c1c47d8e","line":97,"updated":"2025-08-27 04:46:05.000000000","message":"```suggestion\n    # NOTE: admin privileges needed to delete app creds outside requestor scope\n```","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"},{"author":{"_account_id":38227,"name":"Matthew Northcott","display_name":"Matthew Northcott","email":"matthewnorthcott@catalystcloud.nz","username":"northcottmt"},"change_message_id":"a14537ed50e1c3312583cf96a18710a459b742e9","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def delete_app_cred(cluster, app_cred_id):"},{"line_number":97,"context_line":"    # admin privileges needed to delete app creds outside requestor scope"},{"line_number":98,"context_line":"    context \u003d ctx.make_admin_context()"},{"line_number":99,"context_line":"    osc \u003d clients.OpenStackClients(context)"},{"line_number":100,"context_line":"    kst \u003d osc.keystone()"}],"source_content_type":"text/x-python","patch_set":3,"id":"89047f8d_d4011cd1","line":97,"in_reply_to":"ad8183eb_c1c47d8e","updated":"2025-08-28 02:20:15.000000000","message":"Done","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"}],"magnum_capi_helm/driver.py":[{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"de1bab7b0d7485987a13b21c978bf307b3dc18cb","unresolved":true,"context_lines":[{"line_number":1239,"context_line":"        else:"},{"line_number":1240,"context_line":"            if old_app_cred_id:"},{"line_number":1241,"context_line":"                # remove the old credential now that the new one is working"},{"line_number":1242,"context_line":"                app_creds.delete_app_cred(cluster, old_app_cred_id)"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_COMPLETE"},{"line_number":1245,"context_line":"            cluster.status_reason \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"43c879ff_f634d773","line":1242,"updated":"2025-08-27 04:46:05.000000000","message":"if we fail here, nobody knows something failed, unless they look in the logs - right?","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"},{"author":{"_account_id":38227,"name":"Matthew Northcott","display_name":"Matthew Northcott","email":"matthewnorthcott@catalystcloud.nz","username":"northcottmt"},"change_message_id":"a14537ed50e1c3312583cf96a18710a459b742e9","unresolved":true,"context_lines":[{"line_number":1239,"context_line":"        else:"},{"line_number":1240,"context_line":"            if old_app_cred_id:"},{"line_number":1241,"context_line":"                # remove the old credential now that the new one is working"},{"line_number":1242,"context_line":"                app_creds.delete_app_cred(cluster, old_app_cred_id)"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_COMPLETE"},{"line_number":1245,"context_line":"            cluster.status_reason \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"fe7b1ebb_48d9df59","line":1242,"in_reply_to":"43c879ff_f634d773","updated":"2025-08-28 02:20:15.000000000","message":"This is true as the exception handling in delete_app_cred is functionally unchanged to support cleanup of broken clusters. Though perhaps the try/except should be moved into the driver code so we can report something to the user. Is there a preferred way to do this while still reporting an UPDATE_COMPLETE status? It could be reported in status_reason but the user would still need to check for it.","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"},{"author":{"_account_id":38227,"name":"Matthew Northcott","display_name":"Matthew Northcott","email":"matthewnorthcott@catalystcloud.nz","username":"northcottmt"},"change_message_id":"3f742ad45a9a9307aba891dd9474fcdbe5266a9b","unresolved":false,"context_lines":[{"line_number":1239,"context_line":"        else:"},{"line_number":1240,"context_line":"            if old_app_cred_id:"},{"line_number":1241,"context_line":"                # remove the old credential now that the new one is working"},{"line_number":1242,"context_line":"                app_creds.delete_app_cred(cluster, old_app_cred_id)"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_COMPLETE"},{"line_number":1245,"context_line":"            cluster.status_reason \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"fabe607a_f16c4b12","line":1242,"in_reply_to":"b1ea17d4_4b5e7ff8","updated":"2025-08-29 02:21:43.000000000","message":"Thanks. I\u0027ve moved the error handling logic to driver.py to accommodate this.","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"dc80d6b2d1182ef14654a787c4e599615280922b","unresolved":true,"context_lines":[{"line_number":1239,"context_line":"        else:"},{"line_number":1240,"context_line":"            if old_app_cred_id:"},{"line_number":1241,"context_line":"                # remove the old credential now that the new one is working"},{"line_number":1242,"context_line":"                app_creds.delete_app_cred(cluster, old_app_cred_id)"},{"line_number":1243,"context_line":""},{"line_number":1244,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_COMPLETE"},{"line_number":1245,"context_line":"            cluster.status_reason \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"b1ea17d4_4b5e7ff8","line":1242,"in_reply_to":"fe7b1ebb_48d9df59","updated":"2025-08-28 07:27:15.000000000","message":"Let\u0027s just start with status_reason and UPDATE_COMPLETE, we don\u0027t have any other mechanism to inform the user","commit_id":"de64a74d0c055fa1265f9e1bb3f87da03b722aa3"},{"author":{"_account_id":14394,"name":"Dale Smith","email":"dale@catalystcloud.nz","username":"dalees"},"change_message_id":"0f492dfa4daa22a59765525f0ed86770173c6428","unresolved":true,"context_lines":[{"line_number":1261,"context_line":"        if error_msg:"},{"line_number":1262,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_FAILED"},{"line_number":1263,"context_line":"            cluster.status_reason \u003d error_msg"},{"line_number":1264,"context_line":"            cluster.user_id \u003d old_user_id"},{"line_number":1265,"context_line":"            raise exception.MagnumException(message\u003derror_msg)"},{"line_number":1266,"context_line":""},{"line_number":1267,"context_line":"        if old_app_cred_id:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9e0e3ac4_3711ce9e","line":1264,"updated":"2025-08-28 21:30:08.000000000","message":"is this control path missing a `cluster.save()`, or is it performed elsewhere?","commit_id":"c06fe27d17a962aca9f0f9e75bdcf0f9145fe932"},{"author":{"_account_id":38227,"name":"Matthew Northcott","display_name":"Matthew Northcott","email":"matthewnorthcott@catalystcloud.nz","username":"northcottmt"},"change_message_id":"3f742ad45a9a9307aba891dd9474fcdbe5266a9b","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"        if error_msg:"},{"line_number":1262,"context_line":"            cluster.status \u003d fields.ClusterStatus.UPDATE_FAILED"},{"line_number":1263,"context_line":"            cluster.status_reason \u003d error_msg"},{"line_number":1264,"context_line":"            cluster.user_id \u003d old_user_id"},{"line_number":1265,"context_line":"            raise exception.MagnumException(message\u003derror_msg)"},{"line_number":1266,"context_line":""},{"line_number":1267,"context_line":"        if old_app_cred_id:"}],"source_content_type":"text/x-python","patch_set":5,"id":"d7c84d18_aaec0c00","line":1264,"in_reply_to":"9e0e3ac4_3711ce9e","updated":"2025-08-29 02:21:43.000000000","message":"Good catch - yes it was missing. Fixed now.","commit_id":"c06fe27d17a962aca9f0f9e75bdcf0f9145fe932"}]}
