)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ede74041ad6e2087de845cdf98ec11b85f9b60c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Move project_id and user_id to Allocation object"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It had been on the AllocationList object but this make it impossible to"},{"line_number":10,"context_line":"have a an AllocationList from multiple project ids, something we want to"},{"line_number":11,"context_line":"be able to do with the new use of migration uuids."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In later patches this will also be used to demonstrate correction of an"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f287b81_f16d8999","line":10,"range":{"start_line":10,"start_character":5,"end_line":10,"end_character":7},"updated":"2017-09-05 14:50:56.000000000","message":"typo?","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d5a812a4cd8c44e1cac034cb0397c4f16280a062","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Move project_id and user_id to Allocation object"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It had been on the AllocationList object but this make it impossible to"},{"line_number":10,"context_line":"have a an AllocationList from multiple project ids, something we want to"},{"line_number":11,"context_line":"be able to do with the new use of migration uuids."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In later patches this will also be used to demonstrate correction of an"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f287b81_0a5a17f2","line":10,"in_reply_to":"7f287b81_f16d8999","updated":"2017-09-07 22:51:13.000000000","message":"Done","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5ede74041ad6e2087de845cdf98ec11b85f9b60c","unresolved":false,"context_lines":[{"line_number":16,"context_line":"In the proces of making this change the obj_make_compatible in the"},{"line_number":17,"context_line":"Allocation class was removed. We don\u0027t do RPC. We don\u0027t need this and"},{"line_number":18,"context_line":"the choice in this change was either to add more compatibility changes"},{"line_number":19,"context_line":"or simply get rid of the routine. Leaving it as is would be misleading."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I3cf887bd541c187ec3baca2ae3f8c16f1754e96e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f287b81_7130799f","line":19,"range":{"start_line":19,"start_character":34,"end_line":19,"end_character":71},"updated":"2017-09-05 14:50:56.000000000","message":"+1","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"049d1eeaf0adebd5f6ea98b8bffa386c64de462b","unresolved":false,"context_lines":[{"line_number":19,"context_line":"continue to support."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Inspecting the placement fixtures related to this change revealed a bug"},{"line_number":22,"context_line":"with a missing user_id in one Allocation being created."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"In later patches this will also be used to demonstrate correction of an"},{"line_number":25,"context_line":"issue with consumer ids not being properly associated with project ids."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"9f5c4f37_270d7a90","line":22,"updated":"2017-09-21 00:06:08.000000000","message":"this isn\u0027t actually true, that was an artifact of an earlier patch","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee98bde37a09604fa7eb3d1c18e1120c099d0d54","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Move project_id and user_id to Allocation object"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It had been on the AllocationList object but this makes it impossible to"},{"line_number":10,"context_line":"have an AllocationList from multiple project ids, something we want to"},{"line_number":11,"context_line":"be able to do with the new use of migration uuids."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In the process, ensure that if a persisted Allocation has a project_id"},{"line_number":14,"context_line":"and user_id associated with it, load those values when the Allocation is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"5f4e5783_1f1ce868","line":11,"range":{"start_line":10,"start_character":50,"end_line":11,"end_character":50},"updated":"2017-10-10 17:18:01.000000000","message":"as mentioned on IRC, the use of migration UUIDs as consumers isn\u0027t really related to being able to change an allocation\u0027s user or project, but ok. :)","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c01730c5276db0941f97ad718156ea606f65e81","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Move project_id and user_id to Allocation object"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It had been on the AllocationList object but this makes it impossible to"},{"line_number":10,"context_line":"have an AllocationList from multiple project ids, something we want to"},{"line_number":11,"context_line":"be able to do with the new use of migration uuids."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"In the process, ensure that if a persisted Allocation has a project_id"},{"line_number":14,"context_line":"and user_id associated with it, load those values when the Allocation is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"5f4e5783_f0c5bb5c","line":11,"range":{"start_line":10,"start_character":50,"end_line":11,"end_character":50},"in_reply_to":"5f4e5783_1f1ce868","updated":"2017-10-10 17:25:47.000000000","message":"yeah, I\u0027ll clean up the message to make the intent more clear: if cleaning, may as well clean for max flex.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"}],"nova/api/openstack/placement/handlers/allocation.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"45758bfe4ea457ce92fbece4d3c68c98a67cbc0c","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                used\u003dresources[resource_class])"},{"line_number":242,"context_line":"            allocation_objects.append(allocation)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    allocations \u003d rp_obj.AllocationList("},{"line_number":245,"context_line":"        context, objects\u003dallocation_objects)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    try:"},{"line_number":248,"context_line":"        allocations.create_all()"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f4b6375_1f955f33","line":245,"range":{"start_line":244,"start_character":0,"end_line":245,"end_character":44},"updated":"2017-10-25 16:47:53.000000000","message":"nit - no need for the continuation line anymore.","commit_id":"e000a8f290e16e0e357f3707e6223676fcda05fc"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ddbb17ab19b7e3c1af4ce40d4a73e748bee8270f","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_46a71a0d","line":1767,"updated":"2017-09-04 13:09:15.000000000","message":"Does removing fields needs a major version bump?","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"581464b7c36ebb3b519a1e0a3ca5f11135d64b1c","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_7365cbb4","line":1767,"in_reply_to":"7f287b81_337f4367","updated":"2017-09-07 22:55:50.000000000","message":"Sure.","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"fe9a73ab4aad015b08dc1bd1d324d7d84c58a40b","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_7a6f49c5","line":1767,"in_reply_to":"7f287b81_46a71a0d","updated":"2017-09-04 15:46:21.000000000","message":"Because we\u0027re not doing (and won\u0027t do) RPC with these objects, versioning them at all isn\u0027t necessary, we\u0027ve just maintained that habit as a sort of easy history. We could potentially unregister them, but nobody has bothered yet.","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6bd798157e84bca179b6ecdd3612bd95b0be1156","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_f3a0cda7","line":1767,"in_reply_to":"7f287b81_7a6f49c5","updated":"2017-09-06 15:01:43.000000000","message":"I didn\u0027t know that. Thanks for the info.","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d5a812a4cd8c44e1cac034cb0397c4f16280a062","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_337f4367","line":1767,"in_reply_to":"7f287b81_b343333f","updated":"2017-09-07 22:51:13.000000000","message":"I\u0027d be pretty happy if we went ahead and unregistered this stuff. If nobody has any objections, I\u0027d be willing to push up a patch (or series) that unregisters everything in this file. For the time being I\u0027ll add a more verbose comment and then if we agree, I\u0027ll do the unregistering elsewhere and put this above it.","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"55a26ad73adeb20adcbe508f12fd1ed3e5de43ba","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"    # Version 1.1: Add create_all() and delete_all()"},{"line_number":1765,"context_line":"    # Version 1.2: Turn off remotable"},{"line_number":1766,"context_line":"    # Version 1.3: Add project_id and user_id fields"},{"line_number":1767,"context_line":"    # Version 1.4: Move project_id and user_id to Allocation"},{"line_number":1768,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f287b81_b343333f","line":1767,"in_reply_to":"7f287b81_f3a0cda7","updated":"2017-09-07 22:46:12.000000000","message":"Yeah, it\u0027s really wrong to do this because it\u0027s confusing.\n\nTBH, I\u0027d rather us unregister it before we do this and maybe throw a comment on top to make it clear. Unfortunately, someone will call this out in a year as an example of being able to remove fields as long as you throw a version on it.\n\nIf we\u0027re not going to unregister it here or before this, Chris can you just add more to this comment about why it\u0027s okay here and you\u0027re just bumping the version?","commit_id":"59145b978fab216ce64dc4db9a656d117fc8c700"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"    def _from_db_object(context, target, source):"},{"line_number":1323,"context_line":"        _ensure_rc_cache(context)"},{"line_number":1324,"context_line":"        for field in target.fields:"},{"line_number":1325,"context_line":"            # In addition to skipping resource proivder and resource class"},{"line_number":1326,"context_line":"            # (handled below), skip project_id and user_id which we do not"},{"line_number":1327,"context_line":"            # expose outside the Allocation object."},{"line_number":1328,"context_line":"            if field not in (\u0027resource_provider\u0027, \u0027resource_class\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_4735ecd9","line":1325,"range":{"start_line":1325,"start_character":47,"end_line":1325,"end_character":55},"updated":"2017-09-20 17:33:47.000000000","message":"spelling","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"        for field in target.fields:"},{"line_number":1325,"context_line":"            # In addition to skipping resource proivder and resource class"},{"line_number":1326,"context_line":"            # (handled below), skip project_id and user_id which we do not"},{"line_number":1327,"context_line":"            # expose outside the Allocation object."},{"line_number":1328,"context_line":"            if field not in (\u0027resource_provider\u0027, \u0027resource_class\u0027,"},{"line_number":1329,"context_line":"                             \u0027project_id\u0027, \u0027user_id\u0027):"},{"line_number":1330,"context_line":"                setattr(target, field, source[field])"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_a7f708f6","line":1327,"updated":"2017-09-20 17:33:47.000000000","message":"Okay, from reading this I thought you meant that they\u0027re on the allocation row, but we don\u0027t want to load them because the api won\u0027t show them. In reality, I think you\u0027re skipping them here because they\u0027re on the consumer not the allocation, right? I would tend to think that what we should do here is be joining that table so we have access to it and keep this object complete. Otherwise, we can\u0027t pull it out as a whole piece and would always have to do the stitching up of these fields later. Any reason not to do the join and fully load? Should be basically free I think.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5f53b87abcb68e507f2e31a471c5f7597070e096","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"        for field in target.fields:"},{"line_number":1325,"context_line":"            # In addition to skipping resource proivder and resource class"},{"line_number":1326,"context_line":"            # (handled below), skip project_id and user_id which we do not"},{"line_number":1327,"context_line":"            # expose outside the Allocation object."},{"line_number":1328,"context_line":"            if field not in (\u0027resource_provider\u0027, \u0027resource_class\u0027,"},{"line_number":1329,"context_line":"                             \u0027project_id\u0027, \u0027user_id\u0027):"},{"line_number":1330,"context_line":"                setattr(target, field, source[field])"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_0a8ffb23","line":1327,"in_reply_to":"bf57530f_2a199f78","updated":"2017-09-20 18:15:05.000000000","message":"I think the object should be consistent unless there\u0027s a really good reason otherwise. Since I don\u0027t think there\u0027s a good reason, I\u0027d rather do the join and fix this.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"        for field in target.fields:"},{"line_number":1325,"context_line":"            # In addition to skipping resource proivder and resource class"},{"line_number":1326,"context_line":"            # (handled below), skip project_id and user_id which we do not"},{"line_number":1327,"context_line":"            # expose outside the Allocation object."},{"line_number":1328,"context_line":"            if field not in (\u0027resource_provider\u0027, \u0027resource_class\u0027,"},{"line_number":1329,"context_line":"                             \u0027project_id\u0027, \u0027user_id\u0027):"},{"line_number":1330,"context_line":"                setattr(target, field, source[field])"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_2a199f78","line":1327,"in_reply_to":"bf57530f_a7f708f6","updated":"2017-09-20 18:10:46.000000000","message":"What this is doing/allowing is making it so the project_id and user_id do not need to be exposed where the object is used. That is: we don\u0027t (yet) see project_id and user_id when making a GET request.\n\nSince we don\u0027t yet need it I figured: don\u0027t require it, as that would require (as you say) doing the join, and meh to that if we\u0027re not using it.\n\nBut happy to do otherwise if we want to prepare for it.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1457,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":1458,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1459,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1460,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1461,"context_line":"    }"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_8773e480","line":1460,"updated":"2017-09-20 17:33:47.000000000","message":"Why are these nullable? We don\u0027t want to ever allow this and only have them nullable in the schema because we added them later right?","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1457,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":1458,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1459,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1460,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1461,"context_line":"    }"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_ea514710","line":1460,"in_reply_to":"bf57530f_8773e480","updated":"2017-09-20 18:10:46.000000000","message":"See my comments above, but also I was pretty much just preserving things as they were.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5f53b87abcb68e507f2e31a471c5f7597070e096","unresolved":false,"context_lines":[{"line_number":1457,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":1458,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1459,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1460,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1461,"context_line":"    }"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_0ab81bc6","line":1460,"in_reply_to":"bf57530f_ea514710","updated":"2017-09-20 18:15:05.000000000","message":"They don\u0027t need to be nullable to be unset. Anything can be unset (which you\u0027re doing above). Nullable means \"can be None\" and I don\u0027t think that is something we want.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"    def destroy(self):"},{"line_number":1483,"context_line":"        self._destroy(self._context, self.id)"},{"line_number":1484,"context_line":""},{"line_number":1485,"context_line":"    def ensure_consumer_project_user(self, conn, consumer_id):"},{"line_number":1486,"context_line":"        \"\"\"Examines the project_id, user_id of the object along with the"},{"line_number":1487,"context_line":"        supplied consumer_id and ensures that there are records in the"},{"line_number":1488,"context_line":"        consumers, projects, and users table for these entities."}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_2718b81b","line":1485,"range":{"start_line":1485,"start_character":49,"end_line":1485,"end_character":60},"updated":"2017-09-20 17:33:47.000000000","message":"We don\u0027t need this, since it\u0027s just self.consumer_id right?","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"    def destroy(self):"},{"line_number":1483,"context_line":"        self._destroy(self._context, self.id)"},{"line_number":1484,"context_line":""},{"line_number":1485,"context_line":"    def ensure_consumer_project_user(self, conn, consumer_id):"},{"line_number":1486,"context_line":"        \"\"\"Examines the project_id, user_id of the object along with the"},{"line_number":1487,"context_line":"        supplied consumer_id and ensures that there are records in the"},{"line_number":1488,"context_line":"        consumers, projects, and users table for these entities."}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_8a408b37","line":1485,"range":{"start_line":1485,"start_character":49,"end_line":1485,"end_character":60},"in_reply_to":"bf57530f_2718b81b","updated":"2017-09-20 18:10:46.000000000","message":"Yeah, is pretty much a pasto","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1489,"context_line":""},{"line_number":1490,"context_line":"        :param consumer_id: Comes from the Allocation object being processed"},{"line_number":1491,"context_line":"        \"\"\""},{"line_number":1492,"context_line":"        if (self.obj_attr_is_set(\u0027project_id\u0027) and"},{"line_number":1493,"context_line":"                self.project_id is not None and"},{"line_number":1494,"context_line":"                self.obj_attr_is_set(\u0027user_id\u0027) and"},{"line_number":1495,"context_line":"                self.user_id is not None):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_2751d8d9","line":1492,"updated":"2017-09-20 17:33:47.000000000","message":"I know this is pre-existing, but the nicer way to do these is:\n\n if \u0027project_id\u0027 in self","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"        :param consumer_id: Comes from the Allocation object being processed"},{"line_number":1491,"context_line":"        \"\"\""},{"line_number":1492,"context_line":"        if (self.obj_attr_is_set(\u0027project_id\u0027) and"},{"line_number":1493,"context_line":"                self.project_id is not None and"},{"line_number":1494,"context_line":"                self.obj_attr_is_set(\u0027user_id\u0027) and"},{"line_number":1495,"context_line":"                self.user_id is not None):"},{"line_number":1496,"context_line":"            # Grab the project internal ID if it exists in the projects table"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_6770f073","line":1493,"updated":"2017-09-20 17:33:47.000000000","message":"These should not be nullable, IMHO.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"        :param consumer_id: Comes from the Allocation object being processed"},{"line_number":1491,"context_line":"        \"\"\""},{"line_number":1492,"context_line":"        if (self.obj_attr_is_set(\u0027project_id\u0027) and"},{"line_number":1493,"context_line":"                self.project_id is not None and"},{"line_number":1494,"context_line":"                self.obj_attr_is_set(\u0027user_id\u0027) and"},{"line_number":1495,"context_line":"                self.user_id is not None):"},{"line_number":1496,"context_line":"            # Grab the project internal ID if it exists in the projects table"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_4a917399","line":1493,"in_reply_to":"bf57530f_6770f073","updated":"2017-09-20 18:10:46.000000000","message":"at this point, true, can change when I fix up the pasto-ness of all this","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1512,"context_line":"                    # We assume at this time that a consumer project/user can\u0027t"},{"line_number":1513,"context_line":"                    # change, so if we get here, we raced and should just pass"},{"line_number":1514,"context_line":"                    # if the consumer already exists."},{"line_number":1515,"context_line":"                    pass"},{"line_number":1516,"context_line":""},{"line_number":1517,"context_line":""},{"line_number":1518,"context_line":"def _delete_current_allocs(conn, consumer_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_27ff98ab","line":1515,"updated":"2017-09-20 17:33:47.000000000","message":"If we don\u0027t have these set for some reason, we should explode, right? Otherwise this is \"ensure_consumer_if_set_otherwise_I_dunno_just_ignore()\"","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1512,"context_line":"                    # We assume at this time that a consumer project/user can\u0027t"},{"line_number":1513,"context_line":"                    # change, so if we get here, we raced and should just pass"},{"line_number":1514,"context_line":"                    # if the consumer already exists."},{"line_number":1515,"context_line":"                    pass"},{"line_number":1516,"context_line":""},{"line_number":1517,"context_line":""},{"line_number":1518,"context_line":"def _delete_current_allocs(conn, consumer_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_aa458f19","line":1515,"in_reply_to":"bf57530f_27ff98ab","updated":"2017-09-20 18:10:46.000000000","message":"tru\n\nwill fix, looks like the earlier method had the same problem","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1801,"context_line":"            # First delete any existing allocations for this consumer."},{"line_number":1802,"context_line":"            for alloc in allocs:"},{"line_number":1803,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1804,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1805,"context_line":"            for alloc in allocs:"},{"line_number":1806,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1807,"context_line":"                alloc.ensure_consumer_project_user(conn, consumer_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_87e64416","line":1804,"updated":"2017-09-20 17:33:47.000000000","message":"Since we\u0027re in a transaction, any reason not to just do this in the loop below?","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1801,"context_line":"            # First delete any existing allocations for this consumer."},{"line_number":1802,"context_line":"            for alloc in allocs:"},{"line_number":1803,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1804,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1805,"context_line":"            for alloc in allocs:"},{"line_number":1806,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1807,"context_line":"                alloc.ensure_consumer_project_user(conn, consumer_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_cadc4348","line":1804,"in_reply_to":"bf57530f_87e64416","updated":"2017-09-20 18:10:46.000000000","message":"It was to be able have all the deleting done before the _check_capacity_exceeded, but then I forgot to move that line, so a definite problem here.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5821f798e932b0760dacfaf9777c1a49fe28a150","unresolved":false,"context_lines":[{"line_number":1804,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1805,"context_line":"            for alloc in allocs:"},{"line_number":1806,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1807,"context_line":"                alloc.ensure_consumer_project_user(conn, consumer_id)"},{"line_number":1808,"context_line":"                rp \u003d alloc.resource_provider"},{"line_number":1809,"context_line":"                rc_id \u003d _RC_CACHE.id_from_string(alloc.resource_class)"},{"line_number":1810,"context_line":"                ins_stmt \u003d _ALLOC_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_a72ca847","line":1807,"updated":"2017-09-20 17:33:47.000000000","message":"I wonder if we should cache the ones we\u0027ve confirmed so far to avoid multiple checks for the same consumer? Might be pre-optimization, but it\u0027d be so easy.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb7f1fb83ad1595af781aeaf0696f5d2e31da68d","unresolved":false,"context_lines":[{"line_number":1804,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1805,"context_line":"            for alloc in allocs:"},{"line_number":1806,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1807,"context_line":"                alloc.ensure_consumer_project_user(conn, consumer_id)"},{"line_number":1808,"context_line":"                rp \u003d alloc.resource_provider"},{"line_number":1809,"context_line":"                rc_id \u003d _RC_CACHE.id_from_string(alloc.resource_class)"},{"line_number":1810,"context_line":"                ins_stmt \u003d _ALLOC_TBL.insert().values("}],"source_content_type":"text/x-python","patch_set":8,"id":"bf57530f_8ae64b16","line":1807,"in_reply_to":"bf57530f_a72ca847","updated":"2017-09-20 18:10:46.000000000","message":"Huh, I thought I had done that. Maybe I never pushed that code, or did it in a dream.","commit_id":"36dbbf9327f4e6ab7dd7c6a9a146a32dea7e7dc1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"049d1eeaf0adebd5f6ea98b8bffa386c64de462b","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"        for field in target.fields:"},{"line_number":1325,"context_line":"            # In addition to skipping resource provider and resource class"},{"line_number":1326,"context_line":"            # (handled below), skip project_id and user_id which we do not"},{"line_number":1327,"context_line":"            # expose outside the Allocation object."},{"line_number":1328,"context_line":"            if field not in (\u0027resource_provider\u0027, \u0027resource_class\u0027):"},{"line_number":1329,"context_line":"                setattr(target, field, source[field])"},{"line_number":1330,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f5c4f37_07eb96b0","line":1327,"updated":"2017-09-21 00:06:08.000000000","message":"this comment can be removed","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"049d1eeaf0adebd5f6ea98b8bffa386c64de462b","unresolved":false,"context_lines":[{"line_number":1494,"context_line":"        # move on. This allows pre-existing tests and code which did not"},{"line_number":1495,"context_line":"        # require those fields to continue to work. In contemporary code"},{"line_number":1496,"context_line":"        # the fields are required and the enforcement is at the HTTP API"},{"line_number":1497,"context_line":"        # layer. We should probably enforce it at the object layer too."},{"line_number":1498,"context_line":"        if not (\u0027project_id\u0027 in self and"},{"line_number":1499,"context_line":"            self.project_id is not None and"},{"line_number":1500,"context_line":"            \u0027user_id\u0027 in self and"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f5c4f37_470aae8a","line":1497,"updated":"2017-09-21 00:06:08.000000000","message":"this comment should mention microversions rather than tests","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d01c0688550b167722995b03aafbd826577201ad","unresolved":false,"context_lines":[{"line_number":1494,"context_line":"        # move on. This allows pre-existing tests and code which did not"},{"line_number":1495,"context_line":"        # require those fields to continue to work. In contemporary code"},{"line_number":1496,"context_line":"        # the fields are required and the enforcement is at the HTTP API"},{"line_number":1497,"context_line":"        # layer. We should probably enforce it at the object layer too."},{"line_number":1498,"context_line":"        if not (\u0027project_id\u0027 in self and"},{"line_number":1499,"context_line":"            self.project_id is not None and"},{"line_number":1500,"context_line":"            \u0027user_id\u0027 in self and"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f5c4f37_fb69208b","line":1497,"in_reply_to":"9f5c4f37_470aae8a","updated":"2017-09-21 15:15:37.000000000","message":"I thought it was said in IRC yesterday that the requirement came before we crystallized the first microversion. Is that not true?\n\nGiven the small scope of people/projects consuming the allocations API at this point, I\u0027d be in favor of an early lift of the minimum microversion to above this point so we can avoid having to carry support for nullablility in these fields. However, that can be a parallel effort to this.","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"98faa32a8ab21bfac1bd30bbdd52e42733a1a54b","unresolved":false,"context_lines":[{"line_number":1494,"context_line":"        # move on. This allows pre-existing tests and code which did not"},{"line_number":1495,"context_line":"        # require those fields to continue to work. In contemporary code"},{"line_number":1496,"context_line":"        # the fields are required and the enforcement is at the HTTP API"},{"line_number":1497,"context_line":"        # layer. We should probably enforce it at the object layer too."},{"line_number":1498,"context_line":"        if not (\u0027project_id\u0027 in self and"},{"line_number":1499,"context_line":"            self.project_id is not None and"},{"line_number":1500,"context_line":"            \u0027user_id\u0027 in self and"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_461872d2","line":1497,"in_reply_to":"9f5c4f37_fb69208b","updated":"2017-09-21 19:35:44.000000000","message":"requiring project and user id came in microversion 1.8: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/allocation.py#L83\n\nIf we are going to raise the microversion, then let\u0027s do that in a different stack, this stack of stuff is already over-encumbered with dependencies.","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d01c0688550b167722995b03aafbd826577201ad","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"            # First delete any existing allocations for this consumer."},{"line_number":1824,"context_line":"            for alloc in allocs:"},{"line_number":1825,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1826,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1827,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1828,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1829,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":9,"id":"9f5c4f37_1b1bd41b","line":1826,"updated":"2017-09-21 15:15:37.000000000","message":"Were you going to move this down as well?","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"98faa32a8ab21bfac1bd30bbdd52e42733a1a54b","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"            # First delete any existing allocations for this consumer."},{"line_number":1824,"context_line":"            for alloc in allocs:"},{"line_number":1825,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1826,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1827,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1828,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1829,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_e6d99ef4","line":1826,"in_reply_to":"9f5c4f37_1b1bd41b","updated":"2017-09-21 19:35:44.000000000","message":"See the earlier comments on ps8. The gist is that the delete must happen before the _check_capacity_exceeded, otherwise we\u0027re only able to make allocations above what we had before, which doesn\u0027t align with the allocations being a replace.","commit_id":"ad868db4720a6f4a42e1527499897ee0be631c89"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee98bde37a09604fa7eb3d1c18e1120c099d0d54","unresolved":false,"context_lines":[{"line_number":1455,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1456,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1457,"context_line":"        # required."},{"line_number":1458,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1459,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1460,"context_line":"    }"},{"line_number":1461,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7f515b1d_a08aca17","line":1458,"range":{"start_line":1458,"start_character":16,"end_line":1458,"end_character":19},"updated":"2017-10-10 17:18:01.000000000","message":"How about calling these fields just \"project\" and \"user\"? The values of the fields are not the internal integer identifiers but rather the external identifiers, so leaving the \"_id\" off seems like a good idea to me.\n\nIf not, then at a minimum we should have a code comment above alerting readers that these are the external identifiers, not the internal ones.","commit_id":"8199cbe726ab28bd813a8527a2878ae27655e7bc"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c01730c5276db0941f97ad718156ea606f65e81","unresolved":false,"context_lines":[{"line_number":1455,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1456,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1457,"context_line":"        # required."},{"line_number":1458,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1459,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1460,"context_line":"    }"},{"line_number":1461,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"5f4e5783_f0a81b78","line":1458,"range":{"start_line":1458,"start_character":16,"end_line":1458,"end_character":19},"in_reply_to":"7f515b1d_a08aca17","updated":"2017-10-10 17:25:47.000000000","message":"Because project_id and user_id is what they were called on AllocationList (l 1699) and it seemed a bit too much to change that.","commit_id":"8199cbe726ab28bd813a8527a2878ae27655e7bc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee98bde37a09604fa7eb3d1c18e1120c099d0d54","unresolved":false,"context_lines":[{"line_number":1820,"context_line":"            # must be done before checking capacity."},{"line_number":1821,"context_line":"            for alloc in allocs:"},{"line_number":1822,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1823,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1824,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1825,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1826,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":11,"id":"7f515b1d_c0d38600","line":1823,"updated":"2017-10-10 17:18:01.000000000","message":"Would be more efficient to do:\n\n consumer_ids \u003d set(a.consumer_id for a in allocs)\n for consumer_id in consumer_ids:\n     _delete_current_allocs(conn, consumer_id)","commit_id":"8199cbe726ab28bd813a8527a2878ae27655e7bc"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a5ba981a89484ed478afbf99538ef4044c3d057e","unresolved":false,"context_lines":[{"line_number":1510,"context_line":"        # then the fields are required and the enforcement is at the HTTP"},{"line_number":1511,"context_line":"        # API layer."},{"line_number":1512,"context_line":"        if not (\u0027project_id\u0027 in self and"},{"line_number":1513,"context_line":"            self.project_id is not None and"},{"line_number":1514,"context_line":"            \u0027user_id\u0027 in self and"},{"line_number":1515,"context_line":"            self.user_id is not None):"},{"line_number":1516,"context_line":"            return"},{"line_number":1517,"context_line":"        # Grab the project internal ID if it exists in the projects table"},{"line_number":1518,"context_line":"        pid \u003d _ensure_project(conn, self.project_id)"},{"line_number":1519,"context_line":"        # Grab the user internal ID if it exists in the users table"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_d2d1a759","line":1516,"range":{"start_line":1513,"start_character":12,"end_line":1516,"end_character":18},"updated":"2017-10-10 16:30:49.000000000","message":"Please indent the continued lines 2 levels, not 1. The \u0027return\u0027 line starts a new block, and should not be at the same indentation level as the line above it.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee98bde37a09604fa7eb3d1c18e1120c099d0d54","unresolved":false,"context_lines":[{"line_number":1781,"context_line":"                     models.Consumer.project_id \u003d\u003d models.Project.id)"},{"line_number":1782,"context_line":"                 .outerjoin("},{"line_number":1783,"context_line":"                     models.User,"},{"line_number":1784,"context_line":"                     models.Consumer.user_id \u003d\u003d models.User.id))"},{"line_number":1785,"context_line":"        if resource_provider_uuid:"},{"line_number":1786,"context_line":"            query \u003d query.filter("},{"line_number":1787,"context_line":"                models.ResourceProvider.uuid \u003d\u003d resource_provider_uuid)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_ff9a2c6c","line":1784,"updated":"2017-10-10 17:18:01.000000000","message":"we\u0027ll merge-race on this section, be warned :)\n\nhttps://review.openstack.org/#/c/509033/","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c01730c5276db0941f97ad718156ea606f65e81","unresolved":false,"context_lines":[{"line_number":1781,"context_line":"                     models.Consumer.project_id \u003d\u003d models.Project.id)"},{"line_number":1782,"context_line":"                 .outerjoin("},{"line_number":1783,"context_line":"                     models.User,"},{"line_number":1784,"context_line":"                     models.Consumer.user_id \u003d\u003d models.User.id))"},{"line_number":1785,"context_line":"        if resource_provider_uuid:"},{"line_number":1786,"context_line":"            query \u003d query.filter("},{"line_number":1787,"context_line":"                models.ResourceProvider.uuid \u003d\u003d resource_provider_uuid)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_f0ec9bf4","line":1784,"in_reply_to":"5f4e5783_ff9a2c6c","updated":"2017-10-10 17:25:47.000000000","message":"yeah, I look forward to the eventual cleanliness","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a5ba981a89484ed478afbf99538ef4044c3d057e","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"            # First delete any existing allocations for this consumer. This"},{"line_number":1838,"context_line":"            # must be done before checking capacity."},{"line_number":1839,"context_line":"            for alloc in allocs:"},{"line_number":1840,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1841,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1842,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1843,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_f25023d7","line":1840,"updated":"2017-10-10 16:30:49.000000000","message":"Is there only ever going to be one alloc per consumer_id? Lines 1848 - 1853 seem to suggest that will not be the case, so I think it would be best to get the set of consumer_ids, so we only delete once per consumer.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ee0f5a4aba5ff8f261089af32e71ec5b1bb4f429","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"            # First delete any existing allocations for this consumer. This"},{"line_number":1838,"context_line":"            # must be done before checking capacity."},{"line_number":1839,"context_line":"            for alloc in allocs:"},{"line_number":1840,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1841,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1842,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1843,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_3200fb34","line":1840,"in_reply_to":"5f4e5783_f25023d7","updated":"2017-10-10 16:43:33.000000000","message":"that\u0027s done in a later patch: https://review.openstack.org/#/c/507224/","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee98bde37a09604fa7eb3d1c18e1120c099d0d54","unresolved":false,"context_lines":[{"line_number":1838,"context_line":"            # must be done before checking capacity."},{"line_number":1839,"context_line":"            for alloc in allocs:"},{"line_number":1840,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1841,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1842,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1843,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1844,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_dfdc101a","line":1841,"updated":"2017-10-10 17:18:01.000000000","message":"would be better to do:\n\n consumer_ids \u003d set(a.consumer_id for a in allocs)\n for consumer_id in consumer_ids:\n     _delete_current_allocs(conn, consumer_id)\n\nsince it\u0027s likely that there will be many allocations but only one consumer and you\u0027ll just needlessly be executing a DELETE statement more than you need to.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2af67718cf89db248fbea97fd8f4d3e278fff029","unresolved":false,"context_lines":[{"line_number":1838,"context_line":"            # must be done before checking capacity."},{"line_number":1839,"context_line":"            for alloc in allocs:"},{"line_number":1840,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1841,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1842,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1843,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1844,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_b0662351","line":1841,"in_reply_to":"5f4e5783_b0d74324","updated":"2017-10-10 17:31:05.000000000","message":"gotcha.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c01730c5276db0941f97ad718156ea606f65e81","unresolved":false,"context_lines":[{"line_number":1838,"context_line":"            # must be done before checking capacity."},{"line_number":1839,"context_line":"            for alloc in allocs:"},{"line_number":1840,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1841,"context_line":"                _delete_current_allocs(conn, consumer_id)"},{"line_number":1842,"context_line":"            # If there are any allocations with string resource class names"},{"line_number":1843,"context_line":"            # that don\u0027t exist this will raise a ResourceClassNotFound"},{"line_number":1844,"context_line":"            # exception."}],"source_content_type":"text/x-python","patch_set":12,"id":"5f4e5783_b0d74324","line":1841,"in_reply_to":"5f4e5783_dfdc101a","updated":"2017-10-10 17:25:47.000000000","message":"yeah, see above in response to ed, where I point to where exactly that is done\n\nDan didn\u0027t want that change in this change.","commit_id":"64bd3214e3690b283b4436ab27094ccbf1c80c85"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f5af4ec0ef92a9354b3a56a2181c9c9e5ed1218d","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1451,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":1452,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1453,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1454,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1455,"context_line":"        # required."},{"line_number":1456,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1457,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1458,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_c2b615fb","line":1455,"range":{"start_line":1453,"start_character":0,"end_line":1455,"end_character":19},"updated":"2017-10-23 15:52:52.000000000","message":"A TODO to change this when/if the minimum microversion is bumped could be an idea","commit_id":"745d18eac75aafe5ad8155ae1ee891961e9d83f3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2fecba3a968d90c206d8a68231b1bda77cba6c64","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"        \u0027consumer_id\u0027: fields.UUIDField(),"},{"line_number":1451,"context_line":"        \u0027resource_class\u0027: fields.ResourceClassField(),"},{"line_number":1452,"context_line":"        \u0027used\u0027: fields.IntegerField(),"},{"line_number":1453,"context_line":"        # The following two fields are allowed to be set to None to"},{"line_number":1454,"context_line":"        # support Allocations that were created before the fields were"},{"line_number":1455,"context_line":"        # required."},{"line_number":1456,"context_line":"        \u0027project_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1457,"context_line":"        \u0027user_id\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":1458,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_f44e2f36","line":1455,"range":{"start_line":1453,"start_character":0,"end_line":1455,"end_character":19},"in_reply_to":"3f4b6375_c2b615fb","updated":"2017-10-25 12:30:21.000000000","message":"I suspect if we ever chose to bump the minimum microversion of placement it would end up being a major version bump instead. Or to put it another way: in the absence of super-major-huge compelling reason to bump the placement min microversion we shouldn\u0027t.","commit_id":"745d18eac75aafe5ad8155ae1ee891961e9d83f3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f5af4ec0ef92a9354b3a56a2181c9c9e5ed1218d","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"                self.project_id is not None and"},{"line_number":1472,"context_line":"                \u0027user_id\u0027 in self and"},{"line_number":1473,"context_line":"                self.user_id is not None):"},{"line_number":1474,"context_line":"            return"},{"line_number":1475,"context_line":"        # Grab the project internal ID if it exists in the projects table"},{"line_number":1476,"context_line":"        pid \u003d _ensure_project(conn, self.project_id)"},{"line_number":1477,"context_line":"        # Grab the user internal ID if it exists in the users table"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f4b6375_429fc55e","line":1474,"range":{"start_line":1474,"start_character":12,"end_line":1474,"end_character":18},"updated":"2017-10-23 15:52:52.000000000","message":"better","commit_id":"745d18eac75aafe5ad8155ae1ee891961e9d83f3"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"45758bfe4ea457ce92fbece4d3c68c98a67cbc0c","unresolved":false,"context_lines":[{"line_number":1788,"context_line":"        with conn.begin():"},{"line_number":1789,"context_line":"            # First delete any existing allocations for this consumer. This"},{"line_number":1790,"context_line":"            # must be done before checking capacity."},{"line_number":1791,"context_line":"            for alloc in allocs:"},{"line_number":1792,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1793,"context_line":"                _delete_allocations_for_consumer(context, consumer_id)"},{"line_number":1794,"context_line":"            # If there are any allocations with string resource class names"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f4b6375_9fc94f5a","line":1791,"range":{"start_line":1791,"start_character":12,"end_line":1791,"end_character":31},"updated":"2017-10-25 16:47:53.000000000","message":"Most cases will have the same consumer_id for all allocs, so it would be more efficient to only delete them once.\n\n  for consumer_id in {alloc.consumer_id for alloc in allocs}:\n    _delete_allocations_for_consumer(context, consumer_id)","commit_id":"e000a8f290e16e0e357f3707e6223676fcda05fc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f069f76d9b8302a37a02c100c0db942693df023e","unresolved":false,"context_lines":[{"line_number":1788,"context_line":"        with conn.begin():"},{"line_number":1789,"context_line":"            # First delete any existing allocations for this consumer. This"},{"line_number":1790,"context_line":"            # must be done before checking capacity."},{"line_number":1791,"context_line":"            for alloc in allocs:"},{"line_number":1792,"context_line":"                consumer_id \u003d alloc.consumer_id"},{"line_number":1793,"context_line":"                _delete_allocations_for_consumer(context, consumer_id)"},{"line_number":1794,"context_line":"            # If there are any allocations with string resource class names"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f4b6375_9ad38004","line":1791,"range":{"start_line":1791,"start_character":12,"end_line":1791,"end_character":31},"in_reply_to":"3f4b6375_9fc94f5a","updated":"2017-10-26 14:07:46.000000000","message":"Right, which is exactly what I\u0027d said in PS11:\n\nhttps://review.openstack.org/#/c/500410/11/nova/objects/resource_provider.py@1823","commit_id":"e000a8f290e16e0e357f3707e6223676fcda05fc"}]}