)]}'
{"nova/cmd/manage.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7a659d40263cbfbb32f3aeaa92300967b7892390","unresolved":false,"context_lines":[{"line_number":1727,"context_line":"            allocations \u003d None"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"        need_healing \u003d False"},{"line_number":1730,"context_line":"        # get_allocations_for_consumer uses safe_connect which will"},{"line_number":1731,"context_line":"        # return None if we can\u0027t communicate with Placement, and the"},{"line_number":1732,"context_line":"        # response can have an empty {\u0027allocations\u0027: {}} response if"},{"line_number":1733,"context_line":"        # there are no allocations for the instance so handle both"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_229509e5","side":"PARENT","line":1730,"updated":"2019-07-08 15:12:51.000000000","message":"OK this comment was from Idba40838b7b1d5389ab308f2ea40e28911aecffa which was back when this code was still using get_allocations_for_consumer which was using safe_connect at the time.","commit_id":"1fdbe88ce7f79e87dfef8147ad24f3e9c63df3a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4f31a93e112528589418a5dea67c110e93f7935c","unresolved":false,"context_lines":[{"line_number":1727,"context_line":"            allocations \u003d None"},{"line_number":1728,"context_line":""},{"line_number":1729,"context_line":"        need_healing \u003d False"},{"line_number":1730,"context_line":"        # get_allocations_for_consumer uses safe_connect which will"},{"line_number":1731,"context_line":"        # return None if we can\u0027t communicate with Placement, and the"},{"line_number":1732,"context_line":"        # response can have an empty {\u0027allocations\u0027: {}} response if"},{"line_number":1733,"context_line":"        # there are no allocations for the instance so handle both"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_352d4595","side":"PARENT","line":1730,"in_reply_to":"7faddb67_229509e5","updated":"2019-07-08 15:26:15.000000000","message":"correct.","commit_id":"1fdbe88ce7f79e87dfef8147ad24f3e9c63df3a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7a659d40263cbfbb32f3aeaa92300967b7892390","unresolved":false,"context_lines":[{"line_number":1731,"context_line":"        # return None if we can\u0027t communicate with Placement, and the"},{"line_number":1732,"context_line":"        # response can have an empty {\u0027allocations\u0027: {}} response if"},{"line_number":1733,"context_line":"        # there are no allocations for the instance so handle both"},{"line_number":1734,"context_line":"        if not allocations or not allocations.get(\u0027allocations\u0027):"},{"line_number":1735,"context_line":"            # This instance doesn\u0027t have allocations"},{"line_number":1736,"context_line":"            need_healing \u003d \u0027Create\u0027"},{"line_number":1737,"context_line":"            allocations \u003d self._heal_missing_alloc(ctxt, instance, node_cache)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_a2b1f991","side":"PARENT","line":1734,"updated":"2019-07-08 15:12:51.000000000","message":"And the logic here was modified in change Iff5b73d8e818fb1145690d0eeff880d98424fa1d, meaning if we get ConsumerAllocationRetrievalFailed from get_allocs_for_consumer we\u0027ll set allocations \u003d None and fall into this block assuming there are no allocations and proceed to overwrite them.\n\nNow, in this case we would be passing consumer_generation\u003dNone to PUT /allocations/{consumer_id} in placement with microversion 1.28 indicating we expect the consumer doesn\u0027t yet exist. If the consumer does exist, won\u0027t we hit a generation conflict error? If so, then the heal_allocations code was already failing if someone hit ConsumerAllocationRetrievalFailed above, but likely failing in a more confusing way, and failing earlier is more clear.","commit_id":"1fdbe88ce7f79e87dfef8147ad24f3e9c63df3a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4f31a93e112528589418a5dea67c110e93f7935c","unresolved":false,"context_lines":[{"line_number":1731,"context_line":"        # return None if we can\u0027t communicate with Placement, and the"},{"line_number":1732,"context_line":"        # response can have an empty {\u0027allocations\u0027: {}} response if"},{"line_number":1733,"context_line":"        # there are no allocations for the instance so handle both"},{"line_number":1734,"context_line":"        if not allocations or not allocations.get(\u0027allocations\u0027):"},{"line_number":1735,"context_line":"            # This instance doesn\u0027t have allocations"},{"line_number":1736,"context_line":"            need_healing \u003d \u0027Create\u0027"},{"line_number":1737,"context_line":"            allocations \u003d self._heal_missing_alloc(ctxt, instance, node_cache)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_95c51991","side":"PARENT","line":1734,"in_reply_to":"7faddb67_a2b1f991","updated":"2019-07-08 15:26:15.000000000","message":"\u003e And the logic here was modified in change Iff5b73d8e818fb1145690d0eeff880d98424fa1d,\n \u003e meaning if we get ConsumerAllocationRetrievalFailed from\n \u003e get_allocs_for_consumer we\u0027ll set allocations \u003d None and fall into\n \u003e this block assuming there are no allocations and proceed to\n \u003e overwrite them.\n\nIt wasn\u0027t Iff5b73d8e818fb1145690d0eeff880d98424fa1d that caused the trouble. \n\nIt was I0e9a804ae7717252175f7fe409223f5eb8f50013 that set the allocation \u003d None an let https://review.opendev.org/#/c/584599/22/nova/cmd/manage.py@1865 re-write the allocation.\n\n \u003e \n \u003e Now, in this case we would be passing consumer_generation\u003dNone to\n \u003e PUT /allocations/{consumer_id} in placement with microversion 1.28\n \u003e indicating we expect the consumer doesn\u0027t yet exist. If the\n \u003e consumer does exist, won\u0027t we hit a generation conflict error? If\n \u003e so, then the heal_allocations code was already failing if someone\n \u003e hit ConsumerAllocationRetrievalFailed above, but likely failing in\n \u003e a more confusing way, and failing earlier is more clear.\n\nGood point about consumer generation. Since https://review.opendev.org/#/c/591647/8/nova/cmd/manage.py@1875 this code uses consumer generation and would fail as you described. \n\nAnd yes I think the current solution cause that it is more clearer for the user reading the error message as well as the developer reading the code to know what caused the problem.","commit_id":"1fdbe88ce7f79e87dfef8147ad24f3e9c63df3a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7a659d40263cbfbb32f3aeaa92300967b7892390","unresolved":false,"context_lines":[{"line_number":1716,"context_line":"            return"},{"line_number":1717,"context_line":""},{"line_number":1718,"context_line":"        try:"},{"line_number":1719,"context_line":"            allocations \u003d placement.get_allocs_for_consumer("},{"line_number":1720,"context_line":"                ctxt, instance.uuid)"},{"line_number":1721,"context_line":"        except (ks_exc.ClientException,"},{"line_number":1722,"context_line":"                exception.ConsumerAllocationRetrievalFailed) as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_828f1dcb","line":1719,"range":{"start_line":1719,"start_character":36,"end_line":1719,"end_character":59},"updated":"2019-07-08 15:12:51.000000000","message":"And we started using this in change I0e9a804ae7717252175f7fe409223f5eb8f50013 to use get_allocs_for_consumer which didn\u0027t use safe_connect and raises ConsumerAllocationRetrievalFailed which should be an error (this patch).","commit_id":"b4ba59261db8438f92a4665bf79fdf5a60777dff"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5469f0d80f56f6f55c3c4d98dd9b7fb771d0b89a","unresolved":false,"context_lines":[{"line_number":1716,"context_line":"            return"},{"line_number":1717,"context_line":""},{"line_number":1718,"context_line":"        try:"},{"line_number":1719,"context_line":"            allocations \u003d placement.get_allocs_for_consumer("},{"line_number":1720,"context_line":"                ctxt, instance.uuid)"},{"line_number":1721,"context_line":"        except (ks_exc.ClientException,"},{"line_number":1722,"context_line":"                exception.ConsumerAllocationRetrievalFailed) as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_b5f5b5cd","line":1719,"range":{"start_line":1719,"start_character":36,"end_line":1719,"end_character":59},"in_reply_to":"7faddb67_828f1dcb","updated":"2019-07-08 15:35:36.000000000","message":"Correct.","commit_id":"b4ba59261db8438f92a4665bf79fdf5a60777dff"}],"nova/tests/unit/test_nova_manage.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7a659d40263cbfbb32f3aeaa92300967b7892390","unresolved":false,"context_lines":[{"line_number":2537,"context_line":"                                                         mock_getinst):"},{"line_number":2538,"context_line":"        self.assertEqual(3, self.cli.heal_allocations())"},{"line_number":2539,"context_line":"        # Verify that the script exited early"},{"line_number":2540,"context_line":"        mock_put.assert_not_called()"},{"line_number":2541,"context_line":""},{"line_number":2542,"context_line":"    @mock.patch(\u0027nova.objects.CellMappingList.get_all\u0027,"},{"line_number":2543,"context_line":"                return_value\u003dobjects.CellMappingList(objects\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_421fe56b","line":2540,"updated":"2019-07-08 15:12:51.000000000","message":"This is redundant with the NonCallableMagicMock, so you could remove one of them.","commit_id":"b4ba59261db8438f92a4665bf79fdf5a60777dff"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5469f0d80f56f6f55c3c4d98dd9b7fb771d0b89a","unresolved":false,"context_lines":[{"line_number":2537,"context_line":"                                                         mock_getinst):"},{"line_number":2538,"context_line":"        self.assertEqual(3, self.cli.heal_allocations())"},{"line_number":2539,"context_line":"        # Verify that the script exited early"},{"line_number":2540,"context_line":"        mock_put.assert_not_called()"},{"line_number":2541,"context_line":""},{"line_number":2542,"context_line":"    @mock.patch(\u0027nova.objects.CellMappingList.get_all\u0027,"},{"line_number":2543,"context_line":"                return_value\u003dobjects.CellMappingList(objects\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_95f8b9c4","line":2540,"in_reply_to":"7faddb67_421fe56b","updated":"2019-07-08 15:35:36.000000000","message":"Done","commit_id":"b4ba59261db8438f92a4665bf79fdf5a60777dff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b88b8c949a792e0ebc5ff428fb76ded0a01a989","unresolved":false,"context_lines":[{"line_number":2531,"context_line":"                new\u003dmock.Mock("},{"line_number":2532,"context_line":"                    side_effect\u003dexception.ConsumerAllocationRetrievalFailed("},{"line_number":2533,"context_line":"                        consumer_uuid\u003d\u0027CONSUMER\u0027, error\u003d\u0027ERROR\u0027)))"},{"line_number":2534,"context_line":"    @mock.patch(\u0027nova.objects.ComputeNode.get_by_host_and_nodename\u0027,"},{"line_number":2535,"context_line":"                new\u003dmock.Mock("},{"line_number":2536,"context_line":"                    return_value\u003dobjects.ComputeNode(uuid\u003duuidsentinel.node)))"},{"line_number":2537,"context_line":"    @mock.patch(\u0027nova.scheduler.utils.resources_from_flavor\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_d5e95180","side":"PARENT","line":2534,"updated":"2019-07-08 15:53:02.000000000","message":"Just a note to myself, if you revert the code change the test fails because this mock is removed because of the DatabasePoisonFixture.","commit_id":"1fdbe88ce7f79e87dfef8147ad24f3e9c63df3a9"}]}