)]}'
{"nova/api/openstack/placement/objects/resource_provider.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dfcd2ebf9bc2e02fbe3a38cceb4f0d242b3171de","unresolved":false,"context_lines":[{"line_number":4094,"context_line":"            # prepopulated from the allocations."},{"line_number":4095,"context_line":"            rp \u003d new_inv_list[0].resource_provider"},{"line_number":4096,"context_line":"        elif rp_uuid in affected_providers:"},{"line_number":4097,"context_line":"            # No inventory object; second choice is from an allocation object."},{"line_number":4098,"context_line":"            rp \u003d affected_providers[rp_uuid]"},{"line_number":4099,"context_line":"        else:"},{"line_number":4100,"context_line":"            # No inventory or allocations for this provider - which makes sense"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_36bf8dac","line":4097,"updated":"2018-07-24 09:35:46.000000000","message":"When would this happen? I guess when allocations are being moved to a resource provider that already has the \"correct\" inventory on it, and we need to make no changes.","commit_id":"92082bc3a7b29da8b7e29e3cdc70946c374a5672"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"65eed3cb59e54868d996c2b9cda651c83d0a884b","unresolved":false,"context_lines":[{"line_number":4094,"context_line":"            # prepopulated from the allocations."},{"line_number":4095,"context_line":"            rp \u003d new_inv_list[0].resource_provider"},{"line_number":4096,"context_line":"        elif rp_uuid in affected_providers:"},{"line_number":4097,"context_line":"            # No inventory object; second choice is from an allocation object."},{"line_number":4098,"context_line":"            rp \u003d affected_providers[rp_uuid]"},{"line_number":4099,"context_line":"        else:"},{"line_number":4100,"context_line":"            # No inventory or allocations for this provider - which makes sense"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_cde0fffc","line":4097,"in_reply_to":"5f7c97a3_36bf8dac","updated":"2018-07-24 13:38:25.000000000","message":"Yes","commit_id":"92082bc3a7b29da8b7e29e3cdc70946c374a5672"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"dfcd2ebf9bc2e02fbe3a38cceb4f0d242b3171de","unresolved":false,"context_lines":[{"line_number":4108,"context_line":"        # replacing it with itself (and incrementing the generation)"},{"line_number":4109,"context_line":"        # unnecessarily."},{"line_number":4110,"context_line":"        if not new_inv_list:"},{"line_number":4111,"context_line":"            continue"},{"line_number":4112,"context_line":""},{"line_number":4113,"context_line":"        # A dict, keyed by resource class, of the Inventory objects. We start"},{"line_number":4114,"context_line":"        # with the original inventory list."}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_b6f95d83","line":4111,"updated":"2018-07-24 09:35:46.000000000","message":"If the intent is to optimize wouldn\u0027t it be better to put this before the previous conditional?\n\nAh, I guess because that cache has to be fully hot, for its use in the third block below.","commit_id":"92082bc3a7b29da8b7e29e3cdc70946c374a5672"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"65eed3cb59e54868d996c2b9cda651c83d0a884b","unresolved":false,"context_lines":[{"line_number":4108,"context_line":"        # replacing it with itself (and incrementing the generation)"},{"line_number":4109,"context_line":"        # unnecessarily."},{"line_number":4110,"context_line":"        if not new_inv_list:"},{"line_number":4111,"context_line":"            continue"},{"line_number":4112,"context_line":""},{"line_number":4113,"context_line":"        # A dict, keyed by resource class, of the Inventory objects. We start"},{"line_number":4114,"context_line":"        # with the original inventory list."}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_edddbb3f","line":4111,"in_reply_to":"5f7c97a3_b6f95d83","updated":"2018-07-24 13:38:25.000000000","message":"Right","commit_id":"92082bc3a7b29da8b7e29e3cdc70946c374a5672"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"90fef9793744b52e5d6aea590d639339708fc3b9","unresolved":false,"context_lines":[{"line_number":4099,"context_line":"        else:"},{"line_number":4100,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4101,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4102,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4103,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4104,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4105,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_eeebbaa1","line":4102,"updated":"2018-08-01 19:00:06.000000000","message":"note to reviewers... this can potentially raise NotFound, but the handler code already checks for the existence of the provider here:\n\nhttps://review.openstack.org/#/c/576927/23/nova/api/openstack/placement/handlers/reshaper.py@62\n\nThere is, however, a race possibility where a provider is deleted in between the handler and here. If that\u0027s the case, then reshape() will end up raising NotFound.\n\nAnd in the case that reshape() ends up raising NotFound, the handler will need to be changed to accomodate that and transform into HTTPNotFound appropriately here:\n\nhttps://review.openstack.org/#/c/576927/23/nova/api/openstack/placement/handlers/reshaper.py@103\n\nNot sure we care all that much about addressing this race condition, though. If we do, make sure to add a :raises: NotFound to the docstring above.","commit_id":"b1d9e6cb92592c8374af505750b06837ba4c8f87"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"efe10912c90ddfba351f16d0ef52c79023b79c8a","unresolved":false,"context_lines":[{"line_number":4099,"context_line":"        else:"},{"line_number":4100,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4101,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4102,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4103,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4104,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4105,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_bfe88ddf","line":4102,"in_reply_to":"5f7c97a3_eeebbaa1","updated":"2018-08-01 21:21:19.000000000","message":"There\u0027s a generic top level NotFound handler in handler.py when it calls dispatch(). There because it is a holdover from an earlier time when we thought that a generic NotFound in resource_provider.py was going to be common, so may as well catch it in one place.","commit_id":"b1d9e6cb92592c8374af505750b06837ba4c8f87"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8e32e94eb196cc7ed96e651c18fa4e2c408a506c","unresolved":false,"context_lines":[{"line_number":4192,"context_line":"        else:"},{"line_number":4193,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4194,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4195,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4196,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4197,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4198,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_621e2dbc","line":4195,"updated":"2018-08-23 09:35:48.000000000","message":"I just quickly checked what happens if the rp_uuid is not found. It results in HTTP 400 and that is OK for me.","commit_id":"cf57c8e5af4adb1ee33fbb7cfe8beb73c00e24f0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d65f240acc6c0e82eef2fa49ea31eb4a79be2a68","unresolved":false,"context_lines":[{"line_number":4192,"context_line":"        else:"},{"line_number":4193,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4194,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4195,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4196,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4197,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4198,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_1eb46681","line":4195,"updated":"2018-08-23 19:42:44.000000000","message":"Note that there is a race condition that exists for this particular scenario. The provider generation is checked here:\n\nhttps://review.openstack.org/#/c/576927/35/nova/api/openstack/placement/handlers/reshaper.py@67\n\nhowever, for the scenario where there is no allocation nor inventory block for the resource provider, no resource provider object is passed to the nova.api.openstack.placement.objects.resource_provider.reshape() function. That means that the resource provider object that was used to check the generation on the line above is discarded (unlike when the provider has inventories or allocations in the reshape request payload).\n\nAnother thread could have changed the state of that provider in between line 67 in the reshaper handler and here, and we have no way of determining that or preventing a generation conflict because the reshaper handler isn\u0027t passing the provider object to the reshape() function that was used to do the original generation check in the handler code.","commit_id":"d13e5a5011af75cfb6141a6b5d6aaf0f1a042aeb"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4d74151b2fce82a590e4d97b8befc57f278aa25","unresolved":false,"context_lines":[{"line_number":4192,"context_line":"        else:"},{"line_number":4193,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4194,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4195,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4196,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4197,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4198,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_3294e036","line":4195,"in_reply_to":"3f79a3b5_1eb46681","updated":"2018-08-24 15:08:41.000000000","message":"Got it. Is a ResourceProvider object hashable? I.e. would it be possible to change the inventories dict to be keyed by ResourceProvider instead of rp_uuid?","commit_id":"d13e5a5011af75cfb6141a6b5d6aaf0f1a042aeb"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b58d014d200f95cfd8427cd01545393c85ff2f60","unresolved":false,"context_lines":[{"line_number":4192,"context_line":"        else:"},{"line_number":4193,"context_line":"            # No inventory or allocations for this provider - which makes sense"},{"line_number":4194,"context_line":"            # when we\u0027re \"clearing out\" a provider. Look it up."},{"line_number":4195,"context_line":"            rp \u003d ResourceProvider.get_by_uuid(ctx, rp_uuid)"},{"line_number":4196,"context_line":"        # Update the cache. This may be replacing an entry that came from"},{"line_number":4197,"context_line":"        # allocations, or adding a new entry from inventories and/or db lookup."},{"line_number":4198,"context_line":"        affected_providers[rp_uuid] \u003d rp"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_6236ff05","line":4195,"in_reply_to":"3f79a3b5_3294e036","updated":"2018-08-24 22:08:06.000000000","message":"https://review.openstack.org/596497","commit_id":"d13e5a5011af75cfb6141a6b5d6aaf0f1a042aeb"}],"nova/tests/functional/api/openstack/placement/gabbits/reshaper.yaml":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"285af3721f50c63afb89b49617663fc509a9c662","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                      max_unit: 8"},{"line_number":462,"context_line":"          $ENVIRON[\u0027ALT_RP_UUID\u0027]:"},{"line_number":463,"context_line":"              resource_provider_generation: 3"},{"line_number":464,"context_line":"              # bug https://bugs.launchpad.net/nova/+bug/1783130"},{"line_number":465,"context_line":"              # means that this will cause an IndexError."},{"line_number":466,"context_line":"              inventories: {}"},{"line_number":467,"context_line":"      # these consumer generations are all 1 because they have"},{"line_number":468,"context_line":"      # previously allocated"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"5f7c97a3_6b86ae18","line":465,"range":{"start_line":464,"start_character":13,"end_line":465,"end_character":57},"updated":"2018-08-02 09:08:34.000000000","message":"as this patch fixes the bug you can drop this comment.","commit_id":"52511510c6afe660df84626460a14d020fbdceaf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"93d06b0dd2b899c9b647482980564ca398750a7c","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                      max_unit: 8"},{"line_number":462,"context_line":"          $ENVIRON[\u0027ALT_RP_UUID\u0027]:"},{"line_number":463,"context_line":"              resource_provider_generation: 3"},{"line_number":464,"context_line":"              # bug https://bugs.launchpad.net/nova/+bug/1783130"},{"line_number":465,"context_line":"              # means that this will cause an IndexError."},{"line_number":466,"context_line":"              inventories: {}"},{"line_number":467,"context_line":"      allocations:"},{"line_number":468,"context_line":"          $ENVIRON[\u0027CONSUMER_0\u0027]:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"3f79a3b5_3db04543","line":465,"range":{"start_line":464,"start_character":1,"end_line":465,"end_character":57},"updated":"2018-08-13 13:49:28.000000000","message":"as this patch fixes the bug you can drop this comment.","commit_id":"824c4b5266f78379daa6b5eea05f1c0ebf2515a3"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"6257b08277b770ceccfbd5db63fce4d15e5bd539","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                      max_unit: 8"},{"line_number":462,"context_line":"          $ENVIRON[\u0027ALT_RP_UUID\u0027]:"},{"line_number":463,"context_line":"              resource_provider_generation: 3"},{"line_number":464,"context_line":"              # bug https://bugs.launchpad.net/nova/+bug/1783130"},{"line_number":465,"context_line":"              # means that this will cause an IndexError."},{"line_number":466,"context_line":"              inventories: {}"},{"line_number":467,"context_line":"      allocations:"},{"line_number":468,"context_line":"          $ENVIRON[\u0027CONSUMER_0\u0027]:"}],"source_content_type":"text/x-yaml","patch_set":11,"id":"3f79a3b5_da12c158","line":465,"range":{"start_line":464,"start_character":1,"end_line":465,"end_character":57},"in_reply_to":"3f79a3b5_3db04543","updated":"2018-08-21 17:16:57.000000000","message":"Done","commit_id":"824c4b5266f78379daa6b5eea05f1c0ebf2515a3"}]}
