)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e0d53e0e5f7cb3f52b6db3cf176f7731035f7e71","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Christian Rohmann \u003cchristian.rohmann@inovex.de\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-06-12 10:15:50 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":" db: Drop redundant index on ports table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":" * There already exists a unique constraint on the same columns, making"},{"line_number":10,"context_line":"   an additional index redundant."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"de7e1d31_3192da22","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":5},"updated":"2023-06-15 16:59:18.000000000","message":"Please, remove this","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Christian Rohmann \u003cchristian.rohmann@inovex.de\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-06-12 10:15:50 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":" db: Drop redundant index on ports table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":" * There already exists a unique constraint on the same columns, making"},{"line_number":10,"context_line":"   an additional index redundant."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bb400f0c_ac678a6b","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":5},"in_reply_to":"de7e1d31_3192da22","updated":"2023-06-16 09:03:23.000000000","message":"Ack","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e0d53e0e5f7cb3f52b6db3cf176f7731035f7e71","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":" db: Drop redundant index on ports table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":" * There already exists a unique constraint on the same columns, making"},{"line_number":10,"context_line":"   an additional index redundant."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1988421"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a015a9ed_a142d1d5","line":9,"range":{"start_line":9,"start_character":2,"end_line":9,"end_character":4},"updated":"2023-06-15 16:59:18.000000000","message":"nit: there is no need to add a bullet point here","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":" db: Drop redundant index on ports table"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":" * There already exists a unique constraint on the same columns, making"},{"line_number":10,"context_line":"   an additional index redundant."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Closes-Bug: #1988421"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ec76a334_28e4544e","line":9,"range":{"start_line":9,"start_character":2,"end_line":9,"end_character":4},"in_reply_to":"a015a9ed_a142d1d5","updated":"2023-06-16 09:03:23.000000000","message":"Ack","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"f08528a85d60586474a33bbf3443f95e8df18c31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"65d00391_b8ab3b0e","updated":"2023-06-07 11:05:28.000000000","message":"Dear reviewers, I know there is an Alembic revision missing. \nI tried as instructed in https://docs.openstack.org/neutron/latest/contributor/alembic_migrations.html but was unable to quickly figure out how to auto-generate a revision in this case.\n\nMaybe something as changed during the (ongoing) migration to SQLAlchemy 2.0?\nOr I am simply holding this thing wrong.","commit_id":"670f0daac171d274ee2349ab76778317122b7b6a"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b7c761e215c40852f01e217cc590e10c45ef802a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1c6c48dd_b43c36f3","updated":"2023-06-07 13:53:03.000000000","message":"The rationale of this index is to speed up the SQL select for ports with \u0027network_id + mac_address\u0027 (from [1] commit message).\n\nActually this filter is being used in [2], every time a new port is created and a MAC is assigned, in order to check if this MAC is present in this network.\n\n[1]https://review.opendev.org/c/openstack/neutron/+/169641\n[2]https://github.com/openstack/neutron/blob/ad81687e2955f494c28e2122763a2332afd3d26d/neutron/db/db_base_plugin_common.py#L111-L113","commit_id":"39f7ed45e42d3e8087200a28f2fc012cbd07c814"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"7f1892a875b2ad054059cbb03d12cc499fbad01c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8305827e_bd5c6ffa","in_reply_to":"1c6c48dd_b43c36f3","updated":"2023-06-13 12:00:40.000000000","message":"This change is not about dropping all indices on these columns, but rather about dropping a second, redundant index on the the two same columns. This causes a warning by MariaDB ([4]) which is then logged by SQLAlchemy.\n\nSee my references in the corresponding bug [1] about unique constraints automatically maintaining an index without the need to create one yourself.\nThen there also is a view helping to identify those indices which are redundant.\n\n[1] https://bugs.launchpad.net/neutron/+bug/1988421\n[2] https://mariadb.com/docs/skysql-dbaas/ref/mdb/sys/schema_redundant_indexes/\n[3] https://docs.percona.com/percona-toolkit/pt-duplicate-key-checker.html\n[4] https://mariadb.com/kb/en/mariadb-error-codes/ -\u003e 1831","commit_id":"39f7ed45e42d3e8087200a28f2fc012cbd07c814"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e0d53e0e5f7cb3f52b6db3cf176f7731035f7e71","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fd70d371_250f67f7","updated":"2023-06-15 16:59:18.000000000","message":"We no longer use \"contract\" migrations. Actually this is not strictly a contract one; please move it to \"expand\".","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"49cf8514_c3fcbd74","in_reply_to":"fd70d371_250f67f7","updated":"2023-06-16 09:03:23.000000000","message":"I created the migration simply by using \n\ntox -e venv -- alembic -c placement/db/sqlalchemy/alembic.ini revision -m \u0027Drop redundant indexes for unique constraints\u0027 --autogenerate\n\n\nWas that the wrong move?","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"556c1455_4648da68","updated":"2023-06-16 09:03:23.000000000","message":"I uploaded a new patchset. PTAL.","commit_id":"406465ed8ebb504a75173b7294acfb7d0ab02b07"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6685b697e658497db83277ac0090032d852c228b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"eca2dfc4_51ced20d","updated":"2023-06-30 10:19:07.000000000","message":"+2 waiting for the CI, but looks good","commit_id":"8e2d305d9e3f8311ccf745916ebec4917d2f017e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1152e5473a9c12330a8c7156c833c984978dde93","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"97f8ae2b_fbe2e912","updated":"2023-06-30 09:43:52.000000000","message":"Just for context, this is the table definition for ports (snippet):\n```\n  UNIQUE KEY `uniq_ports0network_id0mac_address` (`network_id`,`mac_address`),\n  KEY `ix_ports_network_id_mac_address` (`network_id`,`mac_address`),\n```  \nIt makes sense to drop the second one.","commit_id":"8e2d305d9e3f8311ccf745916ebec4917d2f017e"}],"neutron/db/migration/alembic_migrations/versions/2023.2/contract/b1199a3adbef_de_duplicate_indices_for_ports.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e0d53e0e5f7cb3f52b6db3cf176f7731035f7e71","unresolved":true,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    # ### commands auto generated by Alembic - please adjust! ###"},{"line_number":34,"context_line":"    op.drop_index(\u0027ix_ports_network_id_mac_address\u0027, table_name\u003d\u0027ports\u0027)"},{"line_number":35,"context_line":"    # ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":3,"id":"7e7c6f7f_cc53b90b","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":16},"updated":"2023-06-15 16:59:18.000000000","message":"nit: Please remove these comments","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    # ### commands auto generated by Alembic - please adjust! ###"},{"line_number":34,"context_line":"    op.drop_index(\u0027ix_ports_network_id_mac_address\u0027, table_name\u003d\u0027ports\u0027)"},{"line_number":35,"context_line":"    # ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":3,"id":"0092290b_f07259bb","line":33,"range":{"start_line":33,"start_character":0,"end_line":33,"end_character":16},"in_reply_to":"7e7c6f7f_cc53b90b","updated":"2023-06-16 09:03:23.000000000","message":"Ack","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e0d53e0e5f7cb3f52b6db3cf176f7731035f7e71","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    # ### commands auto generated by Alembic - please adjust! ###"},{"line_number":34,"context_line":"    op.drop_index(\u0027ix_ports_network_id_mac_address\u0027, table_name\u003d\u0027ports\u0027)"},{"line_number":35,"context_line":"    # ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":3,"id":"79213afc_cc6a5a33","line":34,"range":{"start_line":34,"start_character":0,"end_line":34,"end_character":15},"updated":"2023-06-15 16:59:18.000000000","message":"Do not drop an index by its name, list the table indexes and then find the one that refers to the affected columns. Use that to delete it.\n\nUse something like this:\n    import sqlalchemy as sa\n    inspector \u003d sa.inspect(op.get_bind())\n    indexes \u003d inspector.get_indexes(TABLE)\n    \nThis will return something like this to \"indexes\" https://paste.opendev.org/show/bk0pYzzI5joVwsn2OqOs/\n\nFind the index with both columns and \u0027unique\u0027: False; then use the returned name of this index (that could be different depending on the DB engine) and use the drop_index method.","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"24165e3cb4d7c7cc41d24006e787b57c25f05217","unresolved":true,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    # ### commands auto generated by Alembic - please adjust! ###"},{"line_number":34,"context_line":"    op.drop_index(\u0027ix_ports_network_id_mac_address\u0027, table_name\u003d\u0027ports\u0027)"},{"line_number":35,"context_line":"    # ### end Alembic commands ###"}],"source_content_type":"text/x-python","patch_set":3,"id":"31b3ecca_7bdb9ccf","line":34,"range":{"start_line":34,"start_character":0,"end_line":34,"end_character":15},"in_reply_to":"79213afc_cc6a5a33","updated":"2023-06-16 09:03:23.000000000","message":"Are you certain this is really necessary? Should Alembic not keep the naming of indices consistent across DB engines?\n\nDon\u0027t get me wrong, but having to write all the migrations manually in this way, while using an ORM like Alembic, seems to render the use of this abstraction or at least its built-in schema autogeneration and migration a little pointless. Not being able to rely on names being consistent and writing code to pattern match indices feels a little strange. It\u0027s not just some imperative statements of getting your database from state A to state B anymore. Rather you are switching to running code determining the actually required and executed actions at runtime.\n\nMaybe at least the (contributors) documentation at https://docs.openstack.org/neutron/latest/contributor/alembic_migrations.html#script-auto-generation needs some updating?\n\n\nI changed the code as requested though, PTAL.","commit_id":"5194528a15c8925c86632bef3381926756a99ac7"}],"neutron/db/migration/alembic_migrations/versions/2023.2/expand/b1199a3adbef_de_duplicate_indices_for_ports.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6685b697e658497db83277ac0090032d852c228b","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    indexes \u003d inspector.get_indexes(\"ports\")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    for index in indexes:"},{"line_number":37,"context_line":"        if index[\u0027unique\u0027] is False:"},{"line_number":38,"context_line":"            if index[\u0027column_names\u0027] \u003d\u003d [\u0027network_id\u0027, \u0027mac_address\u0027]:"},{"line_number":39,"context_line":"                op.drop_index(index[\u0027name\u0027], table_name\u003d\u0027ports\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ab626926_b1e2123f","line":37,"updated":"2023-06-30 10:19:07.000000000","message":"You\u0027ll need an exception for this command. Something like https://review.opendev.org/c/openstack/neutron/+/886213/6/neutron/db/migration/alembic_migrations/versions/2023.2/expand/0aefee21cd87_remove_dedundant_indexes.py","commit_id":"8e2d305d9e3f8311ccf745916ebec4917d2f017e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"312ca6cb450f33abe30ca553e9090c651334b65e","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    indexes \u003d inspector.get_indexes(\"ports\")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    for index in indexes:"},{"line_number":37,"context_line":"        if index[\u0027unique\u0027] is False:"},{"line_number":38,"context_line":"            if index[\u0027column_names\u0027] \u003d\u003d [\u0027network_id\u0027, \u0027mac_address\u0027]:"},{"line_number":39,"context_line":"                op.drop_index(index[\u0027name\u0027], table_name\u003d\u0027ports\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0c6c4b04_1c8fc821","line":37,"in_reply_to":"9c8e2434_ad1d6dcb","updated":"2023-07-03 07:52:21.000000000","message":"Done","commit_id":"8e2d305d9e3f8311ccf745916ebec4917d2f017e"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"dd43d32305e7dca047338ddf90bfd40a94875cbf","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    indexes \u003d inspector.get_indexes(\"ports\")"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    for index in indexes:"},{"line_number":37,"context_line":"        if index[\u0027unique\u0027] is False:"},{"line_number":38,"context_line":"            if index[\u0027column_names\u0027] \u003d\u003d [\u0027network_id\u0027, \u0027mac_address\u0027]:"},{"line_number":39,"context_line":"                op.drop_index(index[\u0027name\u0027], table_name\u003d\u0027ports\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9c8e2434_ad1d6dcb","line":37,"in_reply_to":"ab626926_b1e2123f","updated":"2023-06-30 15:09:43.000000000","message":"I pushed a change now.\nIf this is not correct, can you please explain a little more what the exception does and what kind of signature the returned objects need to have? Do I reference just the index which was dropped by name or do I reference the table name? Table name and columns?","commit_id":"8e2d305d9e3f8311ccf745916ebec4917d2f017e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4bdf08059d49bead14d203592abc84791c43da4e","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    redundant."},{"line_number":47,"context_line":"    \"\"\""},{"line_number":48,"context_line":"    return {"},{"line_number":49,"context_line":"        sa.Index: \"ix_ports_network_id_mac_address\","},{"line_number":50,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":7,"id":"f0c349cd_a36ae209","line":49,"range":{"start_line":49,"start_character":19,"end_line":49,"end_character":50},"updated":"2023-06-30 15:06:45.000000000","message":"This must be the table name, not the index name. Please, check the example provided (or other examples in the Neutron repository).","commit_id":"a6e0743cfdde87da3029593cc3d75961d364142e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"312ca6cb450f33abe30ca553e9090c651334b65e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    redundant."},{"line_number":47,"context_line":"    \"\"\""},{"line_number":48,"context_line":"    return {"},{"line_number":49,"context_line":"        sa.Index: \"ix_ports_network_id_mac_address\","},{"line_number":50,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":7,"id":"a6fed84e_b3652abb","line":49,"range":{"start_line":49,"start_character":19,"end_line":49,"end_character":50},"in_reply_to":"f0c349cd_a36ae209","updated":"2023-07-03 07:52:21.000000000","message":"Done","commit_id":"a6e0743cfdde87da3029593cc3d75961d364142e"}]}
