)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"78b7ef69dffde82d0b232b546283b6c4f34c859a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b6f0a869_101b49ae","updated":"2026-08-11 23:05:24.000000000","message":"Two comments inline.","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c96e1e1b8de0debd65311369254ea537e10b8746","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"482c9e21_6e1a25db","updated":"2026-08-13 18:43:41.000000000","message":"I looked into this a bit, and think that this is a good approach.  Raising the priority because this is blocking an upper-constraints change to use alembic 1.19.0","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"afb81499fd7d5f01334e7e6899d0b8c8b2defe17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8174e2e3_9b69e564","updated":"2026-08-12 12:58:46.000000000","message":"The requirements patch that changes upper-constraints so that alembic\u003d\u003d\u003d1.19.0 now depends on this patch, so we can check the cross-cinder-py313 job on that patch to see that Simon\u0027s patch fixes the issue.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e4ba9801a7f4ed95afb31054a4653af5743c725b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"76f62e1b_f06b3b2c","updated":"2026-08-11 23:53:40.000000000","message":"Worth noting that PS1\u0027s Verified+1 doesn\u0027t validate this change. The failure only appears with alembic 1.19.0, which added CHECK constraint autogenerate via its new plugin system, and that\u0027s newer than what\u0027s in upper-constraints today — so a green gate here means the new comparator never ran, not that the fix works. The failure is visible in the cross-cinder-py313 job on the requirements upper-constraints bump (996013), where TestModelsSyncMySQL.test_models_sync is the only failure out of 18353 tests.\n\nNot adding a Depends-On: as this should merge ahead of the constraints bump rather than behind it, so cinder isn\u0027t the thing holding that bump up.\n\nVerification so far is filter_metadata_diff() exercised directly against the exact twelve-element diff from that job\u0027s log, plus negative cases — a CHECK constraint on an unlisted table, a non-CHECK constraint on a listed table, and the pre-existing nullable and nested-quota filters, all of which still report as before. It has not been proven end-to-end against a real MySQL schema with alembic 1.19.0, and won\u0027t be until the bump lands.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"87fe3210feb9c989498d4f9e5e150592089a908c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"36cfc8e6_b358e8a9","updated":"2026-08-16 08:14:32.000000000","message":"tested in https://review.opendev.org/c/openstack/requirements/+/977551","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5fc254e91c9df7cda277e4eabee5d7cb6b742679","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8680b657_34adf952","in_reply_to":"8174e2e3_9b69e564","updated":"2026-08-13 13:44:48.000000000","message":"The cross-cinder-py313 job passes with alembic 1.19.0","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a4aa43a45019aa4757262a780192f0643868f18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"3aa3857a_f77dfdee","updated":"2026-08-18 12:32:09.000000000","message":"AFAIK, we (openstack) still support (or at least haven\u0027t claimed not to support) Postgres [0], which has a real boolean type, so the check constraint (even if it makes sense to have one on a boolean column in Postgres) would not be limiting the value to (0, 1).  So it\u0027s not obvious to me what kind of check constraint we would define in the models.  I think Simon\u0027s approach makes sense until we reach the point where we really want/need to use check constraints.  We don\u0027t allow third-party tooling access to the DB (at least not officially); they\u0027re supposed to go through the REST API to get information about the resources managed by cinder.  (IIUC, the primary use case for check constraints is when there are multiple applications manipulating the database, and you want to maintain consistency between them.)  So I\u0027m not convinced that this situation (i.e., where we really need check constraints in both the models and the database initialization) will arise.  But if it does, we\u0027ll have a real use case to work with, which will help guide development.  My worry is that if we make some changes now just to satisfy the tests, we may do an incorrect/incomplete job that will have to be re-done later to really be useful.\n\n[0] https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/IUVWTGY5IYWT5PNRSG2MPDDX56O6F452/#IUVWTGY5IYWT5PNRSG2MPDDX56O6F452","commit_id":"913fa91a8eee20bf852387fe8c01a2d3d45cb87e"}],"cinder/tests/unit/db/test_migrations.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"78b7ef69dffde82d0b232b546283b6c4f34c859a","unresolved":true,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE(sdodsley): The initial migration creates the \u0027deleted\u0027 column"},{"line_number":145,"context_line":"        # of these tables as \u0027sa.Boolean(create_constraint\u003dTrue, name\u003dNone)\u0027,"},{"line_number":146,"context_line":"        # which emits a \u0027CHECK (deleted IN (0, 1))\u0027 constraint, while"},{"line_number":147,"context_line":"        # models.py declares a plain \u0027sa.Boolean\u0027 (\u0027create_constraint\u0027 has"},{"line_number":148,"context_line":"        # defaulted to False since SQLAlchemy 1.4).  The constraint therefore"},{"line_number":149,"context_line":"        # exists in the database but not in the model.  alembic only started"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ef54c06_d801a418","line":146,"range":{"start_line":146,"start_character":10,"end_line":146,"end_character":62},"updated":"2026-08-11 23:05:24.000000000","message":"This is true for mysql and mariadb, but probably not for postgres, which has a \"real\" boolean type.  It\u0027s probably sufficient to say that the initial migration has create_constraint\u003dTrue for the \u0027deleted\u0027 column of 12 tables, whereas none of their corresponding models specify create_constraint, and the default is False.","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c96e1e1b8de0debd65311369254ea537e10b8746","unresolved":false,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE(sdodsley): The initial migration creates the \u0027deleted\u0027 column"},{"line_number":145,"context_line":"        # of these tables as \u0027sa.Boolean(create_constraint\u003dTrue, name\u003dNone)\u0027,"},{"line_number":146,"context_line":"        # which emits a \u0027CHECK (deleted IN (0, 1))\u0027 constraint, while"},{"line_number":147,"context_line":"        # models.py declares a plain \u0027sa.Boolean\u0027 (\u0027create_constraint\u0027 has"},{"line_number":148,"context_line":"        # defaulted to False since SQLAlchemy 1.4).  The constraint therefore"},{"line_number":149,"context_line":"        # exists in the database but not in the model.  alembic only started"}],"source_content_type":"text/x-python","patch_set":1,"id":"faa86576_5a4fe2f1","line":146,"range":{"start_line":146,"start_character":10,"end_line":146,"end_character":62},"in_reply_to":"34e5df78_a4825b3d","updated":"2026-08-13 18:43:41.000000000","message":"Done","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e4ba9801a7f4ed95afb31054a4653af5743c725b","unresolved":true,"context_lines":[{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        # NOTE(sdodsley): The initial migration creates the \u0027deleted\u0027 column"},{"line_number":145,"context_line":"        # of these tables as \u0027sa.Boolean(create_constraint\u003dTrue, name\u003dNone)\u0027,"},{"line_number":146,"context_line":"        # which emits a \u0027CHECK (deleted IN (0, 1))\u0027 constraint, while"},{"line_number":147,"context_line":"        # models.py declares a plain \u0027sa.Boolean\u0027 (\u0027create_constraint\u0027 has"},{"line_number":148,"context_line":"        # defaulted to False since SQLAlchemy 1.4).  The constraint therefore"},{"line_number":149,"context_line":"        # exists in the database but not in the model.  alembic only started"}],"source_content_type":"text/x-python","patch_set":1,"id":"34e5df78_a4825b3d","line":146,"range":{"start_line":146,"start_character":10,"end_line":146,"end_character":62},"in_reply_to":"6ef54c06_d801a418","updated":"2026-08-11 23:53:40.000000000","message":"True - noted and reworded","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"78b7ef69dffde82d0b232b546283b6c4f34c859a","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            # NOTE(sdodsley): The constraints are unnamed, so the name is"},{"line_number":171,"context_line":"            # generated by the backend (\u0027CONSTRAINT_1\u0027 on MariaDB,"},{"line_number":172,"context_line":"            # \u0027\u003ctable\u003e_chk_1\u0027 on MySQL) and can\u0027t be matched on.  These tables"},{"line_number":173,"context_line":"            # have no other CHECK constraint, in the model or the database."},{"line_number":174,"context_line":"            if element[0] !\u003d \u0027remove_constraint\u0027:"},{"line_number":175,"context_line":"                return False"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"85f40aec_5d6c1f59","line":173,"updated":"2026-08-11 23:05:24.000000000","message":"I was thinking we could just add the check_constraint to the models (which wouldn\u0027t require a DB migration), but since the initial migration doesn\u0027t give them names, I guess we have no way to see that the constraint in the model matches the constraint in the db.\n\nnit: this function should be named \u0027ignore_check_constraints\u0027, since that\u0027s what it does (it just so happens that the only ones defined are \u0027deleted\u0027 checks).","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c96e1e1b8de0debd65311369254ea537e10b8746","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            # NOTE(sdodsley): The constraints are unnamed, so the name is"},{"line_number":171,"context_line":"            # generated by the backend (\u0027CONSTRAINT_1\u0027 on MariaDB,"},{"line_number":172,"context_line":"            # \u0027\u003ctable\u003e_chk_1\u0027 on MySQL) and can\u0027t be matched on.  These tables"},{"line_number":173,"context_line":"            # have no other CHECK constraint, in the model or the database."},{"line_number":174,"context_line":"            if element[0] !\u003d \u0027remove_constraint\u0027:"},{"line_number":175,"context_line":"                return False"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"37267377_6bb7663d","line":173,"in_reply_to":"79047aa8_b8817af7","updated":"2026-08-13 18:43:41.000000000","message":"Done","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e4ba9801a7f4ed95afb31054a4653af5743c725b","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            # NOTE(sdodsley): The constraints are unnamed, so the name is"},{"line_number":171,"context_line":"            # generated by the backend (\u0027CONSTRAINT_1\u0027 on MariaDB,"},{"line_number":172,"context_line":"            # \u0027\u003ctable\u003e_chk_1\u0027 on MySQL) and can\u0027t be matched on.  These tables"},{"line_number":173,"context_line":"            # have no other CHECK constraint, in the model or the database."},{"line_number":174,"context_line":"            if element[0] !\u003d \u0027remove_constraint\u0027:"},{"line_number":175,"context_line":"                return False"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"79047aa8_b8817af7","line":173,"in_reply_to":"85f40aec_5d6c1f59","updated":"2026-08-11 23:53:40.000000000","message":"function renamed and note added to the comments","commit_id":"54e0c87ac00f652b19d885e65fe77fdb7a67423d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"4e6021f708911b3c7ad5786d594a2ca4f5f9aeed","unresolved":true,"context_lines":[{"line_number":127,"context_line":"        or a list of tuples:"},{"line_number":128,"context_line":"        https://alembic.sqlalchemy.org/en/latest/api/autogenerate.html#getting-diffs"},{"line_number":129,"context_line":"        \"\"\""},{"line_number":130,"context_line":"        # TODO: (D Release) Remove this function and its usage"},{"line_number":131,"context_line":"        def ignore_leftover_nested_quota(element):"},{"line_number":132,"context_line":"            operation \u003d element[0]"},{"line_number":133,"context_line":"            if operation \u003d\u003d \u0027remove_column\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a651b807_fc175322","line":130,"updated":"2026-08-17 15:06:01.000000000","message":"\"D Release\" here is referring to the Dalmatian release -- changing this comment now almost 5 years later will just introduce confusion when we\u0027re digging through git history.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"0580defa295d73ed2e2742f43209ece8ef8e9c4b","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        or a list of tuples:"},{"line_number":128,"context_line":"        https://alembic.sqlalchemy.org/en/latest/api/autogenerate.html#getting-diffs"},{"line_number":129,"context_line":"        \"\"\""},{"line_number":130,"context_line":"        # TODO: (D Release) Remove this function and its usage"},{"line_number":131,"context_line":"        def ignore_leftover_nested_quota(element):"},{"line_number":132,"context_line":"            operation \u003d element[0]"},{"line_number":133,"context_line":"            if operation \u003d\u003d \u0027remove_column\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"8174a6e4_9658f4a0","line":130,"in_reply_to":"a651b807_fc175322","updated":"2026-08-18 03:45:35.000000000","message":"Thanks for fixing that directly — agreed, wasn\u0027t worth the blame churn.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c96e1e1b8de0debd65311369254ea537e10b8746","unresolved":true,"context_lines":[{"line_number":179,"context_line":"            constraint \u003d element[1]"},{"line_number":180,"context_line":"            if not isinstance(constraint, sqlalchemy.CheckConstraint):"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            return constraint.table.name in check_constraint_tables"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        def include_element(element):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6af22011_9d269e96","line":182,"updated":"2026-08-13 18:43:41.000000000","message":"I was hoping that we might be able to do something more fine-grained here (like see if the constraint is on the \u0027deleted\u0027 column), but the element we\u0027re working with doesn\u0027t have a lot of useful info:\n(\u0027remove_constraint\u0027, CheckConstraint(\u003csqlalchemy.sql.elements.TextClause object at 0x7fa994e80140\u003e, name\u003d\u0027CONSTRAINT_1\u0027, table\u003dTable(\u0027volume_glance_metadata\u0027, MetaData(), Column(\u0027x\u0027, Integer(), table\u003d\u003cvolume_glance_metadata\u003e), schema\u003dNone)))\n\nSo I think the way Simon decided to do this is fine for now.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1ef960f2549d6d2eaa66fd2da259df10a2bdcd50","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            constraint \u003d element[1]"},{"line_number":180,"context_line":"            if not isinstance(constraint, sqlalchemy.CheckConstraint):"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            return constraint.table.name in check_constraint_tables"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        def include_element(element):"}],"source_content_type":"text/x-python","patch_set":2,"id":"40c49d34_f85ecc79","line":182,"in_reply_to":"58a9f492_81e61aec","updated":"2026-08-18 12:50:59.000000000","message":"I think we would have to define a unique name for each constraint, which would mean doing a DB migration, which we wanted to avoid.  From what I can see from poking around, each constraint is named relative to the table that holds it.  So for all the current tables, each constraint is named \u0027CONSTRAINT_1\u0027 *in each table*.  (At least that\u0027s what I think is going on.)  So if we introduced a check constraint on an int column to say limit the value to (0,255) in a table not in the current list, I believe it would also be named by default \u0027CONSTRAINT_1\u0027.  It seems like what we\u0027d have to do is name each constraint similar to how we name indices or FK constraints.  I think this is still an evolving part of alembic, so maybe the situation will improve.  But this patch leaves us no worse off than we are now.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ec48037872d7306adaa97638f9555677a9d84077","unresolved":true,"context_lines":[{"line_number":179,"context_line":"            constraint \u003d element[1]"},{"line_number":180,"context_line":"            if not isinstance(constraint, sqlalchemy.CheckConstraint):"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            return constraint.table.name in check_constraint_tables"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        def include_element(element):"}],"source_content_type":"text/x-python","patch_set":2,"id":"8967ce7c_877c12a2","line":182,"in_reply_to":"6af22011_9d269e96","updated":"2026-08-17 15:01:22.000000000","message":"I\u0027d suggested checking [the name of the constraint rather than the table](https://review.opendev.org/c/openstack/cinder/+/1001176). We could still do that, assuming we can enumerate the different potential names (I\u0027d only accounted for MySQL)? What\u0027s done here is probably good enough though 🤷","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"0580defa295d73ed2e2742f43209ece8ef8e9c4b","unresolved":true,"context_lines":[{"line_number":179,"context_line":"            constraint \u003d element[1]"},{"line_number":180,"context_line":"            if not isinstance(constraint, sqlalchemy.CheckConstraint):"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            return constraint.table.name in check_constraint_tables"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        def include_element(element):"}],"source_content_type":"text/x-python","patch_set":2,"id":"902b285f_bbb713db","line":182,"in_reply_to":"8967ce7c_877c12a2","updated":"2026-08-18 03:45:35.000000000","message":"Agreed name-matching is the more fragile option — the names are backend-generated (`CONSTRAINT_1` on MariaDB, `\u003ctable\u003e_chk_1` on MySQL 8), so any enumeration rots silently and fails red on somebody\u0027s gate node. Keying on the table is backend-independent, and these twelve tables have no other CHECK constraint in either the model or the DB.\n\nOn the finer-grained check: the repr is misleading — the `TextClause` should carry the actual SQL, so `str(constraint.sqltext)` ought to give something like ``deleted` in (0,1)`, which would let us match on the column after all. I haven\u0027t confirmed what MySQL reflection normalizes that to, and with 1001202 stacked on this one I\u0027d rather not re-parent it for extra precision neither of you needs. Worth a follow-up if it turns out to be reliable.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"},{"author":{"_account_id":13425,"name":"Simon Dodsley","email":"simon@everpuredata.com","username":"sdodsley"},"change_message_id":"e56e81f2cb12005d43e90a29460d16bf4f0842b3","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            constraint \u003d element[1]"},{"line_number":180,"context_line":"            if not isinstance(constraint, sqlalchemy.CheckConstraint):"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            return constraint.table.name in check_constraint_tables"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        def include_element(element):"}],"source_content_type":"text/x-python","patch_set":2,"id":"58a9f492_81e61aec","line":182,"in_reply_to":"902b285f_bbb713db","updated":"2026-08-18 03:45:58.000000000","message":"Marked as resolved.","commit_id":"259bf514f8cfd80e10a9dfe54a82d2c45125b88d"}]}