)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"cde2a41e02daccd313b20b39e53b9caba15a66b1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7213baac_82a79a06","updated":"2022-04-27 12:34:30.000000000","message":"I\u0027m not convinced this is beneficial enough considering it is change to old released migration script. In our disallowed minor code changes [0] we list migration scripts as one of those things that should not be touched for spelling/grammar changes and it also do state \"\"\"Modifying migration scripts confuses operators and administrators – we only want them to notice serious problems.\"\"\" I\u0027m not convinced this is one of those serious problems we should be changing old migration scripts for.\n\n[0] https://docs.openstack.org/glance/13.0.0/contributing/minor-code-changes.html","commit_id":"2adfe24b2a42b15a5b97a06b379bbe2fdaa45e81"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c34c0c2a9ce0f8d031bbd368343b8f388b5eb85e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"173e8799_74a81d78","updated":"2022-04-13 15:14:38.000000000","message":"Looks good,thank you!\n\nJust curious, ss everyone following this standard? because it will be little bit difficult for developer who doesn\u0027t follow this standard. (and they will not know about this unless they run pep8 before posting the patch)","commit_id":"2adfe24b2a42b15a5b97a06b379bbe2fdaa45e81"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c9a87ad3716059eaac80e0a58b65b6b2772996c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"950dba5c_e14d8dd6","updated":"2022-04-13 17:50:22.000000000","message":"Mostly looks good, but see comments in the sqlalchemy files.","commit_id":"2adfe24b2a42b15a5b97a06b379bbe2fdaa45e81"}],"glance/db/sqlalchemy/alembic_migrations/data_migrations/ocata_migrate01_community_images.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c9a87ad3716059eaac80e0a58b65b6b2772996c6","unresolved":true,"context_lines":[{"line_number":32,"context_line":"    images \u003d Table(\u0027images\u0027, meta, autoload\u003dTrue)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    rows_with_null_visibility \u003d (select([images.c.id])"},{"line_number":35,"context_line":"                                 .where(images.c.visibility is None)"},{"line_number":36,"context_line":"                                 .limit(1)"},{"line_number":37,"context_line":"                                 .execute())"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3305da2a_9b7cc822","line":35,"updated":"2022-04-13 17:50:22.000000000","message":"SQLAlchemy docs recommend using the ColumnOperators function is_() for this:\n\n    .where(images.c.visibility.is_(None))\n\n(something about the Python \u0027is\u0027 giving anomalous behavior for some databases).  Alternatively, you can just leave the \u003d\u003d in there and add\n\n    # noqa: E711\n\nat the end of the line.","commit_id":"2adfe24b2a42b15a5b97a06b379bbe2fdaa45e81"}],"glance/db/sqlalchemy/api.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c9a87ad3716059eaac80e0a58b65b6b2772996c6","unresolved":true,"context_lines":[{"line_number":412,"context_line":"                model_attr \u003d getattr(model, sort_keys[j])"},{"line_number":413,"context_line":"                default \u003d _get_default_column_value("},{"line_number":414,"context_line":"                    model_attr.property.columns[0].type)"},{"line_number":415,"context_line":"                attr \u003d sa_sql.expression.case([(model_attr is not None,"},{"line_number":416,"context_line":"                                              model_attr), ],"},{"line_number":417,"context_line":"                                              else_\u003ddefault)"},{"line_number":418,"context_line":"                crit_attrs.append((attr \u003d\u003d marker_values[j]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"11761f9e_1c4f4d04","line":415,"range":{"start_line":415,"start_character":59,"end_line":415,"end_character":65},"updated":"2022-04-13 17:50:22.000000000","message":"Same comment as in the migration file.  I think you should just leave the code as-is and add the # noqa comment.  Same for the below uses.","commit_id":"2adfe24b2a42b15a5b97a06b379bbe2fdaa45e81"}],"glance/db/sqlalchemy/migrate_repo/versions/010_default_update_at.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"691b54c76853aebabbd234243599b91dd447d8ec","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    conn \u003d migrate_engine.connect()"},{"line_number":40,"context_line":"    conn.execute("},{"line_number":41,"context_line":"        images_table.update("},{"line_number":42,"context_line":"            images_table.c.updated_at is None,"},{"line_number":43,"context_line":"            {images_table.c.updated_at: images_table.c.created_at}))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_731e24bb","line":42,"updated":"2020-07-27 20:58:22.000000000","message":"I was wondering about these in the SQL related code. They aren\u0027t passed in as strings, so I\u0027m not sure why it would be functionally any different. But based on the test failures, it looks like it doesn\u0027t like this syntax.","commit_id":"291999178d9e1c82fe6240ec348e09692972b98b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"de7016ae84ea25f09b702b588bd82b970cc12213","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    conn \u003d migrate_engine.connect()"},{"line_number":40,"context_line":"    conn.execute("},{"line_number":41,"context_line":"        images_table.update("},{"line_number":42,"context_line":"            images_table.c.updated_at is None,"},{"line_number":43,"context_line":"            {images_table.c.updated_at: images_table.c.created_at}))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_17cb274a","line":42,"in_reply_to":"9f560f44_731e24bb","updated":"2020-08-17 19:07:43.000000000","message":"Yeah, I would think the sqlalchemy stuff would get a pass on using \u003d\u003d False. Maybe we can keep this with a #noqa on it?\n\nHowever, I\u0027m not sure what test fails you see that would be related to this. AFAIK, this is just updating pre-existing data in the table and the bulk of the tests that fail are running these migrations against an empty database. Can you point me to something specific?","commit_id":"291999178d9e1c82fe6240ec348e09692972b98b"}]}