)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4d8539d1dd63b2725b311c11c3147fe0dbf3daa9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0d72caec_7c8b9237","updated":"2024-02-12 15:16:49.000000000","message":"Thank you very much for the fix!!\nI just have a concern with the possible size of the SQL query to send to MariaDB.","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"0677b0df8d85411fd2e6f02a281df75253d1988e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d7ab22ca_2ea88466","updated":"2024-02-12 07:40:28.000000000","message":"recheck lvm-lio-barbican job failed\n\ntempest.lib.exceptions.SSHTimeout: Connection to the 172.24.5.169 via SSH timed out.","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"508bcbfa5b1a7e548bfa79bb694f77d75085087d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"35b9a4f9_fbb7b62c","updated":"2024-02-15 12:55:20.000000000","message":"Thanks for a review Gorka, I went ahead with a different approach due to the reasons inline but it achieves the desired results.","commit_id":"2bd93e8cf87762c5a90b6ef195fce92cfb985dee"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5afb97a77f55e05c188a0079f91f29b8eb578669","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3776e265_964bb03f","updated":"2024-02-22 16:00:56.000000000","message":"Code \u0026 tests LGTM.","commit_id":"45250e9a929f20af9e2c40678fbf213e147c89ae"}],"cinder/db/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5afb97a77f55e05c188a0079f91f29b8eb578669","unresolved":true,"context_lines":[{"line_number":1980,"context_line":""},{"line_number":1981,"context_line":"# TODO: (D Release) remove method and this comment"},{"line_number":1982,"context_line":"def remove_temporary_admin_metadata_data_migration(context, max_count):"},{"line_number":1983,"context_line":"    return IMPL.remove_temporary_admin_metadata_data_migration("},{"line_number":1984,"context_line":"        context, max_count)"}],"source_content_type":"text/x-python","patch_set":4,"id":"17940058_32792ec3","line":1983,"range":{"start_line":1983,"start_character":4,"end_line":1983,"end_character":10},"updated":"2024-02-22 16:00:56.000000000","message":"I\u0027m kicking myself for not noticing this missing return statement on the original patch.","commit_id":"45250e9a929f20af9e2c40678fbf213e147c89ae"}],"cinder/db/sqlalchemy/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4d8539d1dd63b2725b311c11c3147fe0dbf3daa9","unresolved":true,"context_lines":[{"line_number":8591,"context_line":"    else:"},{"line_number":8592,"context_line":"        # We cannot use limit with update or delete so create a new query"},{"line_number":8593,"context_line":"        # sqlalchemy.exc.InvalidRequestError: Can\u0027t call Query.update()"},{"line_number":8594,"context_line":"        # or Query.delete() when limit() has been called"},{"line_number":8595,"context_line":"        ids \u003d []"},{"line_number":8596,"context_line":"        for obj in query.slice(0, max_count):"},{"line_number":8597,"context_line":"            ids.append(obj[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"9d2692eb_d749be78","line":8594,"updated":"2024-02-12 15:16:49.000000000","message":"Thank you for catching this!!","commit_id":"9bab09a7f0a2cc06f1fa81c6fe42d75cb20a7bcf"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4d8539d1dd63b2725b311c11c3147fe0dbf3daa9","unresolved":true,"context_lines":[{"line_number":8599,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8600,"context_line":"            update(admin_meta_table.delete_values())"},{"line_number":8601,"context_line":""},{"line_number":8602,"context_line":"    return total, updated"},{"line_number":8603,"context_line":""},{"line_number":8604,"context_line":""},{"line_number":8605,"context_line":"###############################"}],"source_content_type":"text/x-python","patch_set":1,"id":"a4b33433_777fadd0","line":8602,"updated":"2024-02-12 15:16:49.000000000","message":"I think we would need a unit test, since apparently my initial coverage didn\u0027t pick this up.","commit_id":"9bab09a7f0a2cc06f1fa81c6fe42d75cb20a7bcf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"508bcbfa5b1a7e548bfa79bb694f77d75085087d","unresolved":false,"context_lines":[{"line_number":8599,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8600,"context_line":"            update(admin_meta_table.delete_values())"},{"line_number":8601,"context_line":""},{"line_number":8602,"context_line":"    return total, updated"},{"line_number":8603,"context_line":""},{"line_number":8604,"context_line":""},{"line_number":8605,"context_line":"###############################"}],"source_content_type":"text/x-python","patch_set":1,"id":"94bdb47c_f1f1dad0","line":8602,"in_reply_to":"a4b33433_777fadd0","updated":"2024-02-15 12:55:20.000000000","message":"The issue was not with this return but with the DB API return statement.\nIt wasn\u0027t caught because we were calling sqla.api.\u003cmethod\u003e directly instead of db.api.\u003cmethod\u003e\nFixed it in UTs so now we call db.api layer which in turn calls sqla.api method.","commit_id":"9bab09a7f0a2cc06f1fa81c6fe42d75cb20a7bcf"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4d8539d1dd63b2725b311c11c3147fe0dbf3daa9","unresolved":true,"context_lines":[{"line_number":8590,"context_line":"        updated \u003d query.update(admin_meta_table.delete_values())"},{"line_number":8591,"context_line":"    else:"},{"line_number":8592,"context_line":"        # We cannot use limit with update or delete so create a new query"},{"line_number":8593,"context_line":"        ids \u003d []"},{"line_number":8594,"context_line":"        for obj in query.slice(0, max_count):"},{"line_number":8595,"context_line":"            ids.append(obj[\u0027id\u0027])"},{"line_number":8596,"context_line":"        updated \u003d model_query(context, admin_meta_table).\\"},{"line_number":8597,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8598,"context_line":"            update(admin_meta_table.delete_values())"}],"source_content_type":"text/x-python","patch_set":2,"id":"5cc0638b_34dacf84","line":8595,"range":{"start_line":8593,"start_character":0,"end_line":8595,"end_character":33},"updated":"2024-02-12 15:16:49.000000000","message":"```\n    ids \u003d [obj[\u0027id\u0027] for obj in query.slice(0, max_count)]\n```","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"508bcbfa5b1a7e548bfa79bb694f77d75085087d","unresolved":false,"context_lines":[{"line_number":8590,"context_line":"        updated \u003d query.update(admin_meta_table.delete_values())"},{"line_number":8591,"context_line":"    else:"},{"line_number":8592,"context_line":"        # We cannot use limit with update or delete so create a new query"},{"line_number":8593,"context_line":"        ids \u003d []"},{"line_number":8594,"context_line":"        for obj in query.slice(0, max_count):"},{"line_number":8595,"context_line":"            ids.append(obj[\u0027id\u0027])"},{"line_number":8596,"context_line":"        updated \u003d model_query(context, admin_meta_table).\\"},{"line_number":8597,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8598,"context_line":"            update(admin_meta_table.delete_values())"}],"source_content_type":"text/x-python","patch_set":2,"id":"f1fcbf3c_19f393f3","line":8595,"range":{"start_line":8593,"start_character":0,"end_line":8595,"end_character":33},"in_reply_to":"5cc0638b_34dacf84","updated":"2024-02-15 12:55:20.000000000","message":"Done","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4d8539d1dd63b2725b311c11c3147fe0dbf3daa9","unresolved":true,"context_lines":[{"line_number":8593,"context_line":"        ids \u003d []"},{"line_number":8594,"context_line":"        for obj in query.slice(0, max_count):"},{"line_number":8595,"context_line":"            ids.append(obj[\u0027id\u0027])"},{"line_number":8596,"context_line":"        updated \u003d model_query(context, admin_meta_table).\\"},{"line_number":8597,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8598,"context_line":"            update(admin_meta_table.delete_values())"},{"line_number":8599,"context_line":""},{"line_number":8600,"context_line":"    return total, updated"},{"line_number":8601,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2c74e34b_1560df32","line":8598,"range":{"start_line":8596,"start_character":0,"end_line":8598,"end_character":52},"updated":"2024-02-12 15:16:49.000000000","message":"-1: I think this could be problematic depending on the value of max_count, as that could make the generate a huge SQL statement to be sent to MariaDB.\n\nI think there was another workaround to the LIMIT \u0026 UPDATE problem, which was using a nested join on the same table.\n\nMaybe something like this would work?\n\n```\nids_query \u003d (\n    select([admin_metadata_table.c.id])\n    .where(admin_metadata_table.c.key \u003d\u003d \u0027temporary\u0027)\n    .limit(max_count)\n    .correlate(admin_metadata_table)\n)\n\nquery \u003d (\n    update(admin_metadata_table)\n    .values(admin_metadata_table.delete_values())\n    .where(admin_metadata_tablec.id.in_(ids_query))\n)\n```","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"508bcbfa5b1a7e548bfa79bb694f77d75085087d","unresolved":false,"context_lines":[{"line_number":8593,"context_line":"        ids \u003d []"},{"line_number":8594,"context_line":"        for obj in query.slice(0, max_count):"},{"line_number":8595,"context_line":"            ids.append(obj[\u0027id\u0027])"},{"line_number":8596,"context_line":"        updated \u003d model_query(context, admin_meta_table).\\"},{"line_number":8597,"context_line":"            filter(admin_meta_table.id.in_(ids)).\\"},{"line_number":8598,"context_line":"            update(admin_meta_table.delete_values())"},{"line_number":8599,"context_line":""},{"line_number":8600,"context_line":"    return total, updated"},{"line_number":8601,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1432a185_b59bb63d","line":8598,"range":{"start_line":8596,"start_character":0,"end_line":8598,"end_character":52},"in_reply_to":"2c74e34b_1560df32","updated":"2024-02-15 12:55:20.000000000","message":"I tried with the following method but got the error\n\nERROR cinder.cmd.manage [None req-851d65c2-6ad1-4170-ad60-1088881b5f89 None None] Error attempting to run remove_temporary_admin_metadata_data_migration: sqlalchemy.exc.InvalidRequestError: Could not evaluate current criteria in Python: \"Cannot evaluate Select\". Specify \u0027fetch\u0027 or False for the synchronize_session execution option.\n\nAfter that I added the synchronize_session in the execute query but got the following new error\n\nERROR cinder.cmd.manage oslo_db.exception.DBNotSupportedError: (pymysql.err.NotSupportedError) (1235, \"This version of MySQL doesn\u0027t yet support \u0027LIMIT \u0026 IN/ALL/ANY/SOME subquery\u0027\")\n\n\nEventually I went ahead with using subquery instead of the select and it works","commit_id":"6ebefebd253eea12b4bc60a99c41dba041333d0c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4fe35a8ae401ea835abd151f2f156f2e52d47ed8","unresolved":true,"context_lines":[{"line_number":8592,"context_line":"    # We cannot use limit with update or delete so create a new query"},{"line_number":8593,"context_line":"    updated \u003d model_query(context, admin_meta_table).\\"},{"line_number":8594,"context_line":"        filter(admin_meta_table.id.in_(ids_query)).\\"},{"line_number":8595,"context_line":"        update(admin_meta_table.delete_values(), **update_args)"},{"line_number":8596,"context_line":""},{"line_number":8597,"context_line":"    return total, updated"},{"line_number":8598,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9bcc7d85_e16077ba","line":8595,"updated":"2024-02-21 12:33:51.000000000","message":"Thanks, this should be enough to prevent the problem with too many ids.","commit_id":"45250e9a929f20af9e2c40678fbf213e147c89ae"}]}