)]}'
{"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e9a123846d0cc9f6a046a764b202583f07afc3e6","unresolved":false,"context_lines":[{"line_number":2015,"context_line":"    for rec in recs:"},{"line_number":2016,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":2017,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":2018,"context_line":"                project_id\u003dNone, user_id\u003dNone, request_generation\u003dNone)"},{"line_number":2019,"context_line":"    return recs"},{"line_number":2020,"context_line":""},{"line_number":2021,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_fd456418","line":2018,"range":{"start_line":2018,"start_character":16,"end_line":2018,"end_character":45},"updated":"2018-04-17 19:34:09.000000000","message":"Is this saying that, if there\u0027s no consumer record, we\u0027ll create it with null project/user IDs, and the result will end up with Nones; but if there *is* a record, we\u0027ll find it and the resulting project and user IDs will be populated?  This is confusing enough to warrant a comment.","commit_id":"ad6188f456fc9f966998ed9981342b838b743c16"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0a72c1943dd9683caa5d143345300a7b15cfa6ab","unresolved":false,"context_lines":[{"line_number":2015,"context_line":"    for rec in recs:"},{"line_number":2016,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":2017,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":2018,"context_line":"                project_id\u003dNone, user_id\u003dNone, request_generation\u003dNone)"},{"line_number":2019,"context_line":"    return recs"},{"line_number":2020,"context_line":""},{"line_number":2021,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f6a8fd7_fdeb24ad","line":2018,"range":{"start_line":2018,"start_character":16,"end_line":2018,"end_character":45},"in_reply_to":"9f6a8fd7_fd456418","updated":"2018-04-17 19:46:35.000000000","message":"Oh, I see - this was left over from one of the dead ends I went down where the project_id and user_id were removed from the Allocation object, since it now has a Consumer object that has that information. When that proved too hairy to change, I dropped that approach. I\u0027ll fix this to get the project_id and user_id from the allocation record.","commit_id":"ad6188f456fc9f966998ed9981342b838b743c16"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bc65dde5fd7f33210b61c6f5f338e8c15c0114da","unresolved":false,"context_lines":[{"line_number":1728,"context_line":"    fields \u003d {"},{"line_number":1729,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":1730,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":1731,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1732,"context_line":"        \u0027consumer\u0027: fields.ObjectField(\u0027Consumer\u0027, nullable\u003dTrue),"},{"line_number":1733,"context_line":"        \u0027resource_class\u0027: rc_fields.ResourceClassField(),"},{"line_number":1734,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1735,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1736,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1737,"context_line":"        # required."},{"line_number":1738,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1739,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1740,"context_line":"    }"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_a4bb39c2","line":1739,"range":{"start_line":1731,"start_character":0,"end_line":1739,"end_character":53},"updated":"2018-04-18 22:27:26.000000000","message":"I\u0027m going to regret asking this, but...\n\nWhy can\u0027t we get rid of consumer_id, project_id, and user_id?","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e0b46442d81a5efbe0864a71c577a73e26ebf9b4","unresolved":false,"context_lines":[{"line_number":1728,"context_line":"    fields \u003d {"},{"line_number":1729,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":1730,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":1731,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1732,"context_line":"        \u0027consumer\u0027: fields.ObjectField(\u0027Consumer\u0027, nullable\u003dTrue),"},{"line_number":1733,"context_line":"        \u0027resource_class\u0027: rc_fields.ResourceClassField(),"},{"line_number":1734,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1735,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1736,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1737,"context_line":"        # required."},{"line_number":1738,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1739,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1740,"context_line":"    }"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_ee7235ad","line":1739,"range":{"start_line":1731,"start_character":0,"end_line":1739,"end_character":53},"in_reply_to":"9f6a8fd7_678d0c7a","updated":"2018-04-20 18:35:45.000000000","message":"I\u0027d strongly prefer to do it as a follow-up patch. Changing this stuff at the same time we\u0027re changing how it all works would be far too messy.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d4b160e96537209a387004d276a75270e19841a8","unresolved":false,"context_lines":[{"line_number":1728,"context_line":"    fields \u003d {"},{"line_number":1729,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":1730,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":1731,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1732,"context_line":"        \u0027consumer\u0027: fields.ObjectField(\u0027Consumer\u0027, nullable\u003dTrue),"},{"line_number":1733,"context_line":"        \u0027resource_class\u0027: rc_fields.ResourceClassField(),"},{"line_number":1734,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1735,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1736,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1737,"context_line":"        # required."},{"line_number":1738,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1739,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1740,"context_line":"    }"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_678d0c7a","line":1739,"range":{"start_line":1731,"start_character":0,"end_line":1739,"end_character":53},"in_reply_to":"9f6a8fd7_9c3af331","updated":"2018-04-19 16:56:24.000000000","message":"\u003e All the existing\n \u003e code and tests assume that creating an Allocation requires a\n \u003e consumer_(uu)id. So, either all that code has to change to create a\n \u003e Consumer object,\n\nI thought there was going to be some other esoteric reason like versioning.  But if it\u0027s just because we have to change all the references to Allocation, IMO that\u0027s the right thing to do.  Having the consumer, project, and user ID fields duplicated in the object feels wrong, and makes it hard to reason about which gets used where (like in the test case I complained about).","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a17411c24a3f92ba2c532487ab9ab4a51e006f8d","unresolved":false,"context_lines":[{"line_number":1728,"context_line":"    fields \u003d {"},{"line_number":1729,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":1730,"context_line":"        \u0027resource_provider\u0027: fields.ObjectField(\u0027ResourceProvider\u0027),"},{"line_number":1731,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1732,"context_line":"        \u0027consumer\u0027: fields.ObjectField(\u0027Consumer\u0027, nullable\u003dTrue),"},{"line_number":1733,"context_line":"        \u0027resource_class\u0027: rc_fields.ResourceClassField(),"},{"line_number":1734,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1735,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1736,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1737,"context_line":"        # required."},{"line_number":1738,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1739,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1740,"context_line":"    }"},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_9c3af331","line":1739,"range":{"start_line":1731,"start_character":0,"end_line":1739,"end_character":53},"in_reply_to":"9f6a8fd7_a4bb39c2","updated":"2018-04-19 16:38:04.000000000","message":"Heh, that was one of the dead ends I went down. All the existing code and tests assume that creating an Allocation requires a consumer_(uu)id. So, either all that code has to change to create a Consumer object, or we need to do that in the __init__() of the Allocation object.\n\nPerhaps some day we can do that change, but just adding the consumer field turned out to be the least disruptive.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bc65dde5fd7f33210b61c6f5f338e8c15c0114da","unresolved":false,"context_lines":[{"line_number":1975,"context_line":"    recs \u003d [dict(r) for r in ctx.session.execute(sel)]"},{"line_number":1976,"context_line":"    for rec in recs:"},{"line_number":1977,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":1978,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":1979,"context_line":"                request_generation\u003dNone)"},{"line_number":1980,"context_line":"    return recs"},{"line_number":1981,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_a4a499ba","line":1978,"range":{"start_line":1978,"start_character":35,"end_line":1978,"end_character":48},"updated":"2018-04-18 22:27:26.000000000","message":"This should only create the consumer record if the allocation record was an \"old\" (pre-1.8) one, yah?  In which case it\u0027s appropriate that the proj/user IDs will be null in the new record, which is what this will do.\n\n(A comment to that effect would be neat)","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a17411c24a3f92ba2c532487ab9ab4a51e006f8d","unresolved":false,"context_lines":[{"line_number":1975,"context_line":"    recs \u003d [dict(r) for r in ctx.session.execute(sel)]"},{"line_number":1976,"context_line":"    for rec in recs:"},{"line_number":1977,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":1978,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":1979,"context_line":"                request_generation\u003dNone)"},{"line_number":1980,"context_line":"    return recs"},{"line_number":1981,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_dcd4ebbf","line":1978,"range":{"start_line":1978,"start_character":35,"end_line":1978,"end_character":48},"in_reply_to":"9f6a8fd7_a4a499ba","updated":"2018-04-19 16:38:04.000000000","message":"No, the record will be created irrespective of the version. We always want the consumer record created.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e0b46442d81a5efbe0864a71c577a73e26ebf9b4","unresolved":false,"context_lines":[{"line_number":1975,"context_line":"    recs \u003d [dict(r) for r in ctx.session.execute(sel)]"},{"line_number":1976,"context_line":"    for rec in recs:"},{"line_number":1977,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":1978,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":1979,"context_line":"                request_generation\u003dNone)"},{"line_number":1980,"context_line":"    return recs"},{"line_number":1981,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_0e01a905","line":1978,"range":{"start_line":1978,"start_character":35,"end_line":1978,"end_character":48},"in_reply_to":"9f6a8fd7_a7c1e482","updated":"2018-04-20 18:35:45.000000000","message":"Yes, that\u0027s correct.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d4b160e96537209a387004d276a75270e19841a8","unresolved":false,"context_lines":[{"line_number":1975,"context_line":"    recs \u003d [dict(r) for r in ctx.session.execute(sel)]"},{"line_number":1976,"context_line":"    for rec in recs:"},{"line_number":1977,"context_line":"        # TODO(edleafe): change this to use the generation from the request"},{"line_number":1978,"context_line":"        rec[\"consumer\"] \u003d Consumer.get_or_create(ctx, rec[\"consumer_id\"],"},{"line_number":1979,"context_line":"                request_generation\u003dNone)"},{"line_number":1980,"context_line":"    return recs"},{"line_number":1981,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_a7c1e482","line":1978,"range":{"start_line":1978,"start_character":35,"end_line":1978,"end_character":48},"in_reply_to":"9f6a8fd7_dcd4ebbf","updated":"2018-04-19 16:56:24.000000000","message":"Sorry, I wasn\u0027t clear.  I\u0027m confirming my understanding, which is:\n\n- If the record existed beforehand, this will pull it out, with whatever proj/user ID was in the database.\n- But it\u0027s possible the record did *not* exist beforehand, if the allocation was created at a pre-1.8 microversion. In that case, this line will create it - but with null proj/user IDs, which is appropriate because the allocation was created pre-1.8 and therefore lacked proj/user IDs.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"}],"nova/tests/functional/api/openstack/placement/db/test_resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bc65dde5fd7f33210b61c6f5f338e8c15c0114da","unresolved":false,"context_lines":[{"line_number":1701,"context_line":""},{"line_number":1702,"context_line":"    def test_create_all_with_project_user(self):"},{"line_number":1703,"context_line":"        consumer_uuid \u003d uuidsentinel.consumer"},{"line_number":1704,"context_line":"        consumer \u003d rp_obj.Consumer.get_or_create(self.ctx, consumer_uuid,"},{"line_number":1705,"context_line":"                project_id\u003dself.ctx.project_id, user_id\u003dself.ctx.user_id)"},{"line_number":1706,"context_line":"        rp_class \u003d fields.ResourceClass.DISK_GB"},{"line_number":1707,"context_line":"        rp \u003d self._make_rp_and_inventory(resource_class\u003drp_class,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_84b875c8","line":1704,"updated":"2018-04-18 22:27:26.000000000","message":"It looks like all this is doing is ensuring that passing the new args to the Allocation init doesn\u0027t blow up.  Shouldn\u0027t we also be ensuring that the data comes back at us?","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"d4b160e96537209a387004d276a75270e19841a8","unresolved":false,"context_lines":[{"line_number":1701,"context_line":""},{"line_number":1702,"context_line":"    def test_create_all_with_project_user(self):"},{"line_number":1703,"context_line":"        consumer_uuid \u003d uuidsentinel.consumer"},{"line_number":1704,"context_line":"        consumer \u003d rp_obj.Consumer.get_or_create(self.ctx, consumer_uuid,"},{"line_number":1705,"context_line":"                project_id\u003dself.ctx.project_id, user_id\u003dself.ctx.user_id)"},{"line_number":1706,"context_line":"        rp_class \u003d fields.ResourceClass.DISK_GB"},{"line_number":1707,"context_line":"        rp \u003d self._make_rp_and_inventory(resource_class\u003drp_class,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_67174cef","line":1704,"in_reply_to":"9f6a8fd7_7c603f02","updated":"2018-04-19 16:56:24.000000000","message":"Apologies again for my lack of clarity.  I think my gripe stems from the fact that we\u0027ve now got two places we\u0027re storing the consumer, proj, and user IDs.  That makes it hard to construct tests that validate that setting/reading both/either value will \"work\" for a given code path.  So I was saying that the changes you\u0027ve made to the tests aren\u0027t adding validation of the values via the `consumer` object; all consumer/proj/user ID validation is still being done via the old fields.\n\nAs stated in my other comment, if you bite the bullet and *replace* the old fields rather than just adding the new one, you\u0027ll be forced to source the data from the new spot, which will be the only spot, which will make me sane, and there will be much rejoicing.","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a17411c24a3f92ba2c532487ab9ab4a51e006f8d","unresolved":false,"context_lines":[{"line_number":1701,"context_line":""},{"line_number":1702,"context_line":"    def test_create_all_with_project_user(self):"},{"line_number":1703,"context_line":"        consumer_uuid \u003d uuidsentinel.consumer"},{"line_number":1704,"context_line":"        consumer \u003d rp_obj.Consumer.get_or_create(self.ctx, consumer_uuid,"},{"line_number":1705,"context_line":"                project_id\u003dself.ctx.project_id, user_id\u003dself.ctx.user_id)"},{"line_number":1706,"context_line":"        rp_class \u003d fields.ResourceClass.DISK_GB"},{"line_number":1707,"context_line":"        rp \u003d self._make_rp_and_inventory(resource_class\u003drp_class,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f6a8fd7_7c603f02","line":1704,"in_reply_to":"9f6a8fd7_84b875c8","updated":"2018-04-19 16:38:04.000000000","message":"So you want a test that passes in ConsumerX to the Allocation, and that the resulting Allocation has ConsumerX in its \u0027consumer\u0027 field? That sounds, uh, kind of trivial. Do we do that for any of the other parameters we pass in?","commit_id":"aa0c4f4c3a06386a9288adf5497e621c169bf774"}],"nova/tests/unit/api/openstack/placement/objects/test_resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0e8f348ab9c0346fdeb1933a17c673a90ee297ed","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    \u0027resource_provider_id\u0027: _RESOURCE_PROVIDER_ID,"},{"line_number":81,"context_line":"    \u0027resource_provider_uuid\u0027: _RESOURCE_PROVIDER_UUID,"},{"line_number":82,"context_line":"    \u0027resource_provider_name\u0027: _RESOURCE_PROVIDER_NAME,"},{"line_number":83,"context_line":"    \u0027resource_provider_generation\u0027: 0,"},{"line_number":84,"context_line":"    \u0027resource_class_id\u0027: _RESOURCE_CLASS_ID,"},{"line_number":85,"context_line":"    \u0027consumer_id\u0027: uuids.fake_instance,"},{"line_number":86,"context_line":"    \u0027used\u0027: 8,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f6a8fd7_b78d08d5","line":83,"updated":"2018-04-26 10:45:00.000000000","message":"? What\u0027s caused this to be required?","commit_id":"bd21d3932ef2137cc622aff8c88da34752838d71"}]}