)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"There is a set of default data which needs to be ensured at process"},{"line_number":18,"context_line":"startup time. In production this is done via the database migration."},{"line_number":19,"context_line":"The default data is one row in the table: ID 1 with value UNKNOWN."},{"line_number":20,"context_line":"This is the default value for an unspecified consumer type."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"In future changes we may wish to AttributeCache consumer types in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1fa4df85_54510362","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":66},"updated":"2020-03-12 12:20:21.000000000","message":"Do we even need this now?","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b02bda0166634b6f76b1fb75b16e442c4081cd62","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"There is a set of default data which needs to be ensured at process"},{"line_number":18,"context_line":"startup time. In production this is done via the database migration."},{"line_number":19,"context_line":"The default data is one row in the table: ID 1 with value UNKNOWN."},{"line_number":20,"context_line":"This is the default value for an unspecified consumer type."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"In future changes we may wish to AttributeCache consumer types in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"1fa4df85_151d6e72","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":66},"in_reply_to":"1fa4df85_54510362","updated":"2020-03-18 18:14:14.000000000","message":"Hm ... well, I had been thinking to have the row as the \"official\" UNKNOWN type. Otherwise it\u0027s just a label for NULL.\n\nBut, maybe it is not a problem to have it be just a label for NULL. Let me look through the rest of your comments and see.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09c95f6d4d62b0cd227b8cbd8f91ba83e7202c99","unresolved":true,"context_lines":[{"line_number":15,"context_line":"in the consumer_types table."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"The consumer_type_id is nullable and records with a NULL"},{"line_number":18,"context_line":"consumer_type_id are considered as \u0027unknown\u0027."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In future changes we may wish to AttributeCache consumer types in"},{"line_number":21,"context_line":"the same way that traits and resource classes are cached."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"1a80ef58_6b035267","line":18,"updated":"2021-08-06 08:07:53.000000000","message":"Fair enough, this is noted here.","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/db/sqlalchemy/alembic/versions/422ece571366_add_consumer_types_table.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3d51bbcd41f84d28bff544f2820d814032841fca","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    fkey.create()"},{"line_number":52,"context_line":"    utils.add_index(migrate_engine, \u0027consumers\u0027,"},{"line_number":53,"context_line":"                    \u0027consumers_consumer_type_id_idx\u0027,"},{"line_number":54,"context_line":"                    [\u0027consumer_type_id\u0027])"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_1a0dff3d","line":54,"updated":"2019-07-04 13:22:39.000000000","message":"If this lengthier version for sqlite will also work/give the same results for other dbs (despite being wordier) might it make sense to use the same code for all dbs? They don\u0027t look significantly different in terms of results.","commit_id":"d3f158e057427103c0140d2de369318eba040a18"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"bf2954af9d9d78d80fd800978d9fd76c71ee32f2","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    fkey.create()"},{"line_number":52,"context_line":"    utils.add_index(migrate_engine, \u0027consumers\u0027,"},{"line_number":53,"context_line":"                    \u0027consumers_consumer_type_id_idx\u0027,"},{"line_number":54,"context_line":"                    [\u0027consumer_type_id\u0027])"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_dd1a0116","line":54,"in_reply_to":"7faddb67_1a0dff3d","updated":"2019-07-04 13:38:08.000000000","message":"hmm yea makes sense, let me try it the same way for the other db types. I guess it should work fine, its just that I wanted to initially add the foreign constraint while adding the column like in one go. But I think its safe to do it in two steps.","commit_id":"d3f158e057427103c0140d2de369318eba040a18"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"3d51bbcd41f84d28bff544f2820d814032841fca","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def downgrade():"},{"line_number":93,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_fa03430e","line":93,"updated":"2019-07-04 13:22:39.000000000","message":"you can remove this method since nothing is going to call it (the other migrations don\u0027t use it)","commit_id":"d3f158e057427103c0140d2de369318eba040a18"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"bf2954af9d9d78d80fd800978d9fd76c71ee32f2","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def downgrade():"},{"line_number":93,"context_line":"    pass"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_fdfeddac","line":93,"in_reply_to":"7faddb67_fa03430e","updated":"2019-07-04 13:38:08.000000000","message":"Done","commit_id":"d3f158e057427103c0140d2de369318eba040a18"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e0b67d1f87eb38e9311e4b09064cee95fecef7fc","unresolved":false,"context_lines":[{"line_number":31,"context_line":"depends_on \u003d None"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def _add_column_for_sqlite(migrate_engine, metadata):"},{"line_number":35,"context_line":"    # Since SQLite doesn\u0027t allow adding constraints while adding columns"},{"line_number":36,"context_line":"    # to existing tables it has to be handled separately. Same goes for"},{"line_number":37,"context_line":"    # the index."}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_271abb6e","line":34,"updated":"2019-08-29 13:31:39.000000000","message":"My earlier suggestion to just use this will not work. PostgreSQL ends up blocking in the tests if it follows this path.","commit_id":"80809e49672f5dc35630b7ab0d4e3984ade8c2ca"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"5d3a6d49652274010a1acf0eb08db6b9e378bdc4","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    data \u003d {"},{"line_number":71,"context_line":"        \u0027name\u0027: \"UNKNOWN\","},{"line_number":72,"context_line":"    }"},{"line_number":73,"context_line":"    connection.execute(sa.insert(consumer_types).values(data))"},{"line_number":74,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":75,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"},{"line_number":76,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_aa43d26b","line":73,"updated":"2019-08-29 14:46:49.000000000","message":"I\u0027m pretty sure that this is not going to work with the way the tests work. Rather than destroying the database and running the migrations again with each test, we instead wipe back to the default schema.\n\nThis means that we lose this column.\n\nI think.\n\nStill investigating.","commit_id":"80809e49672f5dc35630b7ab0d4e3984ade8c2ca"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"367156cb7d6c7c0838c7764ea22f5d62e136de8d","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            sa.Column("},{"line_number":81,"context_line":"                \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":82,"context_line":"                sa.ForeignKey(\u0027consumer_types.id\u0027),"},{"line_number":83,"context_line":"                server_default\u003dsa.text(\u00271\u0027),"},{"line_number":84,"context_line":"                nullable\u003dFalse"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_76d4eabc","line":83,"updated":"2020-02-06 20:06:29.000000000","message":"Just a note that I wonder if this is a bad thing to do, given the recent bug in nova:\n\nhttps://review.opendev.org/706351\n\nIn nova we use sqlalchemy-migrate whereas in placement we use alembic, so I\u0027m not sure if it would be a problem here?","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"121a1368da07fc0f75fed02b6ddf93d8258cf281","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            sa.Column("},{"line_number":81,"context_line":"                \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":82,"context_line":"                sa.ForeignKey(\u0027consumer_types.id\u0027),"},{"line_number":83,"context_line":"                server_default\u003dsa.text(\u00271\u0027),"},{"line_number":84,"context_line":"                nullable\u003dFalse"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_2c706209","line":83,"in_reply_to":"3fa7e38b_0c2806eb","updated":"2020-02-07 02:02:20.000000000","message":"I\u0027m not going to change it yet -- I just wanted to keep everyone up-to-date about the discussions we had in nova running into a problem involving column defaults.\n\nYes, exactly, we could handle the column in python and could choose to do lazy migrations in python (to set 1 if NULL when read) or we could choose not to migrate at all and treat NULL as unknown (that\u0027s what we\u0027re doing in nova).\n\nEither way, I\u0027m not going to do anything about it in the next patchset so we can think about it more. For the next update, I am only focusing on fixing the bug you found and also address the rest of your comments. After that, I can do another update to handle the \"default\" UNKNOWN consumer type in a different way.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"d164ff3ded142312e464981e8c91e6f7fb61fbb0","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            sa.Column("},{"line_number":81,"context_line":"                \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":82,"context_line":"                sa.ForeignKey(\u0027consumer_types.id\u0027),"},{"line_number":83,"context_line":"                server_default\u003dsa.text(\u00271\u0027),"},{"line_number":84,"context_line":"                nullable\u003dFalse"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_0c2806eb","line":83,"in_reply_to":"3fa7e38b_69cacc47","updated":"2020-02-07 01:53:13.000000000","message":"I think you mean you are going to change this to nullable\u003dTrue. I\u0027m okay with that, but then I wonder if we really need that \"UNKNOWN\" (implicit) default consumer type id and the name? That said, SQL is based on trivalent logic and the NULL value itself already includes the meaning of unknown [1]. We can just translate the null id to the \"unknown\" value in the python layer rather than having online migration for null to default id (name) of 1 (UNKNOWN), can\u0027t we?\n\n[1] SQL Antipatterns: Avoiding the Pitfalls of Database Programming, Chapter13. \"Fear of the Unknown\"","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3260c35f314180f22836a7cbe8fb1d5c3709a6e8","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            sa.Column("},{"line_number":81,"context_line":"                \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":82,"context_line":"                sa.ForeignKey(\u0027consumer_types.id\u0027),"},{"line_number":83,"context_line":"                server_default\u003dsa.text(\u00271\u0027),"},{"line_number":84,"context_line":"                nullable\u003dFalse"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_69cacc47","line":83,"in_reply_to":"3fa7e38b_76d4eabc","updated":"2020-02-07 00:05:04.000000000","message":"(Apologies if this is stating the obvious)\n\nChatted with zzzeek in the nova channel today and he confirmed that a server_default will indeed go and populate already existing records:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2020-02-06.log.html#t2020-02-06T20:38:02\n\nConsumers could be a very large table as they generally map to instances from a nova point of view. So, I\u0027m not sure we should do this in this way and should maybe instead have something in the code that populates consumer_type_id upon access or something like that?","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"efdafec96faec54bb5e3b379841c4b306ec7a9f7","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            sa.Column("},{"line_number":81,"context_line":"                \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":82,"context_line":"                sa.ForeignKey(\u0027consumer_types.id\u0027),"},{"line_number":83,"context_line":"                server_default\u003dsa.text(\u00271\u0027),"},{"line_number":84,"context_line":"                nullable\u003dFalse"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_1f8b4669","line":83,"updated":"2020-02-07 02:56:21.000000000","message":"TODO: do something different than this","commit_id":"62159ce3c1bc8d56dd5465c3f1634ff882ef46d7"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"d9f7e42f9cf22ab91f7d0fdc15d9767eb17b68de","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        \"consumers\","},{"line_number":41,"context_line":"        sa.Column("},{"line_number":42,"context_line":"            \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":43,"context_line":"            nullable\u003dTrue"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":"    )"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_abdfe80a","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":25},"updated":"2020-03-10 00:24:02.000000000","message":"Now this is nullable, then why do we need the DEFAULT consumer type of UNKNOWN?","commit_id":"5bec6b98267869a5e36a8816148dd2217407ca34"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"15c2c12fb26bd96c0ed20044038baa111c0a12b0","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        \"consumers\","},{"line_number":41,"context_line":"        sa.Column("},{"line_number":42,"context_line":"            \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":43,"context_line":"            nullable\u003dTrue"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":"    )"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_9e2d88ff","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":25},"in_reply_to":"1fa4df85_3e3ed4b7","updated":"2020-03-10 02:38:31.000000000","message":"\u003e Instead we do the data migration to fill in the default value\n \u003e UNKNOWN ourselves.\n\nIf we go along this way, we need both lazy fill and online migration here and clean up the lazy fill code with migration checker(blocker) in the next cycle.\n\nAlternatively we could fill (replace null to) the default value of \"UNKNOWN\" in upper layer at application level when it is asked via the REST APIs just to let users see that null means UNKNOWN. \n\nOne concern for this is that NULL often kills us, but having replacement value for NULL(unknown) is also known as anti-pattern because it increases migration codes and operations[1].\n\n[1] SQL Antipatterns: Avoiding the Pitfalls of Database Programming, Chapter13. \"Fear of the Unknown\" \n\nAnother concern is to have the performance degradation forever for this replacement, but this is negligible since we don\u0027t have an API to get details for all the consumers (i.e. GET /allocations)\n\n\nJust thinking out loud.","commit_id":"5bec6b98267869a5e36a8816148dd2217407ca34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"aed757107c3042bba2aa6b9592906f0ce6b04a65","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        \"consumers\","},{"line_number":41,"context_line":"        sa.Column("},{"line_number":42,"context_line":"            \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":43,"context_line":"            nullable\u003dTrue"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":"    )"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_de90e0ef","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":25},"in_reply_to":"1fa4df85_9e2d88ff","updated":"2020-03-10 02:55:24.000000000","message":"These are good points ... and now I understand better what you said in your earlier comment. Why not just use NULL as the default/unknown.\n\nI think NULL isn\u0027t the friendliest representation for the user so I do like the idea of doing a translation of null to default type UNKNOWN at the API or ConsumerType object layer (maybe the latter would be better).\n\nI think your explanation sounds like the better way. I was becoming concerned with how complex it is getting. I\u0027ll try a re-work of it in the next PS. Thanks","commit_id":"5bec6b98267869a5e36a8816148dd2217407ca34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c63d7b0e0c606b9054ff1696c1f316b9a587105b","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        \"consumers\","},{"line_number":41,"context_line":"        sa.Column("},{"line_number":42,"context_line":"            \u0027consumer_type_id\u0027, sa.Integer(),"},{"line_number":43,"context_line":"            nullable\u003dTrue"},{"line_number":44,"context_line":"        )"},{"line_number":45,"context_line":"    )"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_3e3ed4b7","line":43,"range":{"start_line":43,"start_character":11,"end_line":43,"end_character":25},"in_reply_to":"1fa4df85_abdfe80a","updated":"2020-03-10 01:17:15.000000000","message":"Ideally we\u0027d like for this column to be non-nullable. We make it nullable to avoid a bulk backfill of the consumer_type_id for all \"old\" consumers (for example, nova instances). There could be a very large number of consumer records in a running deployment.\n\nInstead we do the data migration to fill in the default value UNKNOWN ourselves. So I think we still need the default consumer type of UNKNOWN.\n\nAs I type this, I realize that we probably also need a placement-manage db online_data_migrations for this too. I had added a lazy fill of default consumer type whenever a read of consumer happened, but it would probably be simpler to handle it as an online data migration instead.","commit_id":"5bec6b98267869a5e36a8816148dd2217407ca34"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    connection \u003d context.get_bind()"},{"line_number":68,"context_line":"    metadata \u003d sa.MetaData(bind\u003dconnection)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    consumer_types \u003d sa.Table(\u0027consumer_types\u0027, metadata, autoload\u003dTrue)"},{"line_number":71,"context_line":"    data \u003d {\u0027name\u0027: \"UNKNOWN\"}"},{"line_number":72,"context_line":"    connection.execute(sa.insert(consumer_types).values(data))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":75,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_94a21b4b","line":72,"range":{"start_line":70,"start_character":0,"end_line":72,"end_character":62},"updated":"2020-03-12 12:20:21.000000000","message":"If we can assume that NULL means unknown, I think this is not necessary.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b02bda0166634b6f76b1fb75b16e442c4081cd62","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    connection \u003d context.get_bind()"},{"line_number":68,"context_line":"    metadata \u003d sa.MetaData(bind\u003dconnection)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    consumer_types \u003d sa.Table(\u0027consumer_types\u0027, metadata, autoload\u003dTrue)"},{"line_number":71,"context_line":"    data \u003d {\u0027name\u0027: \"UNKNOWN\"}"},{"line_number":72,"context_line":"    connection.execute(sa.insert(consumer_types).values(data))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":75,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_d516f655","line":72,"range":{"start_line":70,"start_character":0,"end_line":72,"end_character":62},"in_reply_to":"1fa4df85_94a21b4b","updated":"2020-03-18 18:14:14.000000000","message":"Ack.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    connection \u003d context.get_bind()"},{"line_number":68,"context_line":"    metadata \u003d sa.MetaData(bind\u003dconnection)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    consumer_types \u003d sa.Table(\u0027consumer_types\u0027, metadata, autoload\u003dTrue)"},{"line_number":71,"context_line":"    data \u003d {\u0027name\u0027: \"UNKNOWN\"}"},{"line_number":72,"context_line":"    connection.execute(sa.insert(consumer_types).values(data))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":75,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_703e302a","line":72,"range":{"start_line":70,"start_character":0,"end_line":72,"end_character":62},"in_reply_to":"1fa4df85_d516f655","updated":"2020-03-18 18:38:15.000000000","message":"note this implicitly generates the PK value \"1\" on MySQL but if for some reason the table already existed, then it wouldn\u0027t.  Since you seem to have the \"default_consumer_id\" hardcoded to \"1\" in a different file, it *might* be better to use value \"1\" explicitly here, if MySQL and SQLite are the only target databases.\n\nIf OTOH placement still has some notion of Postgresql somewhat working, then inserting the \"1\" explicitly would conflict with the sequence that PG associates with the SERIAL column and you\u0027d have to bump it explicitly.\n\nso a way to get the \"1\" in there using \"autoincrement\" and then making sure it\u0027s in fact \"1\" would be do to the INSERT without the primary key then SELECT from the table to make sure that\u0027s the only row.\n\notherwise if you can confirm you\u0027re only targeting MySQL / SQLite you can probably explicitly set that value of \"1\" here.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"ed7231531601b496a44a45bc50a2707d857814a8","unresolved":true,"context_lines":[{"line_number":47,"context_line":"    consumers \u003d sa.Table(\u0027consumers\u0027, metadata, autoload\u003dTrue)"},{"line_number":48,"context_line":"    fkey \u003d ForeignKeyConstraint(columns\u003d[consumers.c.consumer_type_id],"},{"line_number":49,"context_line":"                                refcolumns\u003d[consumer_types.c.id])"},{"line_number":50,"context_line":"    fkey.create()"},{"line_number":51,"context_line":"    op.create_index(\u0027consumers_consumer_type_id_idx\u0027, \u0027consumers\u0027,"},{"line_number":52,"context_line":"                    [\u0027consumer_type_id\u0027])"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"5ad220e9_5356672f","line":50,"updated":"2021-07-27 18:01:19.000000000","message":"How does this work, using migrate?  I\u0027ve never heard of this being done before and as migrate is super EOL you dont want to be adding dead code like this.     Assuming Migrate is recreating the entire table, then what you want to be using is batch_migrate for SQLite: https://alembic.sqlalchemy.org/en/latest/batch.html","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fd5d5505d1fea8d8254497d6c3157ecfab29b636","unresolved":true,"context_lines":[{"line_number":47,"context_line":"    consumers \u003d sa.Table(\u0027consumers\u0027, metadata, autoload\u003dTrue)"},{"line_number":48,"context_line":"    fkey \u003d ForeignKeyConstraint(columns\u003d[consumers.c.consumer_type_id],"},{"line_number":49,"context_line":"                                refcolumns\u003d[consumer_types.c.id])"},{"line_number":50,"context_line":"    fkey.create()"},{"line_number":51,"context_line":"    op.create_index(\u0027consumers_consumer_type_id_idx\u0027, \u0027consumers\u0027,"},{"line_number":52,"context_line":"                    [\u0027consumer_type_id\u0027])"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"6b206142_10553fd4","line":50,"in_reply_to":"5ad220e9_5356672f","updated":"2021-07-27 18:22:40.000000000","message":"TBH I don\u0027t know, sorry. I picked up this patch series from authors who\u0027ve moved on from openstack and I\u0027m doing my best to complete it. I will check into your recommended way to do this.","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"ed7231531601b496a44a45bc50a2707d857814a8","unresolved":true,"context_lines":[{"line_number":64,"context_line":"    )"},{"line_number":65,"context_line":"    connection \u003d context.get_bind()"},{"line_number":66,"context_line":"    metadata \u003d sa.MetaData(bind\u003dconnection)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":69,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"},{"line_number":70,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"8b7e7ad6_3a6aeabf","line":67,"updated":"2021-07-27 18:01:19.000000000","message":"...\n\n    with op.batch_alter_table(\"consumers\") as batch_op:\n        batch_op.add_column(\n            \"consumers\",\n            sa.Column(\n                \u0027consumer_type_id\u0027, sa.Integer(),\n                sa.ForeignKey(\u0027consumer_types.id\u0027),\n                nullable\u003dTrue\n            )\n\nif that doesn\u0027t work then we need to fix in alembic.","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fd5d5505d1fea8d8254497d6c3157ecfab29b636","unresolved":true,"context_lines":[{"line_number":64,"context_line":"    )"},{"line_number":65,"context_line":"    connection \u003d context.get_bind()"},{"line_number":66,"context_line":"    metadata \u003d sa.MetaData(bind\u003dconnection)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    if connection.engine.name \u003d\u003d \"sqlite\":"},{"line_number":69,"context_line":"        _add_column_for_sqlite(connection.engine, metadata)"},{"line_number":70,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":18,"id":"fac5d2a3_7613231a","line":67,"in_reply_to":"8b7e7ad6_3a6aeabf","updated":"2021-07-27 18:22:40.000000000","message":"Will try it.","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"c0cf2d896dc49223e8d007618eaf74cdace33f09","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u0027422ece571366\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027b5c396305c25\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"236abea7_78f6a766","line":26,"updated":"2021-07-30 14:59:38.000000000","message":"this is honestly and horribly old greek for me. I have no context about how to create a alembic migration script so we get a new revision ID that differs from the other ones, but I trust you. I just hope this isn\u0027t made by hand.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b7396a752e02f21bbb8772a630baa53132d3b7b8","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u0027422ece571366\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027b5c396305c25\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"82dd0135_010e3096","line":26,"in_reply_to":"236abea7_78f6a766","updated":"2021-07-30 22:20:33.000000000","message":"I didn\u0027t create this myself (Surya or Chris did) but indeed the IDs are not made by hand [1][2]. And this \"down_revision\" refers to the previous migration:\n\nhttps://github.com/openstack/placement/blob/master/placement/db/sqlalchemy/alembic/versions/b5c396305c25_block_on_null_consumer.py\n\n[1] https://docs.openstack.org/placement/latest/contributor/architecture.html#database-schema-changes\n[2] https://alembic.sqlalchemy.org/en/latest/tutorial.html#create-a-migration-script","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u0027422ece571366\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027b5c396305c25\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"631fe7ee_e588664e","line":26,"in_reply_to":"82dd0135_010e3096","updated":"2021-08-03 14:25:00.000000000","message":"Yup, I saw the contributor documentation for the alchemy scripts. Thanks.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"}],"placement/db/sqlalchemy/models.py":[{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":231,"context_line":"    user_id \u003d Column(Integer, nullable\u003dFalse)"},{"line_number":232,"context_line":"    generation \u003d Column(Integer, nullable\u003dFalse, server_default\u003d\"0\", default\u003d0)"},{"line_number":233,"context_line":"    consumer_type_id \u003d Column("},{"line_number":234,"context_line":"        Integer, ForeignKey(\u0027consumer_types.id\u0027), nullable\u003dTrue)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"class ConsumerType(BASE):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_50a774a8","line":234,"updated":"2020-03-18 18:38:15.000000000","message":"one thing you might not like about NULLABLE here is that if you want to load all the rows from \"consumers\" and their \"consumer_type\" at the same time, now you need to use a LEFT OUTER JOIN which less performant than an INNER JOIN.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":243,"context_line":"    )"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    id \u003d Column(Integer, primary_key\u003dTrue, nullable\u003dFalse, autoincrement\u003dTrue)"},{"line_number":246,"context_line":"    name \u003d Column(Unicode(255), nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":19,"id":"e4433932_7238c61b","line":246,"updated":"2021-08-03 14:25:00.000000000","message":"fwiw, no difference with the migration script.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"}],"placement/objects/allocation.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        allocs.c.used,"},{"line_number":291,"context_line":"        consumer.c.id.label(\"consumer_id\"),"},{"line_number":292,"context_line":"        consumer.c.generation.label(\"consumer_generation\"),"},{"line_number":293,"context_line":"        consumer.c.consumer_type_id.label(\"consumer_type_id\"),"},{"line_number":294,"context_line":"        sql.func.coalesce("},{"line_number":295,"context_line":"            consumer.c.uuid, allocs.c.consumer_id).label(\"consumer_uuid\"),"},{"line_number":296,"context_line":"        project.c.id.label(\"project_id\"),"}],"source_content_type":"text/x-python","patch_set":19,"id":"88a6df1b_33177a72","line":293,"updated":"2021-08-03 14:25:00.000000000","message":"FWIW, you don\u0027t need to use a label() as the table attribute name is the same.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d664f91e674c24f8d302662ec9b6b0f73f462ffa","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        allocs.c.used,"},{"line_number":291,"context_line":"        consumer.c.id.label(\"consumer_id\"),"},{"line_number":292,"context_line":"        consumer.c.generation.label(\"consumer_generation\"),"},{"line_number":293,"context_line":"        consumer.c.consumer_type_id.label(\"consumer_type_id\"),"},{"line_number":294,"context_line":"        sql.func.coalesce("},{"line_number":295,"context_line":"            consumer.c.uuid, allocs.c.consumer_id).label(\"consumer_uuid\"),"},{"line_number":296,"context_line":"        project.c.id.label(\"project_id\"),"}],"source_content_type":"text/x-python","patch_set":19,"id":"7c11c769_93097a1c","line":293,"in_reply_to":"88a6df1b_33177a72","updated":"2021-08-05 18:06:03.000000000","message":"Ah, I see, thanks.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":446,"context_line":"        context, id\u003ddb_first[\u0027consumer_id\u0027],"},{"line_number":447,"context_line":"        uuid\u003ddb_first[\u0027consumer_uuid\u0027],"},{"line_number":448,"context_line":"        generation\u003ddb_first[\u0027consumer_generation\u0027],"},{"line_number":449,"context_line":"        consumer_type_id\u003ddb_first[\u0027consumer_type_id\u0027],"},{"line_number":450,"context_line":"        project\u003dproject_obj.Project("},{"line_number":451,"context_line":"            context, id\u003ddb_first[\u0027project_id\u0027],"},{"line_number":452,"context_line":"            external_id\u003ddb_first[\u0027project_external_id\u0027]),"}],"source_content_type":"text/x-python","patch_set":19,"id":"86a5a389_c8938707","line":449,"updated":"2021-08-03 14:25:00.000000000","message":"see, this looks good.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"}],"placement/objects/consumer.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"dcc652cdfc434a63e0906787ada648ab276eac7d","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                # thing here because models.Consumer doesn\u0027t have a"},{"line_number":193,"context_line":"                # project_external_id or user_external_id attribute."},{"line_number":194,"context_line":"                self.id \u003d db_obj.id"},{"line_number":195,"context_line":"                self.generation \u003d db_obj.generation"},{"line_number":196,"context_line":"            except db_exc.DBDuplicateEntry:"},{"line_number":197,"context_line":"                raise exception.ConsumerExists(uuid\u003dself.uuid)"},{"line_number":198,"context_line":"        _create_in_db(self._context)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_0c9ec667","line":195,"updated":"2020-02-07 01:18:37.000000000","message":"Looks like the bug is here, we need:\n\n self.consumer_type_id \u003d db_obj.consumer_type_id\n\nif consumer_type_id was not specified and the database has populated it for us.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        target._context \u003d ctx"},{"line_number":177,"context_line":"        return target"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def __getattr__(self, attr):"},{"line_number":180,"context_line":"        # NOTE(melwitt): NULL is allowed in the database schema to support old"},{"line_number":181,"context_line":"        # consumer records. We translate the NULL value to the default consumer"},{"line_number":182,"context_line":"        # type when accessed in order to provide more friendly data to users."},{"line_number":183,"context_line":"        if attr \u003d\u003d \u0027consumer_type_id\u0027 and not self.consumer_type_id:"},{"line_number":184,"context_line":"            return consumer_type_obj.DEFAULT_CONSUMER_TYPE_ID"},{"line_number":185,"context_line":"        return getattr(self, attr)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @classmethod"},{"line_number":188,"context_line":"    def get_by_uuid(cls, ctx, uuid):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_f71a716d","line":185,"range":{"start_line":179,"start_character":0,"end_line":185,"end_character":34},"updated":"2020-03-12 12:20:21.000000000","message":"or this","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ef02a3b01ba9ca96c90dd0108894315b3122ed1d","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        target._context \u003d ctx"},{"line_number":177,"context_line":"        return target"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def __getattr__(self, attr):"},{"line_number":180,"context_line":"        # NOTE(melwitt): NULL is allowed in the database schema to support old"},{"line_number":181,"context_line":"        # consumer records. We translate the NULL value to the default consumer"},{"line_number":182,"context_line":"        # type when accessed in order to provide more friendly data to users."},{"line_number":183,"context_line":"        if attr \u003d\u003d \u0027consumer_type_id\u0027 and not self.consumer_type_id:"},{"line_number":184,"context_line":"            return consumer_type_obj.DEFAULT_CONSUMER_TYPE_ID"},{"line_number":185,"context_line":"        return getattr(self, attr)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @classmethod"},{"line_number":188,"context_line":"    def get_by_uuid(cls, ctx, uuid):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_3bf3b9a0","line":185,"range":{"start_line":179,"start_character":0,"end_line":185,"end_character":34},"in_reply_to":"1fa4df85_70d070f8","updated":"2020-03-18 19:14:22.000000000","message":"I\u0027m conflicted about what to do here. Originally this patch added the column as nullable\u003dFalse and specified a default value of UNKNOWN. Which would as you said, populate all existing rows with UNKNOWN.\n\nThen, we had this bug in nova:\n\nhttps://review.opendev.org/706351\n\nnot 100% the same issue because the nova bug involved a python-side default wherein sqlalchemy-migrate backfilled the default value itself during the database migration, which failed during a live/rolling upgrade as inserts were simultaneously happening to the table during the migration.\n\nSo in nova, the default was removed.\n\nThen, I got concerned about this change -- would there be a similar problem? Should we not specify a default here? I realize a server default is different than the python-side default from the nova case, but still with the server default the server would chug along filling in the default value for existing columns. Would that be harmful during a live/rolling upgrade? Or would it be OK? I unfortunately don\u0027t have enough knowledge to know for sure.\n\nTo me, it seemed best to avoid the server backfilling and instead either migrate ourselves in batches OR just do no migration and let NULL mean \"unknown\".\n\nDo you have any thoughts to lend about it?","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        target._context \u003d ctx"},{"line_number":177,"context_line":"        return target"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def __getattr__(self, attr):"},{"line_number":180,"context_line":"        # NOTE(melwitt): NULL is allowed in the database schema to support old"},{"line_number":181,"context_line":"        # consumer records. We translate the NULL value to the default consumer"},{"line_number":182,"context_line":"        # type when accessed in order to provide more friendly data to users."},{"line_number":183,"context_line":"        if attr \u003d\u003d \u0027consumer_type_id\u0027 and not self.consumer_type_id:"},{"line_number":184,"context_line":"            return consumer_type_obj.DEFAULT_CONSUMER_TYPE_ID"},{"line_number":185,"context_line":"        return getattr(self, attr)"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    @classmethod"},{"line_number":188,"context_line":"    def get_by_uuid(cls, ctx, uuid):"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_70d070f8","line":185,"range":{"start_line":179,"start_character":0,"end_line":185,"end_character":34},"in_reply_to":"1fa4df85_f71a716d","updated":"2020-03-18 18:38:15.000000000","message":"if the consumer_type_id column were NOT NULL, when you add that column in the migration you would also need to estalish a default value for it, which would populate all existing rows with UNKNOWN.  You still would not need this getattr.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":91,"context_line":"def _get_consumer_by_uuid(ctx, uuid):"},{"line_number":92,"context_line":"    # The SQL for this looks like the following:"},{"line_number":93,"context_line":"    # SELECT"},{"line_number":94,"context_line":"    #   c.id, c.uuid, c.consumer_type_id,"},{"line_number":95,"context_line":"    #   p.id AS project_id, p.external_id AS project_external_id,"},{"line_number":96,"context_line":"    #   u.id AS user_id, u.external_id AS user_external_id,"},{"line_number":97,"context_line":"    #   c.updated_at, c.created_at"}],"source_content_type":"text/x-python","patch_set":19,"id":"6889fbc8_19b0e668","line":94,"updated":"2021-08-03 14:25:00.000000000","message":"++","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":107,"context_line":"    cols \u003d ["},{"line_number":108,"context_line":"        consumers.c.id,"},{"line_number":109,"context_line":"        consumers.c.uuid,"},{"line_number":110,"context_line":"        consumers.c.consumer_type_id,"},{"line_number":111,"context_line":"        projects.c.id.label(\"project_id\"),"},{"line_number":112,"context_line":"        projects.c.external_id.label(\"project_external_id\"),"},{"line_number":113,"context_line":"        users.c.id.label(\"user_id\"),"}],"source_content_type":"text/x-python","patch_set":19,"id":"67b8ed24_65c35509","line":110,"updated":"2021-08-03 14:25:00.000000000","message":"see, here you don\u0027t use the label()","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d664f91e674c24f8d302662ec9b6b0f73f462ffa","unresolved":true,"context_lines":[{"line_number":107,"context_line":"    cols \u003d ["},{"line_number":108,"context_line":"        consumers.c.id,"},{"line_number":109,"context_line":"        consumers.c.uuid,"},{"line_number":110,"context_line":"        consumers.c.consumer_type_id,"},{"line_number":111,"context_line":"        projects.c.id.label(\"project_id\"),"},{"line_number":112,"context_line":"        projects.c.external_id.label(\"project_external_id\"),"},{"line_number":113,"context_line":"        users.c.id.label(\"user_id\"),"}],"source_content_type":"text/x-python","patch_set":19,"id":"28b3367e_e90aab67","line":110,"in_reply_to":"67b8ed24_65c35509","updated":"2021-08-05 18:06:03.000000000","message":"ack","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":152,"context_line":"        self.project \u003d project"},{"line_number":153,"context_line":"        self.user \u003d user"},{"line_number":154,"context_line":"        self.generation \u003d generation"},{"line_number":155,"context_line":"        self.consumer_type_id \u003d consumer_type_id"},{"line_number":156,"context_line":"        self.updated_at \u003d updated_at"},{"line_number":157,"context_line":"        self.created_at \u003d created_at"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"c6d9c238_e5f47442","line":155,"updated":"2021-08-03 14:25:00.000000000","message":"correctly named.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"}],"placement/objects/consumer_type.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE_ID \u003d 1"},{"line_number":24,"context_line":"# Used in tests and fixtures."},{"line_number":25,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027UNKNOWN\u0027"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_17186d73","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":28},"updated":"2020-03-12 12:20:21.000000000","message":"or this","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE_ID \u003d 1"},{"line_number":24,"context_line":"# Used in tests and fixtures."},{"line_number":25,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027UNKNOWN\u0027"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_50d534e4","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":28},"in_reply_to":"1fa4df85_17186d73","updated":"2020-03-18 18:38:15.000000000","message":"this is the hardcoded value of \"1\" I referred towards in the migration that is not explicit in that migration.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE_ID \u003d 1"},{"line_number":24,"context_line":"# Used in tests and fixtures."},{"line_number":25,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027UNKNOWN\u0027"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"@db_api.placement_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_b79959d1","line":25,"range":{"start_line":25,"start_character":25,"end_line":25,"end_character":32},"updated":"2020-03-12 12:20:21.000000000","message":"(*If* we don\u0027t want to have the DEFAULT_CONSUMER_TYPE_ID, \u0027unknown\u0027 is better than \u0027UNKNOWN\u0027.)","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"335c20da22743955fa2bd7eed8c1241b5b2a5205","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        consumer_types.c.created_at"},{"line_number":63,"context_line":"    ]"},{"line_number":64,"context_line":"    sel \u003d sa.select(cols).where(consumer_types.c.name \u003d\u003d name)"},{"line_number":65,"context_line":"    res \u003d ctx.session.execute(sel).fetchone()"},{"line_number":66,"context_line":"    if not res:"},{"line_number":67,"context_line":"        raise exception.ConsumerTypeNotFound(name\u003dname)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"6f913148_cb40f0d3","line":65,"range":{"start_line":65,"start_character":35,"end_line":65,"end_character":43},"updated":"2021-04-27 14:47:12.000000000","message":"name has a unique constraint so this is OK","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"ed7231531601b496a44a45bc50a2707d857814a8","unresolved":true,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_db import exception as db_exc"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from placement.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":18,"id":"a5509bcd_2153e644","line":13,"updated":"2021-07-27 18:01:19.000000000","message":"was it the original idea that SQL code wasn\u0027t supposed to be in \"objects\"?   did nova change this also?","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2045b467d427b0530c95342fbca2610063c819f6","unresolved":true,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_db import exception as db_exc"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from placement.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":18,"id":"20bc6f48_f680a8a5","line":13,"in_reply_to":"2daa3b4c_55ab4dd8","updated":"2021-07-27 19:04:06.000000000","message":"From what I see in https://github.com/openstack/placement/tree/master/placement/db/sqlalchemy, the model definitions and database migration stuff goes under there whereas all the SQL queries go under placement/objects. I think the likely thought was to keep all model definitions and migration logic together in order to be able to easily see a wholistic picture of the database layout when working with the schema itself.\n\nAnd then the SQL queries get organized by table essentially, maybe to keep things tidy as a single file for all queries would get really big (and is really big in nova).","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fd5d5505d1fea8d8254497d6c3157ecfab29b636","unresolved":true,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_db import exception as db_exc"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from placement.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":18,"id":"a5a19d2f_c25b7c5d","line":13,"in_reply_to":"a5509bcd_2153e644","updated":"2021-07-27 18:22:40.000000000","message":"Sorry, what do you mean? I\u0027m probably not understanding your question right but:\n\nWe have been using \"objects\" for pretty much all new/non-legacy database interactions. The old style is the nova/db/sqlalchemy/api.py which stays mostly unchanged save for bug fixes. It has also been acceptable to use the direct session.execute() style of queries rather than the ORM in nova. Placement has always preferred not to use the ORM.\n\nLet me know if I didn\u0027t understand your question and I\u0027ll try again to answer.","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"5c9568f6b9a8db65af0bca6d9f19f36ec1786fdd","unresolved":true,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_db import exception as db_exc"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from placement.db.sqlalchemy import models"}],"source_content_type":"text/x-python","patch_set":18,"id":"2daa3b4c_55ab4dd8","line":13,"in_reply_to":"a5a19d2f_c25b7c5d","updated":"2021-07-27 18:52:17.000000000","message":"that was my Q !    So what determines which logic goes into placement/db/sqlalchemy and which in placement/objects ?     objects is like service layer logic ?","commit_id":"9b8effb9bc5a6bafe47ed75f6d4964e09c53abe1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027unknown\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"@db_api.placement_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":19,"id":"a6f318df_8bba2b7e","line":23,"updated":"2021-08-03 14:25:00.000000000","message":"per the spec, the default is UNKNOWN. Should we do this here too as we only accept uppercases ? I don\u0027t see any other method uppercasing the name. For example, by https://review.opendev.org/c/openstack/placement/+/679441/24/placement/handlers/allocation.py#25 I see you directly set the name.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"beb970c2f9fbb6ac00ecf238e029d010bb76a447","unresolved":true,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027unknown\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"@db_api.placement_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":19,"id":"daca4705_5dfac74e","line":23,"in_reply_to":"a6f318df_8bba2b7e","updated":"2021-08-05 15:50:07.000000000","message":"As we discussed on IRC, the consensus was that the default is not used. Cool with me.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d664f91e674c24f8d302662ec9b6b0f73f462ffa","unresolved":true,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"DEFAULT_CONSUMER_TYPE \u003d \u0027unknown\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"@db_api.placement_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":19,"id":"7eb8955b_2180e5a3","line":23,"in_reply_to":"daca4705_5dfac74e","updated":"2021-08-05 18:06:03.000000000","message":"For completeness, summary of IRC convo is that the reproposed spec does not contain a default consumer type UNKNOWN but instead leaves NULL to indicate the absence of a type and this NULL is translated to a cosmetic label for the user as \"unknown\" (similar to the \"all\" cosmetic label). For the history of the change from the default consumer type \u003d\u003e allowing NULL, please see the earlier discussion comments on this review.\n\nThat said, I think this variable is ill-named and confusing given the design change and I need to rename it. I think gibi mentioned this on an earlier PS and I forgot to rename it.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09c95f6d4d62b0cd227b8cbd8f91ba83e7202c99","unresolved":true,"context_lines":[{"line_number":20,"context_line":"CONSUMER_TYPE_TBL \u003d models.ConsumerType.__table__"},{"line_number":21,"context_line":"_CONSUMER_TYPES_LOCK \u003d \u0027consumer_types_sync\u0027"},{"line_number":22,"context_line":"_CONSUMER_TYPES_SYNCED \u003d False"},{"line_number":23,"context_line":"NULL_CONSUMER_TYPE_ALIAS \u003d \u0027unknown\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"@db_api.placement_context_manager.reader"}],"source_content_type":"text/x-python","patch_set":20,"id":"fe574c6a_56021c96","line":23,"updated":"2021-08-06 08:07:53.000000000","message":"thanks for renaming it, appreciated.","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":33,"context_line":"    # WHERE c.id \u003d $id"},{"line_number":34,"context_line":"    consumer_types \u003d sa.alias(CONSUMER_TYPE_TBL, name\u003d\"c\")"},{"line_number":35,"context_line":"    cols \u003d ["},{"line_number":36,"context_line":"        consumer_types.c.id,"},{"line_number":37,"context_line":"        consumer_types.c.name,"},{"line_number":38,"context_line":"        consumer_types.c.updated_at,"},{"line_number":39,"context_line":"        consumer_types.c.created_at"}],"source_content_type":"text/x-python","patch_set":20,"id":"4373f3fe_17bc1cc9","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":24},"updated":"2021-08-19 17:13:12.000000000","message":"i would have been tempeteed to do somethign like\n\nct \u003d consumer_types.c\nthen jug have ct.id ectra here just to keep it shorter but this all looks fine to me based on the example sql above","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"@db_api.placement_context_manager.reader"},{"line_number":50,"context_line":"def _get_consumer_type_by_name(ctx, name):"},{"line_number":51,"context_line":"    # The SQL for this looks like the following:"},{"line_number":52,"context_line":"    # SELECT"},{"line_number":53,"context_line":"    #   c.id, c.name,"}],"source_content_type":"text/x-python","patch_set":20,"id":"a91415a7_123e3ef0","line":50,"range":{"start_line":50,"start_character":4,"end_line":50,"end_character":30},"updated":"2021-08-19 17:13:12.000000000","message":"i feel like most of this could be factored into a common function shared between this and by id that said its short so maybe its not worht it.","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/objects/usage.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":89,"context_line":"                       usage\u003ditem[1],"},{"line_number":90,"context_line":"                       consumer_type\u003d\"all\", consumer_count\u003ditem[2])"},{"line_number":91,"context_line":"                  for item in query.all()]"},{"line_number":92,"context_line":"    else:"},{"line_number":93,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":94,"context_line":"                       usage\u003ditem[1])"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_37965241","line":91,"updated":"2019-09-13 00:30:43.000000000","message":"Note to self: this is including the consumer_type and consumer_count key/values in the result if consumer_type\u003dTrue.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"11e80a08e037fcc6679478075e46ce036899f7f2","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        self.resource_class \u003d resource_class"},{"line_number":25,"context_line":"        self.usage \u003d int(usage)"},{"line_number":26,"context_line":"        self.consumer_type \u003d consumer_type"},{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_947fdbdc","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":49},"updated":"2020-01-24 07:43:34.000000000","message":"I don\u0027t think we\u0027d like to have this here, that said, this usage object is created and describes the total usages per resource class + consumer_type. We can calculate consumer count per resource class as this patch does, but what we want to return to users is not counts per resource class but total consumer counts per consumer type and that is not relevant to each resource class, right?","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1ecfdf938cb43148a92d4c51999070ddb7d97ea3","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        self.resource_class \u003d resource_class"},{"line_number":25,"context_line":"        self.usage \u003d int(usage)"},{"line_number":26,"context_line":"        self.consumer_type \u003d consumer_type"},{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"1fa4df85_27ee7722","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":49},"in_reply_to":"1fa4df85_59c12554","updated":"2020-02-27 02:53:00.000000000","message":"Thanks for adding the example. Sorry I did not follow your original concern.\n\nI keep confusing myself with this Usage class + consumer_count. :\\\n\nLooking at it again, the consumer_count is *not* related to the resource_class -- it is a count of how many consumers are of the consumer_type. The resource_class shows the usage per resource_class. So in your example, it is option B.\n\nI will add a code comment explaining the meaning of consumer_count.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"8e477ae68798023c25ab47ccce523aceb9d69359","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        self.resource_class \u003d resource_class"},{"line_number":25,"context_line":"        self.usage \u003d int(usage)"},{"line_number":26,"context_line":"        self.consumer_type \u003d consumer_type"},{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"1fa4df85_59c12554","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":49},"in_reply_to":"3fa7e38b_4a2de5fa","updated":"2020-02-26 08:52:18.000000000","message":"My concern was it was not obvious from here if \n\n    usage(resource_class\u003dVCPU,\n          usage\u003d10\n          consumer_type\u003dMIGRATION\n          consumer_count\u003d5)\n\nmeans \n\nA) there are 5 consumers of MIGRATION that use VCPU and the total is 10 (i.e. this counts only consumers that use VCPU. Every consumer in the five uses at least one VCPU)\n\nor \n\nB) there are 5 consumers of MIGRATION and the total usage of VCPU is 10 (there can be a consumer in the five that does not use VCPU at all)\n\nI am good with this data structure design, but I would at least leave a comment for this. (an alternative is to get another class that includes this usage class + consumer_type + consumer_counts to make this point clearer and do counting query completely out of that \"_get_by_consumer_type()\").","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4887476163ea5efe37e61e939491a60ae2835114","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        self.resource_class \u003d resource_class"},{"line_number":25,"context_line":"        self.usage \u003d int(usage)"},{"line_number":26,"context_line":"        self.consumer_type \u003d consumer_type"},{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_4a2de5fa","line":27,"range":{"start_line":27,"start_character":8,"end_line":27,"end_character":49},"in_reply_to":"3fa7e38b_947fdbdc","updated":"2020-01-31 01:17:19.000000000","message":"The context for consumer types is [from my perspective] mostly about being able to count usages to enforce quota on resources. Because of this, we do want to know amount of usage for a resource class + consumer type.\n\nConcrete example is, in nova, during a resize we want to count how much resource is being held by the pending resize (before user chooses confirm or revert). We can\u0027t do it today because we can\u0027t tell the difference between allocations of consumer type INSTANCE vs allocations of consumer type MIGRATION.\n\nToday, without consumer types, we experience the situation where we count resources on both source and destination during a resize. So if old flavor had 2 VCPU and new flavor has 4 VCPU our quota usage shows as 6 VCPU before user chooses confirm or revert.\n\nWith consumer types, we want to use it to let quota usage reflect only the absolute value of the delta. So in the same scenario, if we know that 2 VCPU are of consumer type MIGRATION and 4 VCPU are of consumer type INSTANCE, then we can reflect quota usage as only 4 VCPU before the user chooses confirm or revert.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"80a6f6f8fefce556c2c2b3316d13ef9a168c4f19","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"},{"line_number":31,"context_line":"    \"\"\"Get a list of Usage objects filtered by one resource provider.\"\"\""},{"line_number":32,"context_line":"    usage_list \u003d _get_all_by_resource_provider_uuid(context, rp_uuid)"},{"line_number":33,"context_line":"    return [Usage(**db_item) for db_item in usage_list]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def get_by_consumer_type(context, project_id, user_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_fbf23369","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":55},"updated":"2020-01-16 12:36:35.000000000","message":"This should have been mentioned in the spec so it maybe too late but why did we decide to not change the `GET /usages/\u003crp_uud\u003e` API as well?","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9b5c05e9aaa81001230b96df62ef51fa5193f5a3","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"},{"line_number":31,"context_line":"    \"\"\"Get a list of Usage objects filtered by one resource provider.\"\"\""},{"line_number":32,"context_line":"    usage_list \u003d _get_all_by_resource_provider_uuid(context, rp_uuid)"},{"line_number":33,"context_line":"    return [Usage(**db_item) for db_item in usage_list]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def get_by_consumer_type(context, project_id, user_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_386affeb","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":55},"in_reply_to":"3fa7e38b_8dde57d9","updated":"2020-01-17 06:03:24.000000000","message":"I see, thanks for linking that comment. I skimmed through the other comments as well and it appears it was an oversight to not also cover the \u0027/resource_providers/{uuid}/usages\u0027 API too.\n\nI agree with you, these two APIs should have the same response format and filtering capability.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"84ac641ff6e90942c8d73d55d648db07abb36cb5","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"},{"line_number":31,"context_line":"    \"\"\"Get a list of Usage objects filtered by one resource provider.\"\"\""},{"line_number":32,"context_line":"    usage_list \u003d _get_all_by_resource_provider_uuid(context, rp_uuid)"},{"line_number":33,"context_line":"    return [Usage(**db_item) for db_item in usage_list]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def get_by_consumer_type(context, project_id, user_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_8dde57d9","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":55},"in_reply_to":"3fa7e38b_ad3f93b0","updated":"2020-01-17 02:10:50.000000000","message":"\u003e There isn\u0027t a \u0027GET /usages/\u003crp_uuid\u003e\u0027 API but there is a \u0027GET\n \u003e /resource_providers/{uuid}/usages\u0027 API [1]. That is what you meant,\n \u003e right?\n\nThanks for correcting me. You are right.\n\n \u003e Let me spend some time looking at the spec discussion and\n \u003e things to see if I can find out why it was proposed this way by\n \u003e cdent and tssurya.\n \u003e \n \u003e [1] https://docs.openstack.org/api-ref/placement/#resource-provider-usages\n\nI also looked at the spec discussion and found that Eric has mentioned the \u0027GET /resource_providers/{uuid}/usages\u0027 API  [2] but it was not reflected on the spec later.\n\n[2] https://review.opendev.org/#/c/654799/4/doc/source/specs/train/approved/2005473-support-consumer-types.rst@84\n\nBefore 1.37, GET \u0027/usages\u0027 and \u0027/resource_providers/{uuid}/usages\u0027 had similar response format.  Therefore if one is changed in a microversion I expect the other is also changed in the same manner in the same microversion.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"da88b468c6171b7f4d27906df2ffbf7ceddf3c5b","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_all_by_resource_provider_uuid(context, rp_uuid):"},{"line_number":31,"context_line":"    \"\"\"Get a list of Usage objects filtered by one resource provider.\"\"\""},{"line_number":32,"context_line":"    usage_list \u003d _get_all_by_resource_provider_uuid(context, rp_uuid)"},{"line_number":33,"context_line":"    return [Usage(**db_item) for db_item in usage_list]"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def get_by_consumer_type(context, project_id, user_id\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_ad3f93b0","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":55},"in_reply_to":"3fa7e38b_fbf23369","updated":"2020-01-17 00:20:28.000000000","message":"There isn\u0027t a \u0027GET /usages/\u003crp_uuid\u003e\u0027 API but there is a \u0027GET /resource_providers/{uuid}/usages\u0027 API [1]. That is what you meant, right?\n\nOK, looking in the next patch, I see now that we are only proposing an update to \u0027get_total_usages\u0027 in placement/handlers/usage.py which is the \u0027GET /usages\u0027 API but no change to \u0027list_usages\u0027 which is the \u0027GET /resource_providers/{uuid}/usages\u0027 API.\n\nI understand your concern and I don\u0027t immediately know the reason for this. Let me spend some time looking at the spec discussion and things to see if I can find out why it was proposed this way by cdent and tssurya.\n\n[1] https://docs.openstack.org/api-ref/placement/#resource-provider-usages","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"80a6f6f8fefce556c2c2b3316d13ef9a168c4f19","unresolved":false,"context_lines":[{"line_number":126,"context_line":"    result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":127,"context_line":"                   usage\u003ditem[1], consumer_count\u003ditem[2],"},{"line_number":128,"context_line":"                   consumer_type\u003ditem[3])"},{"line_number":129,"context_line":"              for item in query.all()]"},{"line_number":130,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_e62ffa86","line":129,"updated":"2020-01-16 12:36:35.000000000","message":"Since these three internal functions have similar queries, I think we can refactor them in a follow up, but I don\u0027t block this for this reason.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a448a9ef0567f1ebb299de8e236b9b4b158bedad","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                             consumer_type\u003dFalse):"},{"line_number":75,"context_line":"    query \u003d (context.session.query(models.Allocation.resource_class_id,"},{"line_number":76,"context_line":"             func.coalesce(func.sum(models.Allocation.used), 0),"},{"line_number":77,"context_line":"             func.count(distinct(models.Allocation.consumer_id)))"},{"line_number":78,"context_line":"             .join(models.Consumer,"},{"line_number":79,"context_line":"                   models.Allocation.consumer_id \u003d\u003d models.Consumer.uuid)"},{"line_number":80,"context_line":"             .join(models.Project,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_b52f46f2","line":77,"range":{"start_line":77,"start_character":24,"end_line":77,"end_character":32},"updated":"2020-02-14 04:17:31.000000000","message":"This is needed in order to count unique consumer ids. When I did not use \u0027distinct\u0027, the count contained duplicate consumer ids.","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9b087d4df02e121bcdb224bae207a0c038d81f80","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        query \u003d query.join(models.User,"},{"line_number":85,"context_line":"                           models.Consumer.user_id \u003d\u003d models.User.id)"},{"line_number":86,"context_line":"        query \u003d query.filter(models.User.external_id \u003d\u003d user_id)"},{"line_number":87,"context_line":"    # NOTE(melwitt): We have to without grouping first in order to get a count"},{"line_number":88,"context_line":"    # of unique consumers. If we only count after grouping by resource class,"},{"line_number":89,"context_line":"    # we will count duplicate consumers for any unique consumer that consumes"},{"line_number":90,"context_line":"    # more than one resource class simultaneously (example: an instance"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_4c044819","line":87,"range":{"start_line":87,"start_character":31,"end_line":87,"end_character":32},"updated":"2020-02-14 15:43:37.000000000","message":"count","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"38a65aaaf1a4ee151eab08ca8204936e6ebbc264","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        query \u003d query.join(models.User,"},{"line_number":85,"context_line":"                           models.Consumer.user_id \u003d\u003d models.User.id)"},{"line_number":86,"context_line":"        query \u003d query.filter(models.User.external_id \u003d\u003d user_id)"},{"line_number":87,"context_line":"    # NOTE(melwitt): We have to without grouping first in order to get a count"},{"line_number":88,"context_line":"    # of unique consumers. If we only count after grouping by resource class,"},{"line_number":89,"context_line":"    # we will count duplicate consumers for any unique consumer that consumes"},{"line_number":90,"context_line":"    # more than one resource class simultaneously (example: an instance"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_12c18681","line":87,"range":{"start_line":87,"start_character":31,"end_line":87,"end_character":32},"in_reply_to":"3fa7e38b_4c044819","updated":"2020-02-14 15:59:34.000000000","message":"well you can\u0027t have an aggregate function in the columns clause, with other columns that are not aggregated, and not have a GROUP BY.  MySQL lets this happen in legacy mode but I thought we had STRICT mode turned on which would disallow this.     SQLite lets it happen because it\u0027s SQLite. PostgreSQL will not, nor will standard SQL.   the pattern means the dataase has to non-determinisically discard information that it otherwise would be returning in the rows and it doesn\u0027t want to do that.  it looks like you are only getting the count() column here so just use `query(func.count(distinct(Allocation.consumer_id)))`.","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a448a9ef0567f1ebb299de8e236b9b4b158bedad","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    # we will count duplicate consumers for any unique consumer that consumes"},{"line_number":90,"context_line":"    # more than one resource class simultaneously (example: an instance"},{"line_number":91,"context_line":"    # consuming both VCPU and MEMORY_MB)."},{"line_number":92,"context_line":"    unique_consumer_count \u003d query.all()[0][2]"},{"line_number":93,"context_line":"    query \u003d query.group_by(models.Allocation.resource_class_id)"},{"line_number":94,"context_line":"    if consumer_type:"},{"line_number":95,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_d5268222","line":92,"updated":"2020-02-14 04:17:31.000000000","message":"Is there a better way to do this? Will consult zzzeek for help.","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"77f8f4a47a06042740e957decdaef4b794c5ed51","unresolved":false,"context_lines":[{"line_number":133,"context_line":"    query \u003d (context.session.query("},{"line_number":134,"context_line":"             subq.c.resource_class_id,"},{"line_number":135,"context_line":"             func.coalesce(func.sum(subq.c.used), 0),"},{"line_number":136,"context_line":"             func.count(distinct(subq.c.consumer_id)),"},{"line_number":137,"context_line":"             subq.c.name))"},{"line_number":138,"context_line":"    # NOTE(melwitt): We have to count grouped by only consumer type first in"},{"line_number":139,"context_line":"    # order to get a count of unique consumers for a given consumer type. If we"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_15409aa2","line":136,"range":{"start_line":136,"start_character":24,"end_line":136,"end_character":32},"updated":"2020-02-14 04:20:50.000000000","message":"This is needed in order to count unique consumer ids. When I did not use \u0027distinct\u0027, the count contained duplicate consumer ids.","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"77f8f4a47a06042740e957decdaef4b794c5ed51","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    # class simultaneously (example: an instance consuming both VCPU and"},{"line_number":143,"context_line":"    # MEMORY_MB)."},{"line_number":144,"context_line":"    unique_consumer_counts \u003d {item[3]: item[2] for item in"},{"line_number":145,"context_line":"                              query.group_by(subq.c.name).all()}"},{"line_number":146,"context_line":"    query \u003d query.group_by(subq.c.resource_class_id,"},{"line_number":147,"context_line":"                           subq.c.name)"},{"line_number":148,"context_line":"    result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_95056a6a","line":145,"updated":"2020-02-14 04:20:50.000000000","message":"Is there a better way to do all of this (subquery above ^ etc)? Will consult zzzeek for help.","commit_id":"32d13b04825f0dd63784d168b1b106e42980df24"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":25,"context_line":"                 consumer_count\u003d0):"},{"line_number":26,"context_line":"        self.resource_class \u003d resource_class"},{"line_number":27,"context_line":"        self.usage \u003d int(usage)"},{"line_number":28,"context_line":"        self.consumer_type \u003d consumer_type"},{"line_number":29,"context_line":"        self.consumer_count \u003d int(consumer_count)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_776e81ca","line":28,"range":{"start_line":28,"start_character":29,"end_line":28,"end_character":42},"updated":"2020-03-12 12:20:21.000000000","message":"How about\n\n  consumer_type or consumer_type_obj.DEFAULT_CONSUMER_TYPE\n\nhere, instead of using coalesce for ID in DB layer? See below.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"b8860dd3acdfd4df4333cd27af56eee387ce4738","unresolved":false,"context_lines":[{"line_number":132,"context_line":"             models.ConsumerType.name)"},{"line_number":133,"context_line":"             .join(models.Consumer,"},{"line_number":134,"context_line":"                   models.Allocation.consumer_id \u003d\u003d models.Consumer.uuid)"},{"line_number":135,"context_line":"             .join(models.ConsumerType,"},{"line_number":136,"context_line":"                   # Consider NULL consumer_type_id as the default type ID"},{"line_number":137,"context_line":"                   func.coalesce(models.Consumer.consumer_type_id,"},{"line_number":138,"context_line":"                                 consumer_type_obj.DEFAULT_CONSUMER_TYPE_ID) \u003d\u003d"},{"line_number":139,"context_line":"                   models.ConsumerType.id)"},{"line_number":140,"context_line":"             .join(models.Project,"},{"line_number":141,"context_line":"                   models.Consumer.project_id \u003d\u003d models.Project.id)"},{"line_number":142,"context_line":"             .filter(models.Project.external_id \u003d\u003d project_id))"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_b74b995d","line":139,"range":{"start_line":135,"start_character":13,"end_line":139,"end_character":42},"updated":"2020-03-12 12:20:21.000000000","message":"Instead of coalesce\u0027ing,  can\u0027t we just use outer join? (with a comment that we need to convert it to inner join later, after online-migration code is ready and when we can set nullable\u003dFalse)\n\n  .outerjoin(models.ConsumerType,\n             models.Consumer.consumer_type_id \u003d\u003d models.ConsumerType.id)","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b02bda0166634b6f76b1fb75b16e442c4081cd62","unresolved":false,"context_lines":[{"line_number":132,"context_line":"             models.ConsumerType.name)"},{"line_number":133,"context_line":"             .join(models.Consumer,"},{"line_number":134,"context_line":"                   models.Allocation.consumer_id \u003d\u003d models.Consumer.uuid)"},{"line_number":135,"context_line":"             .join(models.ConsumerType,"},{"line_number":136,"context_line":"                   # Consider NULL consumer_type_id as the default type ID"},{"line_number":137,"context_line":"                   func.coalesce(models.Consumer.consumer_type_id,"},{"line_number":138,"context_line":"                                 consumer_type_obj.DEFAULT_CONSUMER_TYPE_ID) \u003d\u003d"},{"line_number":139,"context_line":"                   models.ConsumerType.id)"},{"line_number":140,"context_line":"             .join(models.Project,"},{"line_number":141,"context_line":"                   models.Consumer.project_id \u003d\u003d models.Project.id)"},{"line_number":142,"context_line":"             .filter(models.Project.external_id \u003d\u003d project_id))"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_70407036","line":139,"range":{"start_line":135,"start_character":13,"end_line":139,"end_character":42},"in_reply_to":"1fa4df85_b74b995d","updated":"2020-03-18 18:14:14.000000000","message":"Probably. Sorry my SQL ability is very poor.\n\nWell, there is a challenge with the idea of making it nullable\u003dFalse later. With old API microversion \u003c\u003d 1.36, the user can cause new consumers with NULL type id to be created. So we are never guaranteed to have zero NULL records.\n\nThis is why on the next patch I set the default consumer type id:\n\nhttps://review.opendev.org/#/c/679441/14/placement/handlers/util.py@179\n\nbecause otherwise we get in an endless cycle where new records with NULL can be created.\n\nThis might be the main reason why I kept the default UNKNOWN consumer type record with id 1.","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"335c20da22743955fa2bd7eed8c1241b5b2a5205","unresolved":true,"context_lines":[{"line_number":110,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":111,"context_line":"                       usage\u003ditem[1],"},{"line_number":112,"context_line":"                       consumer_type\u003d\"all\","},{"line_number":113,"context_line":"                       consumer_count\u003dunique_consumer_count)"},{"line_number":114,"context_line":"                  for item in query.all()]"},{"line_number":115,"context_line":"    else:"},{"line_number":116,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"}],"source_content_type":"text/x-python","patch_set":16,"id":"671b4ba9_3fb23570","line":113,"updated":"2021-04-27 14:47:12.000000000","message":"It is not clear to me what it really means. Is it the number of consumers allocating from the given resource_class? Or is this just the number of consumers the given project/user has regardless of what resource class those consumers are allocating from?","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de2fb9aa3cf8c8f2792fa257737a8ab791813860","unresolved":true,"context_lines":[{"line_number":110,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":111,"context_line":"                       usage\u003ditem[1],"},{"line_number":112,"context_line":"                       consumer_type\u003d\"all\","},{"line_number":113,"context_line":"                       consumer_count\u003dunique_consumer_count)"},{"line_number":114,"context_line":"                  for item in query.all()]"},{"line_number":115,"context_line":"    else:"},{"line_number":116,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"}],"source_content_type":"text/x-python","patch_set":16,"id":"532e2914_f507ff2f","line":113,"in_reply_to":"671b4ba9_3fb23570","updated":"2021-04-27 15:51:36.000000000","message":"I looked at the gabbit tests and now I see what is the intention here. So in a normal /usages query we group by consumer_type, but there are cases when we want to put everything into the same group \"all\" to get back the old /usages functionality. And this is what is implemented here. And the count here is just the total number of consumers for this project/user.","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":99,"context_line":"                  models.Allocation.consumer_id \u003d\u003d models.Consumer.uuid)"},{"line_number":100,"context_line":"            .join(models.Project,"},{"line_number":101,"context_line":"                  models.Consumer.project_id \u003d\u003d models.Project.id)"},{"line_number":102,"context_line":"            .filter(models.Project.external_id \u003d\u003d project_id))"},{"line_number":103,"context_line":"        if user_id:"},{"line_number":104,"context_line":"            count_query \u003d count_query.join("},{"line_number":105,"context_line":"                models.User, models.Consumer.user_id \u003d\u003d models.User.id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5cdb5390_b7e8dd51","line":102,"updated":"2021-08-03 14:25:00.000000000","message":"note to reviewers : above we get a query of how much of each consumer ID we get per project ID.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":104,"context_line":"            count_query \u003d count_query.join("},{"line_number":105,"context_line":"                models.User, models.Consumer.user_id \u003d\u003d models.User.id)"},{"line_number":106,"context_line":"            count_query \u003d count_query.filter("},{"line_number":107,"context_line":"                models.User.external_id \u003d\u003d user_id)"},{"line_number":108,"context_line":"        unique_consumer_count \u003d count_query.scalar()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"}],"source_content_type":"text/x-python","patch_set":19,"id":"bea6bf0b_1ec059b2","line":107,"updated":"2021-08-03 14:25:00.000000000","message":"and here we also get this count per user_id","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                models.User, models.Consumer.user_id \u003d\u003d models.User.id)"},{"line_number":106,"context_line":"            count_query \u003d count_query.filter("},{"line_number":107,"context_line":"                models.User.external_id \u003d\u003d user_id)"},{"line_number":108,"context_line":"        unique_consumer_count \u003d count_query.scalar()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        result \u003d [dict(resource_class\u003dcontext.rc_cache.string_from_id(item[0]),"},{"line_number":111,"context_line":"                       usage\u003ditem[1],"}],"source_content_type":"text/x-python","patch_set":19,"id":"05cd2703_77ca9fd7","line":108,"updated":"2021-08-03 14:25:00.000000000","message":"this makes sense, you just want all of them.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":159,"context_line":"                   usage\u003ditem[1],"},{"line_number":160,"context_line":"                   consumer_count\u003dunique_consumer_counts[item[3]],"},{"line_number":161,"context_line":"                   consumer_type\u003ditem[3])"},{"line_number":162,"context_line":"              for item in query.all()]"},{"line_number":163,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":19,"id":"3b828dc0_3fb836b5","line":162,"updated":"2021-08-03 14:25:00.000000000","message":"the above looks good to me.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    \"\"\"Get a list of Usage objects by consumer type.\"\"\""},{"line_number":42,"context_line":"    usage_list \u003d _get_by_consumer_type(context, project_id, user_id\u003duser_id,"},{"line_number":43,"context_line":"                                       consumer_type\u003dconsumer_type)"},{"line_number":44,"context_line":"    return [Usage(**db_item) for db_item in usage_list]"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def get_all_by_project_user(context, project_id, user_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":20,"id":"c4cf31b1_4babfc66","line":44,"range":{"start_line":44,"start_character":11,"end_line":44,"end_character":55},"updated":"2021-08-19 17:13:12.000000000","message":"it would be a change in the api but we might want to consider returning this as a generator expression instead eventually.\n\nbasiclaly yeild the Usage object to avoid egere evaaluation but this is inline with get_all_by_project_user so +1","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/tests/fixtures.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        ins \u003d consumer_type.CONSUMER_TYPE_TBL.insert()"},{"line_number":80,"context_line":"        args \u003d {\u0027name\u0027: consumer_type.DEFAULT_CONSUMER_TYPE, \u0027id\u0027: 1}"},{"line_number":81,"context_line":"        engine.execute(ins, args)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def cleanup(self):"},{"line_number":84,"context_line":"        trait._TRAITS_SYNCED \u003d False"},{"line_number":85,"context_line":"        resource_class._RESOURCE_CLASSES_SYNCED \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_3732d260","line":82,"updated":"2019-09-13 00:30:43.000000000","message":"So this means migrations are not run on every setUp() of the fixture the way they are in nova\u0027s fixture. I\u0027m curious as to why.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0bcfd6d7dc826791ac37f313118c872aab251bef","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        ins \u003d consumer_type.CONSUMER_TYPE_TBL.insert()"},{"line_number":80,"context_line":"        args \u003d {\u0027name\u0027: consumer_type.DEFAULT_CONSUMER_TYPE, \u0027id\u0027: 1}"},{"line_number":81,"context_line":"        engine.execute(ins, args)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def cleanup(self):"},{"line_number":84,"context_line":"        trait._TRAITS_SYNCED \u003d False"},{"line_number":85,"context_line":"        resource_class._RESOURCE_CLASSES_SYNCED \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_e56b1b1b","line":82,"in_reply_to":"5faad753_3732d260","updated":"2019-09-13 09:35:19.000000000","message":"It\u0027s not quite that, but it is weird. Yes, not all migrations are run on every test, nor are they in nova (it would be painfully slow), but the mechanism in nova and here is different (I\u0027m not 100% clear on the nova mechanism).\n\nThe way the AdHocDbFixture works is that it memorizes schemas and does something fancy with transactions such that after each test a roll back happens to the schema, but rows are a different matter.\n\nHowever this is a step that was done earlier in some experimentation so it may not matter any more. I\u0027ll check to make sure. But if it is required it is because the schema thing mentioned.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d398fd8b32a1bffe05bb920d0754a9f47ffceafa","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        ins \u003d consumer_type.CONSUMER_TYPE_TBL.insert()"},{"line_number":80,"context_line":"        args \u003d {\u0027name\u0027: consumer_type.DEFAULT_CONSUMER_TYPE, \u0027id\u0027: 1}"},{"line_number":81,"context_line":"        engine.execute(ins, args)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def cleanup(self):"},{"line_number":84,"context_line":"        trait._TRAITS_SYNCED \u003d False"},{"line_number":85,"context_line":"        resource_class._RESOURCE_CLASSES_SYNCED \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_e2f9c3c7","line":82,"in_reply_to":"5faad753_e56b1b1b","updated":"2019-09-13 19:41:08.000000000","message":"Oh, whoops. I mistook the application(?) of the schema on every test run [1] for running migrations [2] on every test run (which they indeed are not).\n\nI don\u0027t know much about this stuff but through local testing (back when I first wrote the CellDatabases fixture) I learned that it\u0027s the executescript call that actually adds all the tables to the sqlite database each test run. Without that, you get an empty sqlite database with no tables or anything in it.\n\nI had assumed the executescript call also populated any defaults added during migrations, but perhaps it doesn\u0027t? In nova I don\u0027t know of a case off the top of my head where we populate a default during a migration, so I don\u0027t have experience about whether those would get populated by the executescript call.\n\n[1] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/tests/fixtures.py#L696\n[2] https://github.com/openstack/nova/blob/33d3c74229f969c942df8dcda621eb93d8c4fb3a/nova/tests/fixtures.py#L682","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"80a6f6f8fefce556c2c2b3316d13ef9a168c4f19","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # but has been rolled back to being empty."},{"line_number":79,"context_line":"        ins \u003d consumer_type.CONSUMER_TYPE_TBL.insert()"},{"line_number":80,"context_line":"        args \u003d {\u0027name\u0027: consumer_type.DEFAULT_CONSUMER_TYPE, \u0027id\u0027: 1}"},{"line_number":81,"context_line":"        engine.execute(ins, args)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def cleanup(self):"},{"line_number":84,"context_line":"        trait._TRAITS_SYNCED \u003d False"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_fbe55308","line":81,"updated":"2020-01-16 12:36:35.000000000","message":"Another way to do this is to remove\n\n  migration.create_schema(engine)\n\n@L56 and set\n\n  conf_fixture.config(sync_on_startup\u003d\u0027True\u0027, group\u003d\u0027placement_database\u0027)\n\non the initialization.\n\nNote: This approach took 68 sec for \"tox -e functional\" while the existing one takes 62 sec.\n\nI\u0027m good with this approach also.","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"121a1368da07fc0f75fed02b6ddf93d8258cf281","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        # but has been rolled back to being empty."},{"line_number":79,"context_line":"        ins \u003d consumer_type.CONSUMER_TYPE_TBL.insert()"},{"line_number":80,"context_line":"        args \u003d {\u0027name\u0027: consumer_type.DEFAULT_CONSUMER_TYPE, \u0027id\u0027: 1}"},{"line_number":81,"context_line":"        engine.execute(ins, args)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def cleanup(self):"},{"line_number":84,"context_line":"        trait._TRAITS_SYNCED \u003d False"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_6ccada47","line":81,"in_reply_to":"3fa7e38b_fbe55308","updated":"2020-02-07 02:02:20.000000000","message":"Done","commit_id":"70aeec852b8b1bb9973cd77c20fb28ea6eaacf10"}],"placement/tests/functional/db/test_allocation.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"335c20da22743955fa2bd7eed8c1241b5b2a5205","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected default consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"ea2a3bfd_8be61418","line":185,"range":{"start_line":185,"start_character":50,"end_line":185,"end_character":57},"updated":"2021-04-27 14:47:12.000000000","message":"what does it mean that we have a _default_ consumer type?","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e0344f97c68726c1c362c30adb3418f5115819e5","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected default consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"938bd7d1_c82b55d3","line":185,"range":{"start_line":185,"start_character":50,"end_line":185,"end_character":57},"in_reply_to":"71aba697_a88a9a5c","updated":"2021-04-27 22:01:32.000000000","message":"I think this is residue from earlier patch versions. The tl;dr is the original proposal had a default consumer type \"UNKNOWN\" that had its own row in the consumer_types table and that all existing consumer rows would have their consumer_type_id column backfilled with this default value. After A Lot of review comment discussion, I changed it so there\u0027s no presence of a prescribed \"UNKNOWN\" type and instead an absence of a consumer_type_id (NULL) will mean that the type of the consumer is \u0027unknown\u0027 i.e. it has not been specified. This way avoids any data migration as the original data migration was maybe not too useful?","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f53838fbf670e0150397e60891716700aa9f3aba","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected default consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"d0becf71_46eb1f58","line":185,"range":{"start_line":185,"start_character":50,"end_line":185,"end_character":57},"in_reply_to":"938bd7d1_c82b55d3","updated":"2021-05-17 15:00:50.000000000","message":"Thanks. Then I\u0027m sure this is just a residue from previous patch sets. Now the consumer created with INSTANCE consumer type at L98 so we can simply drop the \"default\" word from this comment. Totally fine to do it in a follow up.","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"de2fb9aa3cf8c8f2792fa257737a8ab791813860","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected default consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"71aba697_a88a9a5c","line":185,"range":{"start_line":185,"start_character":50,"end_line":185,"end_character":57},"in_reply_to":"ea2a3bfd_8be61418","updated":"2021-04-27 15:51:36.000000000","message":"Maybe this wants to refer to the default UNKNOWN consumer type? But then this comment is not valid as this consumer is created with INSTANCE consumer type above.","commit_id":"9243b780041a6bda29a70572752ce8e046e18538"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":95,"context_line":"                             allocation_ratio\u003d1.5)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        # Create an INSTANCE consumer type"},{"line_number":98,"context_line":"        ct \u003d ct_obj.ConsumerType(self.ctx, name\u003d\u0027INSTANCE\u0027)"},{"line_number":99,"context_line":"        ct.create()"},{"line_number":100,"context_line":"        # Save consumer type id for later confirmation."},{"line_number":101,"context_line":"        ct_id \u003d ct.id"}],"source_content_type":"text/x-python","patch_set":19,"id":"098586b3_017e5d76","line":98,"updated":"2021-08-03 14:25:00.000000000","message":"see, you don\u0027t test what happens if you don\u0027t provide a name and you use the default one.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d664f91e674c24f8d302662ec9b6b0f73f462ffa","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected default consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"0e9e4569_c5dfe6f6","line":185,"updated":"2021-08-05 18:06:03.000000000","message":"This comment is (now) wrong and needs to be changed.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09c95f6d4d62b0cd227b8cbd8f91ba83e7202c99","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        self.assertEqual(2, len(consumer_allocs))"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # check the allocations have the expected INSTANCE consumer type"},{"line_number":186,"context_line":"        self.assertEqual(ct_id, consumer_allocs[0].consumer.consumer_type_id)"},{"line_number":187,"context_line":"        self.assertEqual(ct_id, consumer_allocs[1].consumer.consumer_type_id)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"12142d77_142cbf0f","line":185,"updated":"2021-08-06 08:07:53.000000000","message":"++","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/tests/functional/db/test_base.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e8ba51385f0bacd0eeea5b030a9be448276eec39","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        consumer \u003d consumer_obj.Consumer("},{"line_number":98,"context_line":"            ctx, uuid\u003dconsumer_id, user\u003duser, project\u003dproject)"},{"line_number":99,"context_line":"        consumer.create()"},{"line_number":100,"context_line":"        consumer \u003d consumer_obj.Consumer.get_by_uuid(ctx, consumer_id)"},{"line_number":101,"context_line":"    return consumer"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_dffe0eb4","line":100,"updated":"2020-02-07 03:12:32.000000000","message":"Accidental test change left in.","commit_id":"62159ce3c1bc8d56dd5465c3f1634ff882ef46d7"}],"placement/tests/functional/db/test_consumer_type.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":37,"context_line":"    def test_name_not_found(self):"},{"line_number":38,"context_line":"        self.assertRaises("},{"line_number":39,"context_line":"            exception.ConsumerTypeNotFound, ct_obj.ConsumerType.get_by_name,"},{"line_number":40,"context_line":"            self.context, \u0027LOSTPONY\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def test_duplicate_create(self):"},{"line_number":43,"context_line":"        ct \u003d ct_obj.ConsumerType(self.context, name\u003d\u0027MIGRATION\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"ab61fc95_9ca5f70a","line":40,"range":{"start_line":40,"start_character":27,"end_line":40,"end_character":35},"updated":"2021-08-03 14:25:00.000000000","message":"love this name.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":27,"context_line":"        self.assertEqual(ct.id, named_ct.id)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"        id_ct \u003d ct_obj.ConsumerType.get_by_id(self.context, ct.id)"},{"line_number":30,"context_line":"        self.assertEqual(ct.name, id_ct.name)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def test_id_not_found(self):"},{"line_number":33,"context_line":"        self.assertRaises("}],"source_content_type":"text/x-python","patch_set":20,"id":"f742edc9_1c223ff2","line":30,"updated":"2021-08-19 17:13:12.000000000","message":"+1","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":38,"context_line":"        self.assertRaises("},{"line_number":39,"context_line":"            exception.ConsumerTypeNotFound, ct_obj.ConsumerType.get_by_name,"},{"line_number":40,"context_line":"            self.context, \u0027LOSTPONY\u0027)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def test_duplicate_create(self):"},{"line_number":43,"context_line":"        ct \u003d ct_obj.ConsumerType(self.context, name\u003d\u0027MIGRATION\u0027)"},{"line_number":44,"context_line":"        ct.create()"}],"source_content_type":"text/x-python","patch_set":20,"id":"1d776197_9da2b178","line":41,"updated":"2021-08-19 17:13:12.000000000","message":"😊","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/tests/functional/db/test_migrations.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rows \u003d sql.select([consumers]).execute().fetchall()"},{"line_number":261,"context_line":"        self.assertEqual(1, len(rows))"},{"line_number":262,"context_line":"        consumer_type_ids \u003d [r[7] for r in rows]"},{"line_number":263,"context_line":"        self.assertEqual(1, consumer_type_ids[0])"},{"line_number":264,"context_line":"        # Check index and constraints"},{"line_number":265,"context_line":"        fkey \u003d insp.get_foreign_keys(\"consumers\")"},{"line_number":266,"context_line":"        self.assertEqual([\u0027consumer_type_id\u0027], fkey[0][\u0027constrained_columns\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_571f4eb6","line":263,"updated":"2019-09-13 00:30:43.000000000","message":"Should also verify that the UNKNOWN consumer type has name \u0027UNKNOWN\u0027? I see that test_usage.py is testing for the UNKNOWN type but that doesn\u0027t verify it was added by the migration.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rows \u003d sql.select([consumers]).execute().fetchall()"},{"line_number":261,"context_line":"        self.assertEqual(1, len(rows))"},{"line_number":262,"context_line":"        consumer_type_ids \u003d [r[7] for r in rows]"},{"line_number":263,"context_line":"        self.assertEqual(1, consumer_type_ids[0])"},{"line_number":264,"context_line":"        # Check index and constraints"},{"line_number":265,"context_line":"        fkey \u003d insp.get_foreign_keys(\"consumers\")"},{"line_number":266,"context_line":"        self.assertEqual([\u0027consumer_type_id\u0027], fkey[0][\u0027constrained_columns\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_ac505533","line":263,"in_reply_to":"5faad753_571f4eb6","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":231,"context_line":"    def test_consumer_types_422ece571366(self):"},{"line_number":232,"context_line":"        # Upgrade to populate the schema."},{"line_number":233,"context_line":"        self.migration_api.upgrade(\u0027422ece571366\u0027)"},{"line_number":234,"context_line":"        insp \u003d Inspector.from_engine(self.engine)"},{"line_number":235,"context_line":"        # Test creation of consumer_types table"},{"line_number":236,"context_line":"        con \u003d db_utils.get_table(self.engine, \u0027consumer_types\u0027)"},{"line_number":237,"context_line":"        col_names \u003d [column.name for column in con.c]"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_d05e0475","line":234,"updated":"2020-03-18 18:38:15.000000000","message":"please don\u0027t use inspector.from_engine().  this is deprecated in 1.4.   use \"from sqlalchemy import inspect;    insp \u003d inspect(self.engine)\"","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"f259ea03ecf69ff880fcc915a57d7a627dd8a8d8","unresolved":false,"context_lines":[{"line_number":253,"context_line":"    def test_consumer_type_id_column_422ece571366(self):"},{"line_number":254,"context_line":"        # Upgrade to populate the schema."},{"line_number":255,"context_line":"        self.migration_api.upgrade(\u0027422ece571366\u0027)"},{"line_number":256,"context_line":"        insp \u003d Inspector.from_engine(self.engine)"},{"line_number":257,"context_line":"        # Test creation of consumer_types table"},{"line_number":258,"context_line":"        consumers \u003d db_utils.get_table(self.engine, \u0027consumers\u0027)"},{"line_number":259,"context_line":"        col_names \u003d [column.name for column in consumers.c]"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_504a14ac","line":256,"updated":"2020-03-18 18:38:15.000000000","message":"same","commit_id":"ed1180f32959e63aa4b2564102f90011e47f6ed4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        }).execute().inserted_primary_key[0]"},{"line_number":228,"context_line":"        self.migration_api.upgrade(\u0027b5c396305c25\u0027)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    def test_consumer_types_422ece571366(self):"},{"line_number":231,"context_line":"        # Upgrade to populate the schema."},{"line_number":232,"context_line":"        self.migration_api.upgrade(\u0027422ece571366\u0027)"},{"line_number":233,"context_line":"        insp \u003d inspect(self.engine)"}],"source_content_type":"text/x-python","patch_set":20,"id":"dbe198b6_54e02572","line":230,"range":{"start_line":230,"start_character":28,"end_line":230,"end_character":40},"updated":"2021-08-19 17:13:12.000000000","message":"interesting we use the sha in the test name makes sense.","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}],"placement/tests/functional/db/test_usage.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        inv \u003d inv_obj.Inventory(resource_provider\u003ddb_rp,"},{"line_number":71,"context_line":"                                resource_class\u003dorc.DISK_GB,"},{"line_number":72,"context_line":"                                total\u003d1024)"},{"line_number":73,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":76,"context_line":"            self.ctx, self.project_obj.external_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_17647649","line":73,"updated":"2019-09-13 00:30:43.000000000","message":"Note to self: _make_allocation sets some inventory and creates an allocation and the allocation \u0027used\u0027 is 2 and the inventory total is 200. Then the second inventory set changes the total to 1024.\n\nNot sure why the total is being changed? Doesn\u0027t seem to be used in the test.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        inv \u003d inv_obj.Inventory(resource_provider\u003ddb_rp,"},{"line_number":71,"context_line":"                                resource_class\u003dorc.DISK_GB,"},{"line_number":72,"context_line":"                                total\u003d1024)"},{"line_number":73,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":76,"context_line":"            self.ctx, self.project_obj.external_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_4c43e16c","line":73,"in_reply_to":"5faad753_055af781","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0bcfd6d7dc826791ac37f313118c872aab251bef","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        inv \u003d inv_obj.Inventory(resource_provider\u003ddb_rp,"},{"line_number":71,"context_line":"                                resource_class\u003dorc.DISK_GB,"},{"line_number":72,"context_line":"                                total\u003d1024)"},{"line_number":73,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":76,"context_line":"            self.ctx, self.project_obj.external_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_055af781","line":73,"in_reply_to":"5faad753_17647649","updated":"2019-09-13 09:35:19.000000000","message":"probably a pasto","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        inv \u003d inv_obj.Inventory(resource_provider\u003ddb_rp,"},{"line_number":88,"context_line":"                                resource_class\u003dorc.DISK_GB,"},{"line_number":89,"context_line":"                                total\u003d1024)"},{"line_number":90,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":93,"context_line":"            self.ctx, self.project_obj.external_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_17f23671","line":90,"updated":"2019-09-13 00:30:43.000000000","message":"Same.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        inv \u003d inv_obj.Inventory(resource_provider\u003ddb_rp,"},{"line_number":88,"context_line":"                                resource_class\u003dorc.DISK_GB,"},{"line_number":89,"context_line":"                                total\u003d1024)"},{"line_number":90,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":93,"context_line":"            self.ctx, self.project_obj.external_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_0c616905","line":90,"in_reply_to":"5faad753_17f23671","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":93,"context_line":"            self.ctx, self.project_obj.external_id,"},{"line_number":94,"context_line":"            consumer_type\u003dct_obj.DEFAULT_CONSUMER_TYPE)"},{"line_number":95,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":96,"context_line":"        usage \u003d usages[0]"},{"line_number":97,"context_line":"        self.assertEqual(ct_obj.DEFAULT_CONSUMER_TYPE, usage.consumer_type)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_b702423c","line":94,"updated":"2019-09-13 00:30:43.000000000","message":"It doesn\u0027t seem like this is providing any test coverage beyond test_get_by_unspecified_consumer_type? Only the UNKNOWN consumer type has an allocation, so even if consumer_type wasn\u0027t filtering correctly, you\u0027d get the expected UNKNOWN usage back. I would think you would want to filter on a different type and assert you get no usage back instead.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":93,"context_line":"            self.ctx, self.project_obj.external_id,"},{"line_number":94,"context_line":"            consumer_type\u003dct_obj.DEFAULT_CONSUMER_TYPE)"},{"line_number":95,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":96,"context_line":"        usage \u003d usages[0]"},{"line_number":97,"context_line":"        self.assertEqual(ct_obj.DEFAULT_CONSUMER_TYPE, usage.consumer_type)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_ec516d31","line":94,"in_reply_to":"5faad753_b702423c","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":111,"context_line":"            self.ctx, self.project_obj.external_id,"},{"line_number":112,"context_line":"            user_id\u003dself.user_obj.external_id,"},{"line_number":113,"context_line":"            consumer_type\u003dct_obj.DEFAULT_CONSUMER_TYPE)"},{"line_number":114,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":115,"context_line":"        usage \u003d usages[0]"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_d79c9e0f","line":112,"updated":"2019-09-13 00:30:43.000000000","message":"Similar comment here, if self.user_obj.external_id is the default for creating the allocation in the tests, filtering (or not filtering) on the default will always match.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":111,"context_line":"            self.ctx, self.project_obj.external_id,"},{"line_number":112,"context_line":"            user_id\u003dself.user_obj.external_id,"},{"line_number":113,"context_line":"            consumer_type\u003dct_obj.DEFAULT_CONSUMER_TYPE)"},{"line_number":114,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":115,"context_line":"        usage \u003d usages[0]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_8c32390f","line":112,"in_reply_to":"5faad753_d79c9e0f","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d1a686ec8cdce18563035e8aadc56bc1f5021dd6","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":130,"context_line":"            self.ctx, self.project_obj.external_id, consumer_type\u003d\u0027all\u0027)"},{"line_number":131,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":132,"context_line":"        usage \u003d usages[0]"},{"line_number":133,"context_line":"        self.assertEqual(\u0027all\u0027, usage.consumer_type)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5faad753_17c0f6e0","line":130,"updated":"2019-09-13 00:30:43.000000000","message":"I would think this test would want to include at least one additional allocation with a different consumer type to show that both are returned when \u0027all\u0027 is specified.","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"834003b393df4a68ab28ea22522253cafa8eaf67","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        db_rp.set_inventory([inv])"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        usages \u003d usage_obj.get_by_consumer_type("},{"line_number":130,"context_line":"            self.ctx, self.project_obj.external_id, consumer_type\u003d\u0027all\u0027)"},{"line_number":131,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":132,"context_line":"        usage \u003d usages[0]"},{"line_number":133,"context_line":"        self.assertEqual(\u0027all\u0027, usage.consumer_type)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_ac35b5f8","line":130,"in_reply_to":"5faad753_17c0f6e0","updated":"2019-10-16 20:44:06.000000000","message":"Done","commit_id":"7e12ce1d1dacd1e2858a4c3f1801119c6964efc1"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"5bd866389425845e8aba71bc7351badcfe4f077c","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            self.ctx, self.project_obj.external_id)"},{"line_number":80,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":81,"context_line":"        usage \u003d usages[0]"},{"line_number":82,"context_line":"        self.assertEqual(\u0027unknown\u0027, usage.consumer_type)"},{"line_number":83,"context_line":"        self.assertEqual(1, usage.consumer_count)"},{"line_number":84,"context_line":"        self.assertEqual(orc.DISK_GB, usage.resource_class)"},{"line_number":85,"context_line":"        self.assertEqual(2, usage.usage)"}],"source_content_type":"text/x-python","patch_set":19,"id":"22c182f9_bb4092ce","line":82,"range":{"start_line":82,"start_character":26,"end_line":82,"end_character":33},"updated":"2021-08-03 14:25:00.000000000","message":"this is weird, right?","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d664f91e674c24f8d302662ec9b6b0f73f462ffa","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            self.ctx, self.project_obj.external_id)"},{"line_number":80,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":81,"context_line":"        usage \u003d usages[0]"},{"line_number":82,"context_line":"        self.assertEqual(\u0027unknown\u0027, usage.consumer_type)"},{"line_number":83,"context_line":"        self.assertEqual(1, usage.consumer_count)"},{"line_number":84,"context_line":"        self.assertEqual(orc.DISK_GB, usage.resource_class)"},{"line_number":85,"context_line":"        self.assertEqual(2, usage.usage)"}],"source_content_type":"text/x-python","patch_set":19,"id":"2bffc7c2_3171061c","line":82,"range":{"start_line":82,"start_character":26,"end_line":82,"end_character":33},"in_reply_to":"22c182f9_bb4092ce","updated":"2021-08-05 18:06:03.000000000","message":"Yeah ... the alternative here is applying the \"unknown\" cosmetic label higher up the stack at the API handler. I think my reasoning for putting it into the Usage object is because that\u0027s the layer where the \"all\" cosmetic label is handled:\n\nhttps://review.opendev.org/c/openstack/placement/+/669170/19/placement/objects/usage.py","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09c95f6d4d62b0cd227b8cbd8f91ba83e7202c99","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            self.ctx, self.project_obj.external_id)"},{"line_number":80,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":81,"context_line":"        usage \u003d usages[0]"},{"line_number":82,"context_line":"        self.assertEqual(\u0027unknown\u0027, usage.consumer_type)"},{"line_number":83,"context_line":"        self.assertEqual(1, usage.consumer_count)"},{"line_number":84,"context_line":"        self.assertEqual(orc.DISK_GB, usage.resource_class)"},{"line_number":85,"context_line":"        self.assertEqual(2, usage.usage)"}],"source_content_type":"text/x-python","patch_set":19,"id":"c70497cd_1907f817","line":82,"range":{"start_line":82,"start_character":26,"end_line":82,"end_character":33},"in_reply_to":"2bffc7c2_3171061c","updated":"2021-08-06 08:07:53.000000000","message":"Yeah, I understood the reasoning, fair enough.","commit_id":"c3554d504f732f4dfbe37f7f2ead9e4a7d81ba34"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"09c95f6d4d62b0cd227b8cbd8f91ba83e7202c99","unresolved":true,"context_lines":[{"line_number":79,"context_line":"            self.ctx, self.project_obj.external_id)"},{"line_number":80,"context_line":"        self.assertEqual(1, len(usages))"},{"line_number":81,"context_line":"        usage \u003d usages[0]"},{"line_number":82,"context_line":"        self.assertEqual(\u0027unknown\u0027, usage.consumer_type)"},{"line_number":83,"context_line":"        self.assertEqual(1, usage.consumer_count)"},{"line_number":84,"context_line":"        self.assertEqual(orc.DISK_GB, usage.resource_class)"},{"line_number":85,"context_line":"        self.assertEqual(2, usage.usage)"}],"source_content_type":"text/x-python","patch_set":20,"id":"f4e7db5f_cbec9caa","line":82,"range":{"start_line":82,"start_character":25,"end_line":82,"end_character":34},"updated":"2021-08-06 08:07:53.000000000","message":"femto-nit : You could have asserted the consumer_type to be NULL_CONSUMER_TYPE_ALIAS","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3bb9dc2bfbcaa6f21d4415e8d420422db517bd01","unresolved":true,"context_lines":[{"line_number":83,"context_line":"        self.assertEqual(1, usage.consumer_count)"},{"line_number":84,"context_line":"        self.assertEqual(orc.DISK_GB, usage.resource_class)"},{"line_number":85,"context_line":"        self.assertEqual(2, usage.usage)"},{"line_number":86,"context_line":"        # Verify we get nothing back if we filter on a different project"},{"line_number":87,"context_line":"        # external_id that does not exist (will not work if filtering is"},{"line_number":88,"context_line":"        # broken)"},{"line_number":89,"context_line":"        usages \u003d usage_obj.get_by_consumer_type(self.ctx, \u0027BOGUS\u0027)"},{"line_number":90,"context_line":"        self.assertEqual(0, len(usages))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_get_by_specified_consumer_type(self):"}],"source_content_type":"text/x-python","patch_set":20,"id":"21828c3c_7f2c2bff","line":89,"range":{"start_line":86,"start_character":8,"end_line":89,"end_character":66},"updated":"2021-08-19 17:13:12.000000000","message":"at the api level i would expect this to raise a 400 or a 404 if you tried this on a non list endpoint but for a list endpoint i gues ya an empty list with a 200 respoce code is what i woudl expect so i guess this makes sense.","commit_id":"b1f3dd39c3fd140ea77fba474c8a3a118a41de8c"}]}