)]}'
{"nova/scheduler/client/report.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d3fdf41b47cdc836dcb463c7de66c11d3fb84007","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1491,"context_line":"                 attempting to perform the necessary API operations, except"},{"line_number":1492,"context_line":"                 reshape (see below)."},{"line_number":1493,"context_line":"        :raises: ReshapeFailed if a reshape was signaled (allocations not None)"},{"line_number":1494,"context_line":"                 and it fails for any reason."},{"line_number":1495,"context_line":"        \"\"\""},{"line_number":1496,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_e6ecf529","line":1493,"range":{"start_line":1493,"start_character":48,"end_line":1493,"end_character":56},"updated":"2018-07-24 18:01:35.000000000","message":"signalled","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b11747602359f9bcc7c6b4501171b75f73ad05fc","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1491,"context_line":"                 attempting to perform the necessary API operations, except"},{"line_number":1492,"context_line":"                 reshape (see below)."},{"line_number":1493,"context_line":"        :raises: ReshapeFailed if a reshape was signaled (allocations not None)"},{"line_number":1494,"context_line":"                 and it fails for any reason."},{"line_number":1495,"context_line":"        \"\"\""},{"line_number":1496,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_d5507d0c","line":1493,"range":{"start_line":1493,"start_character":48,"end_line":1493,"end_character":56},"in_reply_to":"5f7c97a3_c4ef71f8","updated":"2018-07-24 20:58:58.000000000","message":"curious and curiouser\n\nAs has happened before my insistence on style guidance from the new yorker seems to put me at odds with the commonweal.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7aa3dd59aaa4f1505ac3a5f05e9428c790cd9c87","unresolved":false,"context_lines":[{"line_number":1490,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1491,"context_line":"                 attempting to perform the necessary API operations, except"},{"line_number":1492,"context_line":"                 reshape (see below)."},{"line_number":1493,"context_line":"        :raises: ReshapeFailed if a reshape was signaled (allocations not None)"},{"line_number":1494,"context_line":"                 and it fails for any reason."},{"line_number":1495,"context_line":"        \"\"\""},{"line_number":1496,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_c4ef71f8","line":1493,"range":{"start_line":1493,"start_character":48,"end_line":1493,"end_character":56},"in_reply_to":"5f7c97a3_e6ecf529","updated":"2018-07-24 19:47:47.000000000","message":"https://answers.yahoo.com/question/index?qid\u003d20081123135849AAESeK0","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d3fdf41b47cdc836dcb463c7de66c11d3fb84007","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1577,"context_line":"            # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1578,"context_line":"            # where the scheduler could allocate between here and when we"},{"line_number":1579,"context_line":"            # delete the providers."},{"line_number":1580,"context_line":"            for rp_uuid in uuids_to_remove:"},{"line_number":1581,"context_line":"                inventories[rp_uuid] \u003d {"},{"line_number":1582,"context_line":"                    \"inventories\": {},"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_463609b6","line":1579,"updated":"2018-07-24 18:01:35.000000000","message":"Is there ever a case when some, but not all, of a class of inventory, will be moved from a provider A to provider B, but provider A will keep some of it?","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b11747602359f9bcc7c6b4501171b75f73ad05fc","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1577,"context_line":"            # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1578,"context_line":"            # where the scheduler could allocate between here and when we"},{"line_number":1579,"context_line":"            # delete the providers."},{"line_number":1580,"context_line":"            for rp_uuid in uuids_to_remove:"},{"line_number":1581,"context_line":"                inventories[rp_uuid] \u003d {"},{"line_number":1582,"context_line":"                    \"inventories\": {},"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_b5a5c103","line":1579,"in_reply_to":"5f7c97a3_44d28135","updated":"2018-07-24 20:58:58.000000000","message":"It\u0027s an easy confusion to make. \"new\" means (more often) \"didn\u0027t exist before\" not \"from the newer provider tree\"\n\nI agree that this change covers it, but sounds like a good test to make explicit.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7aa3dd59aaa4f1505ac3a5f05e9428c790cd9c87","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1577,"context_line":"            # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1578,"context_line":"            # where the scheduler could allocate between here and when we"},{"line_number":1579,"context_line":"            # delete the providers."},{"line_number":1580,"context_line":"            for rp_uuid in uuids_to_remove:"},{"line_number":1581,"context_line":"                inventories[rp_uuid] \u003d {"},{"line_number":1582,"context_line":"                    \"inventories\": {},"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_44d28135","line":1579,"in_reply_to":"5f7c97a3_463609b6","updated":"2018-07-24 19:47:47.000000000","message":"I would think so, yes. E.g. three NICs, two of which are affined to NUMA nodes, start off on the compute node and then get moved when we convert to NUMA.\n\nAre you suggesting that scenario wouldn\u0027t be covered? I think it is - new_uuids is all the providers in the end picture; not to be confused with uuids_to_add.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bdcf6df92368c807f66a23ceda7616932a2d85fc","unresolved":false,"context_lines":[{"line_number":1576,"context_line":"            # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1577,"context_line":"            # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1578,"context_line":"            # where the scheduler could allocate between here and when we"},{"line_number":1579,"context_line":"            # delete the providers."},{"line_number":1580,"context_line":"            for rp_uuid in uuids_to_remove:"},{"line_number":1581,"context_line":"                inventories[rp_uuid] \u003d {"},{"line_number":1582,"context_line":"                    \"inventories\": {},"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_7a761f0e","line":1579,"in_reply_to":"5f7c97a3_b5a5c103","updated":"2018-07-25 16:21:35.000000000","message":"Working on it.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d3fdf41b47cdc836dcb463c7de66c11d3fb84007","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            with catch_all(uuid) as status:"},{"line_number":1617,"context_line":"                self._delete_provider(uuid)"},{"line_number":1618,"context_line":"            success \u003d success and status.success"},{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1621,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1622,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_a6f27de3","line":1619,"updated":"2018-07-24 18:01:35.000000000","message":"It would be nice to see this entire green chunk as its own method, perhaps?","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bdcf6df92368c807f66a23ceda7616932a2d85fc","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            with catch_all(uuid) as status:"},{"line_number":1617,"context_line":"                self._delete_provider(uuid)"},{"line_number":1618,"context_line":"            success \u003d success and status.success"},{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1621,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1622,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_7f4a915a","line":1619,"in_reply_to":"5f7c97a3_157ef56c","updated":"2018-07-25 16:21:35.000000000","message":"Done.\n\nI don\u0027t think it\u0027s worth testing separately; let me know if you disagree.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"b11747602359f9bcc7c6b4501171b75f73ad05fc","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            with catch_all(uuid) as status:"},{"line_number":1617,"context_line":"                self._delete_provider(uuid)"},{"line_number":1618,"context_line":"            success \u003d success and status.success"},{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1621,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1622,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_157ef56c","line":1619,"in_reply_to":"5f7c97a3_84c0994c","updated":"2018-07-24 20:58:58.000000000","message":"Yeah, it would extract a nice unit","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"7aa3dd59aaa4f1505ac3a5f05e9428c790cd9c87","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            with catch_all(uuid) as status:"},{"line_number":1617,"context_line":"                self._delete_provider(uuid)"},{"line_number":1618,"context_line":"            success \u003d success and status.success"},{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1621,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1622,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_84c0994c","line":1619,"in_reply_to":"5f7c97a3_a6f27de3","updated":"2018-07-24 19:47:47.000000000","message":"1566-1607? Could do.","commit_id":"78b84d9dd682177b231e6e46d3d5031f3db7974a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"                exception.ResourceProviderUpdateFailed,"},{"line_number":1582,"context_line":"                exception.TraitCreationFailed,"},{"line_number":1583,"context_line":"                exception.TraitRetrievalFailed,"},{"line_number":1584,"context_line":"                # NOTE(efried): We do not trap/convert ReshapeFailed - that one"},{"line_number":1585,"context_line":"                # needs to bubble up right away and cause the death of the"},{"line_number":1586,"context_line":"                # compute service."},{"line_number":1587,"context_line":"            )"},{"line_number":1588,"context_line":"            try:"},{"line_number":1589,"context_line":"                yield s"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_fc1e06ba","line":1586,"range":{"start_line":1584,"start_character":0,"end_line":1586,"end_character":34},"updated":"2018-08-02 12:05:09.000000000","message":"Some of the error, but not all of them, can be retried with a chance of success. I\u0027m OK if we let pacemaker do the retry by restarting nova-compute service but I feel we might need to inform the outside world when it make sense to retry and when it doesn\u0027t.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"                exception.ResourceProviderUpdateFailed,"},{"line_number":1582,"context_line":"                exception.TraitCreationFailed,"},{"line_number":1583,"context_line":"                exception.TraitRetrievalFailed,"},{"line_number":1584,"context_line":"                # NOTE(efried): We do not trap/convert ReshapeFailed - that one"},{"line_number":1585,"context_line":"                # needs to bubble up right away and cause the death of the"},{"line_number":1586,"context_line":"                # compute service."},{"line_number":1587,"context_line":"            )"},{"line_number":1588,"context_line":"            try:"},{"line_number":1589,"context_line":"                yield s"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_284bc6b2","line":1586,"range":{"start_line":1584,"start_character":0,"end_line":1586,"end_character":34},"in_reply_to":"5f7c97a3_fc1e06ba","updated":"2018-08-02 22:41:37.000000000","message":"We talked about this in the spec review and decided that failures in the reshaper itself (note that these others can still be raised - nonfatally - from the rest of the method) to kill the caller. And since reshaper should only be running at startup/nova-manage, I\u0027m pretty sure we decided we don\u0027t expect a retry to change anything.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1608,"context_line":"        uuids_to_add \u003d set(new_uuids) - set(old_uuids)"},{"line_number":1609,"context_line":"        uuids_to_remove \u003d set(old_uuids) - set(new_uuids)"},{"line_number":1610,"context_line":""},{"line_number":1611,"context_line":"        # In case a reshape is happening, we first have to create (or load) any"},{"line_number":1612,"context_line":"        # \"new\" providers."},{"line_number":1613,"context_line":"        # We have to do additions in top-down order, so we don\u0027t error"},{"line_number":1614,"context_line":"        # attempting to create a child before its parent exists."}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_3cb31e9b","line":1611,"range":{"start_line":1611,"start_character":66,"end_line":1611,"end_character":75},"updated":"2018-08-02 12:05:09.000000000","message":"What do you mean by load? It seems to me that the below loop only create RPs.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1608,"context_line":"        uuids_to_add \u003d set(new_uuids) - set(old_uuids)"},{"line_number":1609,"context_line":"        uuids_to_remove \u003d set(old_uuids) - set(new_uuids)"},{"line_number":1610,"context_line":""},{"line_number":1611,"context_line":"        # In case a reshape is happening, we first have to create (or load) any"},{"line_number":1612,"context_line":"        # \"new\" providers."},{"line_number":1613,"context_line":"        # We have to do additions in top-down order, so we don\u0027t error"},{"line_number":1614,"context_line":"        # attempting to create a child before its parent exists."}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_48c96222","line":1611,"range":{"start_line":1611,"start_character":66,"end_line":1611,"end_character":75},"in_reply_to":"5f7c97a3_3cb31e9b","updated":"2018-08-02 22:41:37.000000000","message":"This is cut/paste from the original ufpt flow (it used to happen last), but:\n\nWe could be loading e.g. sharing providers that already exist in placement, but that we didn\u0027t have in the ProviderTree cache before this.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1612,"context_line":"        # \"new\" providers."},{"line_number":1613,"context_line":"        # We have to do additions in top-down order, so we don\u0027t error"},{"line_number":1614,"context_line":"        # attempting to create a child before its parent exists."},{"line_number":1615,"context_line":"        for uuid in new_uuids:"},{"line_number":1616,"context_line":"            if uuid not in uuids_to_add:"},{"line_number":1617,"context_line":"                continue"},{"line_number":1618,"context_line":"            provider \u003d new_tree.data(uuid)"},{"line_number":1619,"context_line":"            if not old_tree.exists(uuid):"},{"line_number":1620,"context_line":"                with catch_all(uuid) as status:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_fcfa8641","line":1617,"range":{"start_line":1615,"start_character":0,"end_line":1617,"end_character":24},"updated":"2018-08-02 12:05:09.000000000","message":"uuids_to_add is a subset of new_uuids. This continue statement basically says ignore those uuids from new_uuids that are not in the uuids_to_add. Therefore we are basically processing uuids_to_add only. Therefore you can simply say\n\nfor uuid in uuids_to_add:","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1612,"context_line":"        # \"new\" providers."},{"line_number":1613,"context_line":"        # We have to do additions in top-down order, so we don\u0027t error"},{"line_number":1614,"context_line":"        # attempting to create a child before its parent exists."},{"line_number":1615,"context_line":"        for uuid in new_uuids:"},{"line_number":1616,"context_line":"            if uuid not in uuids_to_add:"},{"line_number":1617,"context_line":"                continue"},{"line_number":1618,"context_line":"            provider \u003d new_tree.data(uuid)"},{"line_number":1619,"context_line":"            if not old_tree.exists(uuid):"},{"line_number":1620,"context_line":"                with catch_all(uuid) as status:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_e8d94ef1","line":1617,"range":{"start_line":1615,"start_character":0,"end_line":1617,"end_character":24},"in_reply_to":"5f7c97a3_fcfa8641","updated":"2018-08-02 22:41:37.000000000","message":"The comment explains why this is here: we have to do the additions in order, and uuids_to_add is a set (therefore no guaranteed order).","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            if uuid not in uuids_to_add:"},{"line_number":1617,"context_line":"                continue"},{"line_number":1618,"context_line":"            provider \u003d new_tree.data(uuid)"},{"line_number":1619,"context_line":"            if not old_tree.exists(uuid):"},{"line_number":1620,"context_line":"                with catch_all(uuid) as status:"},{"line_number":1621,"context_line":"                    self._ensure_resource_provider("},{"line_number":1622,"context_line":"                        context, uuid, name\u003dprovider.name,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_3c41de62","line":1619,"range":{"start_line":1619,"start_character":12,"end_line":1619,"end_character":41},"updated":"2018-08-02 12:05:09.000000000","message":"uuid is from the uuids_to_add and that is new_uuids - old_uuids. Therefore old_tree.exists(uuid) is False. So this if can be removed.\n\nE.g.\nold \u003d old_tree.get_rps() \u003d 1, 2\nnew \u003d new.tree.get_rps() \u003d 2, 3\nadd \u003d new - old \u003d 3\nold_tree.exists(3) \u003d\u003e False","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1616,"context_line":"            if uuid not in uuids_to_add:"},{"line_number":1617,"context_line":"                continue"},{"line_number":1618,"context_line":"            provider \u003d new_tree.data(uuid)"},{"line_number":1619,"context_line":"            if not old_tree.exists(uuid):"},{"line_number":1620,"context_line":"                with catch_all(uuid) as status:"},{"line_number":1621,"context_line":"                    self._ensure_resource_provider("},{"line_number":1622,"context_line":"                        context, uuid, name\u003dprovider.name,"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_c87812c4","line":1619,"range":{"start_line":1619,"start_character":12,"end_line":1619,"end_character":41},"in_reply_to":"5f7c97a3_3c41de62","updated":"2018-08-02 22:41:37.000000000","message":"Done","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1626,"context_line":"                # inventories etc. later."},{"line_number":1627,"context_line":"                # TODO(efried): We don\u0027t have a good way to set the generation"},{"line_number":1628,"context_line":"                # independently; this is a hack."},{"line_number":1629,"context_line":"                new_tree.update_inventory("},{"line_number":1630,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1631,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1632,"context_line":"                success \u003d success and status.success"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_37a019f2","line":1629,"updated":"2018-08-02 12:05:09.000000000","message":"What if the above context manager catched a helper_exception and invalidated the cache? Can we still go and call update_inventory?","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1626,"context_line":"                # inventories etc. later."},{"line_number":1627,"context_line":"                # TODO(efried): We don\u0027t have a good way to set the generation"},{"line_number":1628,"context_line":"                # independently; this is a hack."},{"line_number":1629,"context_line":"                new_tree.update_inventory("},{"line_number":1630,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1631,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1632,"context_line":"                success \u003d success and status.success"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_88121a64","line":1629,"in_reply_to":"5f7c97a3_37a019f2","updated":"2018-08-02 22:41:37.000000000","message":"I think you\u0027re right: this block should be indented.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa7c93024908f49ecff5c3d38eae6e3a057343b3","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"                new_tree.update_inventory("},{"line_number":1630,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1631,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1632,"context_line":"                success \u003d success and status.success"},{"line_number":1633,"context_line":""},{"line_number":1634,"context_line":"        # If we need to reshape, do it here."},{"line_number":1635,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_97b7ad22","line":1632,"range":{"start_line":1632,"start_character":16,"end_line":1632,"end_character":52},"updated":"2018-08-02 12:05:09.000000000","message":"I think we could bail out at the first failure as even if we ever reaches the end of the function after the first failure we will anyhow raise ResourceProviderSyncFailed back to the caller.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e7982511326e34fa0fc2f74aa8fe720c948a4660","unresolved":false,"context_lines":[{"line_number":1629,"context_line":"                new_tree.update_inventory("},{"line_number":1630,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1631,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1632,"context_line":"                success \u003d success and status.success"},{"line_number":1633,"context_line":""},{"line_number":1634,"context_line":"        # If we need to reshape, do it here."},{"line_number":1635,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5f7c97a3_e8238eba","line":1632,"range":{"start_line":1632,"start_character":16,"end_line":1632,"end_character":52},"in_reply_to":"5f7c97a3_97b7ad22","updated":"2018-08-02 22:41:37.000000000","message":"Again, original ufpt code, but:\n\nThe point was to make this method do as much as it can, to minimize the number of retries it takes to make the dust settle, because it used to be that \"retries\" only happened once per periodic. We now have immediate retries at the _update level - but even so, I\u0027d rather minimize the number of retries necessary, because each one does some fairly expensive GETting.","commit_id":"8d44e8502c65284bd36d402559cdad22f113e8a9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1511,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1512,"context_line":""},{"line_number":1513,"context_line":"        # The reshape updated provider generations, so the ones we have in"},{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_85e8ec7b","line":1514,"range":{"start_line":1514,"start_character":35,"end_line":1514,"end_character":61},"updated":"2018-08-15 17:31:16.000000000","message":"which inventory update \"below\" are you referring to?","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1511,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1512,"context_line":""},{"line_number":1513,"context_line":"        # The reshape updated provider generations, so the ones we have in"},{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_8aee38b2","line":1514,"range":{"start_line":1514,"start_character":35,"end_line":1514,"end_character":61},"in_reply_to":"3f79a3b5_85e8ec7b","updated":"2018-08-21 19:48:49.000000000","message":"L1651. This made more sense before I pulled this helper method out. I\u0027m actually thinking this chunk may play better outside of the helper, so this comment will go back to where it\u0027s in the proper context.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_057ffcaa","line":1517,"updated":"2018-08-15 17:31:16.000000000","message":"The provider generation was originally intended as a signal that something changed on the provider. It was originally intended to be a signal that cause the client to re-get its view of the state of an object as the client needed. Doing a forward refresh-provider-state here seems like it isn\u0027t needed.\n\nWhat I mean is that instead of doing this refresh here, technically you could simply ignore the fact that provider generations will have been incremented and deal with it for each provider as needed in future operations that touch that provider (and will end up *either* getting a fresh provider record *or* getting a 409 Conflict from the placement service and needing to refresh the provider generation (and any associated inventory/aggs) anyway.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_cac6704a","line":1517,"in_reply_to":"3f79a3b5_057ffcaa","updated":"2018-08-21 19:48:49.000000000","message":"In this case the generations *have* been incremented for any providers whose inventories/allocations changed. What *should* have happened is that the reshaper API should have returned a payload containing the new generations, so we could go fix up the cache accordingly. Otherwise, any GET we do in-band is (theoretically) subject to a race with some other placement client. (Same reasoning for having added response payloads to aggregate ops, etc.)\n\nFailing that, the next closest thing is to do an immediate re-GET, at least for the providers that changed (I didn\u0027t think it was worth pawing through the inventories dict to figure out which ones those would be - maybe later).\n\nIt seems like you\u0027re suggesting pushing this down to L1651-5, letting those guys bounce 409 (which, again, they\u0027re *guaranteed* to do for any provider whose inventory/allocs changed via the reshape), and re-GETting them there. Or even letting those 409s happen and bubble up - so they can, what, be retried in the broader context at some later time?","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"171be9b387c5f13ade81d63bc027927186a69dcb","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_c8ae38e6","line":1517,"in_reply_to":"3f79a3b5_05ef391d","updated":"2018-08-23 17:03:09.000000000","message":"which?","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2463aff2c9b56ea1b8f69cde950ceb1010e42cc7","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_55c38638","line":1517,"in_reply_to":"3f79a3b5_88ecc0f4","updated":"2018-08-24 20:56:28.000000000","message":"ftr, IRC discussion: http://eavesdrop.openstack.org/irclogs/%23openstack-placement/%23openstack-placement.2018-08-23.log.html#t2018-08-23T18:07:56","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e74121f805e392c09ee39b8aab2cf8d9968093d6","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_88ecc0f4","line":1517,"in_reply_to":"3f79a3b5_c8ae38e6","updated":"2018-08-23 17:22:50.000000000","message":"I\u0027m suggesting only ever reacting to 409s instead of trying to predict when/if another get-my-provider-info is going to be needed.\n\nIs what you\u0027re doing here going to result in provider generations being \"wrong\" in the cache? Yep. Do I think that\u0027s a problem? No.\n\nHope that\u0027s clear.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"67fd2f801b0198982ca8ce2ef328a32d92d72628","unresolved":false,"context_lines":[{"line_number":1514,"context_line":"        # the cache are now stale. The inventory update below will short"},{"line_number":1515,"context_line":"        # out, but we would still bounce with a provider generation"},{"line_number":1516,"context_line":"        # conflict on the trait and aggregate updates."},{"line_number":1517,"context_line":"        self._refresh_provider_generations(context, rp_uuids\u003dnew_uuids)"},{"line_number":1518,"context_line":""},{"line_number":1519,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1520,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_05ef391d","line":1517,"in_reply_to":"3f79a3b5_cac6704a","updated":"2018-08-23 16:22:31.000000000","message":"\u003e It seems like you\u0027re suggesting pushing this down to L1651-5,\n \u003e letting those guys bounce 409 (which, again, they\u0027re *guaranteed*\n \u003e to do for any provider whose inventory/allocs changed via the\n \u003e reshape), and re-GETting them there. Or even letting those 409s\n \u003e happen and bubble up - so they can, what, be retried in the broader\n \u003e context at some later time?\n\nThat is precisely what I\u0027m suggesting, yes.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"                exception.TraitRetrievalFailed,"},{"line_number":1573,"context_line":"                # NOTE(efried): We do not trap/convert ReshapeFailed - that one"},{"line_number":1574,"context_line":"                # needs to bubble up right away and cause the death of the"},{"line_number":1575,"context_line":"                # compute service."},{"line_number":1576,"context_line":"            )"},{"line_number":1577,"context_line":"            try:"},{"line_number":1578,"context_line":"                yield s"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_776ea300","line":1575,"updated":"2018-08-15 17:31:16.000000000","message":"reshape() isn\u0027t called from within a catch_all() manager, so I\u0027m not sure this comment is useful. perhaps it\u0027s leftover from before you pulled _setup_and_do_reshape() into a helper?","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"                exception.TraitRetrievalFailed,"},{"line_number":1573,"context_line":"                # NOTE(efried): We do not trap/convert ReshapeFailed - that one"},{"line_number":1574,"context_line":"                # needs to bubble up right away and cause the death of the"},{"line_number":1575,"context_line":"                # compute service."},{"line_number":1576,"context_line":"            )"},{"line_number":1577,"context_line":"            try:"},{"line_number":1578,"context_line":"                yield s"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_0a178853","line":1575,"in_reply_to":"3f79a3b5_776ea300","updated":"2018-08-21 19:48:49.000000000","message":"The comment is intended to highlight the fact that we\u0027re not catching ReshapeFailed. In order to do so, we would have to both have it here and invoke reshape from within catch_all. The right thing to do is add a corresponding comment for the reshape invocation.\n\nBut you made me realize I should be doing the generation refresh within catch_all, thanks.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"        # Now we can do provider deletions, because we should have moved any"},{"line_number":1628,"context_line":"        # allocations off of them via reshape."},{"line_number":1629,"context_line":"        # We have to do deletions in bottom-up order, so we don\u0027t error"},{"line_number":1630,"context_line":"        # attempting to delete a parent who still has children."},{"line_number":1631,"context_line":"        for uuid in reversed(old_uuids):"},{"line_number":1632,"context_line":"            if uuid not in uuids_to_remove:"},{"line_number":1633,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_d7053715","line":1630,"updated":"2018-08-15 17:31:16.000000000","message":"might be worth mentioning that reversing the list of old_uuids guarantees that the children will be deleted first because ProviderTree.get_provider_uuids() returns an iterable that is sorted from root down to children.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"        # Now we can do provider deletions, because we should have moved any"},{"line_number":1628,"context_line":"        # allocations off of them via reshape."},{"line_number":1629,"context_line":"        # We have to do deletions in bottom-up order, so we don\u0027t error"},{"line_number":1630,"context_line":"        # attempting to delete a parent who still has children."},{"line_number":1631,"context_line":"        for uuid in reversed(old_uuids):"},{"line_number":1632,"context_line":"            if uuid not in uuids_to_remove:"},{"line_number":1633,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_3dec0c28","line":1630,"in_reply_to":"3f79a3b5_d7053715","updated":"2018-08-21 19:48:49.000000000","message":"done","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"93d912528a798d9560c677a3998b8607f03fe549","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"        # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1471,"context_line":"        # where the scheduler could allocate between here and when we"},{"line_number":1472,"context_line":"        # delete the providers."},{"line_number":1473,"context_line":"        for rp_uuid in set(old_tree.get_provider_uuids()) - set(new_uuids):"},{"line_number":1474,"context_line":"            inventories[rp_uuid] \u003d {"},{"line_number":1475,"context_line":"                \"inventories\": {},"},{"line_number":1476,"context_line":"                \"resource_provider_generation\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_09d2d6d1","line":1473,"range":{"start_line":1473,"start_character":23,"end_line":1473,"end_character":74},"updated":"2018-08-24 20:15:55.000000000","message":"nit: might be easier to read to do:\n\n to_remove \u003d set(old_tree.get_provider_uuids() - set(new_uuids)\n for rp_uuid in to_remove:\n     ...","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2463aff2c9b56ea1b8f69cde950ceb1010e42cc7","unresolved":false,"context_lines":[{"line_number":1470,"context_line":"        # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1471,"context_line":"        # where the scheduler could allocate between here and when we"},{"line_number":1472,"context_line":"        # delete the providers."},{"line_number":1473,"context_line":"        for rp_uuid in set(old_tree.get_provider_uuids()) - set(new_uuids):"},{"line_number":1474,"context_line":"            inventories[rp_uuid] \u003d {"},{"line_number":1475,"context_line":"                \"inventories\": {},"},{"line_number":1476,"context_line":"                \"resource_provider_generation\":"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_07587df7","line":1473,"range":{"start_line":1473,"start_character":23,"end_line":1473,"end_character":74},"in_reply_to":"3f79a3b5_09d2d6d1","updated":"2018-08-24 20:56:28.000000000","message":"Done","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7fb1bdd86307739631f4fdfd3881e1bcb02b199d","unresolved":false,"context_lines":[{"line_number":1589,"context_line":"                new_tree.update_inventory("},{"line_number":1590,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1591,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1592,"context_line":"                success \u003d success and status.success"},{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_30a05f90","line":1592,"updated":"2018-08-24 12:01:25.000000000","message":"catch_all() converts exceptions to a boolean False without leaving any trace in the log about what was the cause of the failure. So when at the end we raise ResourceProviderSyncFailed() there is really hard to figure out what operation failed and why.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3c022223827a471053e0b4024e3aacea86def520","unresolved":false,"context_lines":[{"line_number":1589,"context_line":"                new_tree.update_inventory("},{"line_number":1590,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1591,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1592,"context_line":"                success \u003d success and status.success"},{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_8f7fd113","line":1592,"in_reply_to":"3f79a3b5_30a05f90","updated":"2018-08-24 14:56:20.000000000","message":"tbc, this catch_all doesn\u0027t catch exceptions from ProviderTree.update_inventory - just from _ensure_resource_provider. And _ensure_resource_provider should be emitting logs for whatever went wrong there. (When I was developing the original ufpt, I tried having catch_all emit a log, but it turned out to be doubling and overly verbose.)\n\nThat said, there does appear to be one error path under _ensure_resource_provider that wouldn\u0027t log - namely _get_inventory. We should add logging there. (Possibly in a fup?)","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7f9e91ef36433e480bde7f282d960a4689d4a59f","unresolved":false,"context_lines":[{"line_number":1589,"context_line":"                new_tree.update_inventory("},{"line_number":1590,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1591,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1592,"context_line":"                success \u003d success and status.success"},{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_2859938d","line":1592,"in_reply_to":"3f79a3b5_470755cb","updated":"2018-08-28 15:39:08.000000000","message":"fupping this is OK. \n\nIn more general sense, if later somebody introduce a new error path in update_inventory that raises an exception that is in catch_all then we have the silencing issue of catch_all which feels a bad coupling to me But we don\u0027t have to solve this general thing here.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2463aff2c9b56ea1b8f69cde950ceb1010e42cc7","unresolved":false,"context_lines":[{"line_number":1589,"context_line":"                new_tree.update_inventory("},{"line_number":1590,"context_line":"                    uuid, new_tree.data(uuid).inventory,"},{"line_number":1591,"context_line":"                    generation\u003dself._provider_tree.data(uuid).generation)"},{"line_number":1592,"context_line":"                success \u003d success and status.success"},{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_470755cb","line":1592,"in_reply_to":"3f79a3b5_8f7fd113","updated":"2018-08-24 20:56:28.000000000","message":"Added TODO for the log in _get_inventory","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7fb1bdd86307739631f4fdfd3881e1bcb02b199d","unresolved":false,"context_lines":[{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"},{"line_number":1596,"context_line":"            # NOTE(efried): We do not catch_all here, because ReshapeFailed"},{"line_number":1597,"context_line":"            # needs to bubble up right away and be handled specially."},{"line_number":1598,"context_line":"            self._set_up_and_do_reshape(context, old_tree, new_tree,"},{"line_number":1599,"context_line":"                                        allocations)"},{"line_number":1600,"context_line":"            # The reshape updated provider generations, so the ones we have in"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_50b2bb07","line":1597,"range":{"start_line":1596,"start_character":12,"end_line":1597,"end_character":69},"updated":"2018-08-24 12:01:25.000000000","message":"But catch_all does not catch ReshapeFailed for the exact same reason so we cloud use catch_all here.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3c022223827a471053e0b4024e3aacea86def520","unresolved":false,"context_lines":[{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"},{"line_number":1596,"context_line":"            # NOTE(efried): We do not catch_all here, because ReshapeFailed"},{"line_number":1597,"context_line":"            # needs to bubble up right away and be handled specially."},{"line_number":1598,"context_line":"            self._set_up_and_do_reshape(context, old_tree, new_tree,"},{"line_number":1599,"context_line":"                                        allocations)"},{"line_number":1600,"context_line":"            # The reshape updated provider generations, so the ones we have in"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_afd72dbf","line":1597,"range":{"start_line":1596,"start_character":12,"end_line":1597,"end_character":69},"in_reply_to":"3f79a3b5_50b2bb07","updated":"2018-08-24 14:56:20.000000000","message":"Well, it would be a no-op (since catch_all does not catch ReshapeFailed).\n\nThe point of this comment, along with the one on L1546-7, is to let the reader know that we\u0027re deliberately not using catch_all, and why. Omitting either comment would make the reader have to do extra sleuthing if they noticed it.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7f9e91ef36433e480bde7f282d960a4689d4a59f","unresolved":false,"context_lines":[{"line_number":1593,"context_line":""},{"line_number":1594,"context_line":"        # If we need to reshape, do it here."},{"line_number":1595,"context_line":"        if allocations is not None:"},{"line_number":1596,"context_line":"            # NOTE(efried): We do not catch_all here, because ReshapeFailed"},{"line_number":1597,"context_line":"            # needs to bubble up right away and be handled specially."},{"line_number":1598,"context_line":"            self._set_up_and_do_reshape(context, old_tree, new_tree,"},{"line_number":1599,"context_line":"                                        allocations)"},{"line_number":1600,"context_line":"            # The reshape updated provider generations, so the ones we have in"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_0850575e","line":1597,"range":{"start_line":1596,"start_character":12,"end_line":1597,"end_character":69},"in_reply_to":"3f79a3b5_afd72dbf","updated":"2018-08-28 15:39:08.000000000","message":"fair point.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        \"\"\""},{"line_number":1476,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1477,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1478,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1479,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"        @contextlib.contextmanager"},{"line_number":1482,"context_line":"        def catch_all(rp_uuid):"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_1425d45c","side":"PARENT","line":1479,"range":{"start_line":1478,"start_character":27,"end_line":1479,"end_character":79},"updated":"2018-08-28 14:23:52.000000000","message":"Removing this seems unrelated. If this *is* possible now, then shouldn\u0027t the whole NOTE comment be removed?","commit_id":"2833785f5910d115754c449bc0f0fa2a99aa7239"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        \"\"\""},{"line_number":1476,"context_line":"        # NOTE(efried): We currently do not handle the \"rename\" case.  This is"},{"line_number":1477,"context_line":"        # where new_tree contains a provider named Y whose UUID already exists"},{"line_number":1478,"context_line":"        # but is named X.  Today the only way the consumer could accomplish"},{"line_number":1479,"context_line":"        # this is by deleting the provider and recreating it with the new name."},{"line_number":1480,"context_line":""},{"line_number":1481,"context_line":"        @contextlib.contextmanager"},{"line_number":1482,"context_line":"        def catch_all(rp_uuid):"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_fdfd9b3b","side":"PARENT","line":1479,"range":{"start_line":1478,"start_character":27,"end_line":1479,"end_character":79},"in_reply_to":"3f79a3b5_1425d45c","updated":"2018-08-28 15:34:11.000000000","message":"The first bit still applies. You can\u0027t rename a provider in place.\n\nI removed the second bit because it is (and actually always was) kind of incomplete. You can\u0027t just remove and replace a provider - what if it has allocations? So you would have to do a reshape to do a \"rename\" (by replacing). But you can only do a reshape under certain circumstances. And why would you want to rename a provider anyway? If there comes to be a serious use case for that, then we should make support for it natively rather than requiring a reshape to do it.\n\nAnd I didn\u0027t think it was worth trying to explain all of that in this note. Easier to just say, \"don\u0027t do that\".","commit_id":"2833785f5910d115754c449bc0f0fa2a99aa7239"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1523,"context_line":"        old_uuids \u003d old_tree.get_provider_uuids()"},{"line_number":1524,"context_line":"        new_uuids \u003d new_tree.get_provider_uuids()"},{"line_number":1525,"context_line":""},{"line_number":1526,"context_line":"        # Do provider deletion first, since it has the best chance of failing"},{"line_number":1527,"context_line":"        # for non-generation-conflict reasons (i.e. allocations)."},{"line_number":1528,"context_line":"        uuids_to_remove \u003d set(old_uuids) - set(new_uuids)"},{"line_number":1529,"context_line":"        # We have to do deletions in bottom-up order, so we don\u0027t error"},{"line_number":1530,"context_line":"        # attempting to delete a parent who still has children."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_341010a9","side":"PARENT","line":1527,"range":{"start_line":1526,"start_character":8,"end_line":1527,"end_character":65},"updated":"2018-08-28 14:23:52.000000000","message":"Would there be any reason to leave this intact in the case that we aren\u0027t doing reshape? Or would that be a premature optimization?","commit_id":"2833785f5910d115754c449bc0f0fa2a99aa7239"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1523,"context_line":"        old_uuids \u003d old_tree.get_provider_uuids()"},{"line_number":1524,"context_line":"        new_uuids \u003d new_tree.get_provider_uuids()"},{"line_number":1525,"context_line":""},{"line_number":1526,"context_line":"        # Do provider deletion first, since it has the best chance of failing"},{"line_number":1527,"context_line":"        # for non-generation-conflict reasons (i.e. allocations)."},{"line_number":1528,"context_line":"        uuids_to_remove \u003d set(old_uuids) - set(new_uuids)"},{"line_number":1529,"context_line":"        # We have to do deletions in bottom-up order, so we don\u0027t error"},{"line_number":1530,"context_line":"        # attempting to delete a parent who still has children."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_9d04a71a","side":"PARENT","line":1527,"range":{"start_line":1526,"start_character":8,"end_line":1527,"end_character":65},"in_reply_to":"3f79a3b5_341010a9","updated":"2018-08-28 15:34:11.000000000","message":"You mean use the original code path if not reshaping, and the new code path if reshaping? I thought we would prefer to have one flow for this (already really complicated) method for the sake of maintainability/serviceability.\n\nI\u0027m actually wondering if this was bogus reasoning to begin with, an artifact left over from before I did the best-effort thing with catch_all.\n\nBTW, I\u0027m not at all worried about \"optimization\" here. Actual restructuring of the tree (adding/deleting resource providers) should be a vanishingly rare event.","commit_id":"2833785f5910d115754c449bc0f0fa2a99aa7239"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"            }"},{"line_number":1470,"context_line":"        # Even though we\u0027re going to delete them immediately, we still want"},{"line_number":1471,"context_line":"        # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1472,"context_line":"        # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1473,"context_line":"        # where the scheduler could allocate between here and when we"},{"line_number":1474,"context_line":"        # delete the providers."},{"line_number":1475,"context_line":"        to_remove \u003d set(old_tree.get_provider_uuids()) - set(new_uuids)"},{"line_number":1476,"context_line":"        for rp_uuid in to_remove:"},{"line_number":1477,"context_line":"            inventories[rp_uuid] \u003d {"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_146ab4ec","line":1474,"range":{"start_line":1472,"start_character":54,"end_line":1474,"end_character":31},"updated":"2018-08-28 14:23:52.000000000","message":"Does it really prevent races? I mean, couldn\u0027t the scheduler have selected the rp that we\u0027re going to remove based on it\u0027s current inventory and then we wipe out that inventory? Since the scheduler would need to create allocations against that provider and inventory, we should be moving those allocations to some other provider in the tree, so wiping out this inventory is I guess OK.\n\nThis reminds me of some code I saw in the starlingx fork where they somehow flag compute nodes as being patched/upgraded and weigh those differently in the scheduler so that nodes going through an upgrade, or are not fully patched yet, get weighed lower during scheduling...seems like something useful in this case, i.e. we don\u0027t really want the scheduler picking a compute node that\u0027s undergoing a reshape but I don\u0027t think we have anything that prevents it today either. Did we ever talk about auto-disabling a compute while going through reshape as part of the spec review?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"            }"},{"line_number":1470,"context_line":"        # Even though we\u0027re going to delete them immediately, we still want"},{"line_number":1471,"context_line":"        # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1472,"context_line":"        # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1473,"context_line":"        # where the scheduler could allocate between here and when we"},{"line_number":1474,"context_line":"        # delete the providers."},{"line_number":1475,"context_line":"        to_remove \u003d set(old_tree.get_provider_uuids()) - set(new_uuids)"},{"line_number":1476,"context_line":"        for rp_uuid in to_remove:"},{"line_number":1477,"context_line":"            inventories[rp_uuid] \u003d {"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_c349e0b7","line":1474,"range":{"start_line":1472,"start_character":54,"end_line":1474,"end_character":31},"in_reply_to":"3f79a3b5_0864d7e4","updated":"2018-08-28 16:23:56.000000000","message":"\u003e Doesn\u0027t sound familiar, but seems like not a horrible idea. Something we could add later, yah?\n\nOr we could add a weigher that weighs older computes to be selected after newer computes...we have that information in the nova-compute service version record.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"            }"},{"line_number":1470,"context_line":"        # Even though we\u0027re going to delete them immediately, we still want"},{"line_number":1471,"context_line":"        # to send \"inventory changes\" for to-be-removed providers in this"},{"line_number":1472,"context_line":"        # reshape request so they\u0027re done atomically. This prevents races"},{"line_number":1473,"context_line":"        # where the scheduler could allocate between here and when we"},{"line_number":1474,"context_line":"        # delete the providers."},{"line_number":1475,"context_line":"        to_remove \u003d set(old_tree.get_provider_uuids()) - set(new_uuids)"},{"line_number":1476,"context_line":"        for rp_uuid in to_remove:"},{"line_number":1477,"context_line":"            inventories[rp_uuid] \u003d {"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_0864d7e4","line":1474,"range":{"start_line":1472,"start_character":54,"end_line":1474,"end_character":31},"in_reply_to":"3f79a3b5_146ab4ec","updated":"2018-08-28 15:34:11.000000000","message":"\u003e Does it really prevent races? I mean, couldn\u0027t the scheduler have\n \u003e selected the rp that we\u0027re going to remove based on it\u0027s current\n \u003e inventory and then we wipe out that inventory? Since the scheduler\n \u003e would need to create allocations against that provider and\n \u003e inventory, we should be moving those allocations to some other\n \u003e provider in the tree, so wiping out this inventory is I guess OK.\n\nPerhaps I didn\u0027t explain it especially well.\n\nThe race I\u0027m talking about would be if we ignored to-be-deleted providers here. We do the reshape, which moves all allocations off of that provider. Then another thread schedules to that provider and creates allocations against that inventory. Then the next part of update_from_provider_tree goes to delete the provider, but bounces InventoryInUse.\n\nBy clearing the inventory as part of the reshape, we prevent that allocation from happening at all, and the the deletion can go through.\n\n \u003e Did we ever talk about auto-disabling a\n \u003e compute while going through reshape as part of the spec review?\n\nDoesn\u0027t sound familiar, but seems like not a horrible idea. Something we could add later, yah?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        to_remove \u003d set(old_tree.get_provider_uuids()) - set(new_uuids)"},{"line_number":1476,"context_line":"        for rp_uuid in to_remove:"},{"line_number":1477,"context_line":"            inventories[rp_uuid] \u003d {"},{"line_number":1478,"context_line":"                \"inventories\": {},"},{"line_number":1479,"context_line":"                \"resource_provider_generation\":"},{"line_number":1480,"context_line":"                    old_tree.data(rp_uuid).generation"},{"line_number":1481,"context_line":"            }"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_f48298a5","line":1478,"range":{"start_line":1478,"start_character":31,"end_line":1478,"end_character":33},"updated":"2018-08-28 14:23:52.000000000","message":"So we aren\u0027t actually sending \"inventory changes\" for this provider as the comment says, we\u0027re wiping out the inventory for the provider.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1475,"context_line":"        to_remove \u003d set(old_tree.get_provider_uuids()) - set(new_uuids)"},{"line_number":1476,"context_line":"        for rp_uuid in to_remove:"},{"line_number":1477,"context_line":"            inventories[rp_uuid] \u003d {"},{"line_number":1478,"context_line":"                \"inventories\": {},"},{"line_number":1479,"context_line":"                \"resource_provider_generation\":"},{"line_number":1480,"context_line":"                    old_tree.data(rp_uuid).generation"},{"line_number":1481,"context_line":"            }"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_a8b5a334","line":1478,"range":{"start_line":1478,"start_character":31,"end_line":1478,"end_character":33},"in_reply_to":"3f79a3b5_f48298a5","updated":"2018-08-28 15:34:11.000000000","message":"We\u0027re changing the inventory. To be empty. Swhy I put it in quotes.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1489,"context_line":"            raise"},{"line_number":1490,"context_line":"        except Exception as e:"},{"line_number":1491,"context_line":"            # Make sure the original stack trace gets logged."},{"line_number":1492,"context_line":"            LOG.exception(\u0027Reshape failed\u0027)"},{"line_number":1493,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1496,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_942b2494","line":1493,"range":{"start_line":1492,"start_character":12,"end_line":1493,"end_character":50},"updated":"2018-08-28 14:23:52.000000000","message":"nit: seems excutils.save_and_reraise_exception would be better here to not lose context, but I might be over-thinking this.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1489,"context_line":"            raise"},{"line_number":1490,"context_line":"        except Exception as e:"},{"line_number":1491,"context_line":"            # Make sure the original stack trace gets logged."},{"line_number":1492,"context_line":"            LOG.exception(\u0027Reshape failed\u0027)"},{"line_number":1493,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1496,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_83f2086f","line":1493,"range":{"start_line":1492,"start_character":12,"end_line":1493,"end_character":50},"in_reply_to":"3f79a3b5_6881cb4d","updated":"2018-08-28 16:23:56.000000000","message":"You can override how save_and_reraise_exception works to provide your own exception to raise out of the context manager.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0345f132e547f23654966ff853040d154ee1cdb7","unresolved":false,"context_lines":[{"line_number":1489,"context_line":"            raise"},{"line_number":1490,"context_line":"        except Exception as e:"},{"line_number":1491,"context_line":"            # Make sure the original stack trace gets logged."},{"line_number":1492,"context_line":"            LOG.exception(\u0027Reshape failed\u0027)"},{"line_number":1493,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1496,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_0e361fe9","line":1493,"range":{"start_line":1492,"start_character":12,"end_line":1493,"end_character":50},"in_reply_to":"3f79a3b5_83f2086f","updated":"2018-08-28 16:33:48.000000000","message":"So how would that help us here?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1489,"context_line":"            raise"},{"line_number":1490,"context_line":"        except Exception as e:"},{"line_number":1491,"context_line":"            # Make sure the original stack trace gets logged."},{"line_number":1492,"context_line":"            LOG.exception(\u0027Reshape failed\u0027)"},{"line_number":1493,"context_line":"            raise exception.ReshapeFailed(error\u003de)"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"    def update_from_provider_tree(self, context, new_tree, allocations\u003dNone):"},{"line_number":1496,"context_line":"        \"\"\"Flush changes from a specified ProviderTree back to placement."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_6881cb4d","line":1493,"range":{"start_line":1492,"start_character":12,"end_line":1493,"end_character":50},"in_reply_to":"3f79a3b5_942b2494","updated":"2018-08-28 15:34:11.000000000","message":"save_and_reraise would raise the original exception, which we don\u0027t want. We want to convert to ReshapeFailed. Because that specific exception is the resource tracker\u0027s signal for special behavior.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1627,"context_line":""},{"line_number":1628,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1629,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1630,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"},{"line_number":1631,"context_line":"        # reshaped above, any inventory changes have already been done. But the"},{"line_number":1632,"context_line":"        # helper methods are set up to check and short out when the relevant"},{"line_number":1633,"context_line":"        # property does not differ from what\u0027s in the cache."},{"line_number":1634,"context_line":"        # If we encounter any error and remove a provider from the cache, all"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_37b492d1","line":1631,"range":{"start_line":1630,"start_character":60,"end_line":1631,"end_character":71},"updated":"2018-08-28 14:23:52.000000000","message":"So what happens if we created a new child provider, moved inventory from the root to the child (like VGPU for libvirt/xenapi), and then blew up on updating the new child below and then catch_all deletes it? Or does catch_all only remove that new child provider from the cache, but not actually remove the provider from placement? I\u0027m guessing the new child provider couldn\u0027t be removed after reshape passes because we\u0027d get a 409 conflict on trying to remove the provider since it should have allocations against it.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1627,"context_line":""},{"line_number":1628,"context_line":"        # At this point the local cache should have all the same providers as"},{"line_number":1629,"context_line":"        # new_tree.  Whether we added them or not, walk through and diff/flush"},{"line_number":1630,"context_line":"        # inventories, traits, and aggregates as necessary. Note that, if we"},{"line_number":1631,"context_line":"        # reshaped above, any inventory changes have already been done. But the"},{"line_number":1632,"context_line":"        # helper methods are set up to check and short out when the relevant"},{"line_number":1633,"context_line":"        # property does not differ from what\u0027s in the cache."},{"line_number":1634,"context_line":"        # If we encounter any error and remove a provider from the cache, all"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_48f5efec","line":1631,"range":{"start_line":1630,"start_character":60,"end_line":1631,"end_character":71},"in_reply_to":"3f79a3b5_37b492d1","updated":"2018-08-28 15:34:11.000000000","message":"Right, catch_all only removes it from the cache so that the next iteration re-GETs it and we\u0027ll redrive this bit of the algorithm with freshly-calculated inventory data from update_provider_tree. This is the \"self-healing\" aspect of this method.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"}],"nova/tests/functional/test_report_client.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"               \u0027SRIOV_NET_VF\u0027: {\u0027total\u0027: 2}}"},{"line_number":1026,"context_line":"        ptree.update_inventory(self.compute_uuid, inv)"},{"line_number":1027,"context_line":"        ptree.update_aggregates(self.compute_uuid, [uuids.agg1])"},{"line_number":1028,"context_line":"        ret[self.compute_uuid] \u003d dict("},{"line_number":1029,"context_line":"            name\u003dself.compute_name,"},{"line_number":1030,"context_line":"            parent_uuid\u003dNone,"},{"line_number":1031,"context_line":"            inventory\u003dinv,"},{"line_number":1032,"context_line":"            aggregates\u003dset([uuids.agg1]),"},{"line_number":1033,"context_line":"            traits\u003dset()"},{"line_number":1034,"context_line":"        )"},{"line_number":1035,"context_line":""},{"line_number":1036,"context_line":"        # These are part of the compute node\u0027s tree"},{"line_number":1037,"context_line":"        ptree.new_child(\u0027numa1\u0027, self.compute_uuid, uuid\u003duuids.numa1)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_5a55cef8","line":1034,"range":{"start_line":1028,"start_character":33,"end_line":1034,"end_character":9},"updated":"2018-08-15 17:31:16.000000000","message":"would be great to be consistent and use either dict() or {} notation, but not both.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"               \u0027SRIOV_NET_VF\u0027: {\u0027total\u0027: 2}}"},{"line_number":1026,"context_line":"        ptree.update_inventory(self.compute_uuid, inv)"},{"line_number":1027,"context_line":"        ptree.update_aggregates(self.compute_uuid, [uuids.agg1])"},{"line_number":1028,"context_line":"        ret[self.compute_uuid] \u003d dict("},{"line_number":1029,"context_line":"            name\u003dself.compute_name,"},{"line_number":1030,"context_line":"            parent_uuid\u003dNone,"},{"line_number":1031,"context_line":"            inventory\u003dinv,"},{"line_number":1032,"context_line":"            aggregates\u003dset([uuids.agg1]),"},{"line_number":1033,"context_line":"            traits\u003dset()"},{"line_number":1034,"context_line":"        )"},{"line_number":1035,"context_line":""},{"line_number":1036,"context_line":"        # These are part of the compute node\u0027s tree"},{"line_number":1037,"context_line":"        ptree.new_child(\u0027numa1\u0027, self.compute_uuid, uuid\u003duuids.numa1)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_3d8dec30","line":1034,"range":{"start_line":1028,"start_character":33,"end_line":1034,"end_character":9},"in_reply_to":"3f79a3b5_5a55cef8","updated":"2018-08-21 19:48:49.000000000","message":"This is the Perl programmer in me coming out. Use the syntax (and formatting) that\u0027s most readable on the spot, and rely on the reader being familiar enough with the syntax to not even notice :)\n\nDone here, but I will be sad if you make me do this every time.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0a646d78d4b0e0ea30e388b231fe3a92d845b922","unresolved":false,"context_lines":[{"line_number":1251,"context_line":""},{"line_number":1252,"context_line":"    def test_update_from_provider_tree_reshape(self):"},{"line_number":1253,"context_line":"        \"\"\"Run update_from_provider_tree with reshaping.\"\"\""},{"line_number":1254,"context_line":"        @contextlib.contextmanager"},{"line_number":1255,"context_line":"        def _do_ufpt(expected_ptree_dict, expected_allocs):"},{"line_number":1256,"context_line":"            # Need to do these afresh each time to make sure we have the latest"},{"line_number":1257,"context_line":"            # generations"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_3a48122b","line":1254,"updated":"2018-08-15 17:31:16.000000000","message":"as mentioned in the powervm patch that you used the same technique, I find this hard to read and unncessarily clever. There\u0027s nothing wrong in my mind with having a _do_test() functor that does the needful assertions. But having a context manager that yields mocks that the caller changes inside the run scope of the context manager doesn\u0027t help with readability IMHO. I\u0027m much more in favor of a little repetitive setup code with regards to test fixtures or expectations versus trying to DRY up everything to the point where it\u0027s difficult to follow what a particular helper function does (or it does multiple things...)","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b52b68df68bae1368565d6d2e894c5f017861459","unresolved":false,"context_lines":[{"line_number":1251,"context_line":""},{"line_number":1252,"context_line":"    def test_update_from_provider_tree_reshape(self):"},{"line_number":1253,"context_line":"        \"\"\"Run update_from_provider_tree with reshaping.\"\"\""},{"line_number":1254,"context_line":"        @contextlib.contextmanager"},{"line_number":1255,"context_line":"        def _do_ufpt(expected_ptree_dict, expected_allocs):"},{"line_number":1256,"context_line":"            # Need to do these afresh each time to make sure we have the latest"},{"line_number":1257,"context_line":"            # generations"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_1df03067","line":1254,"in_reply_to":"3f79a3b5_3a48122b","updated":"2018-08-21 19:48:49.000000000","message":"\u003e unnecessarily clever\n\nAt least you find it clever.\n\n \u003e repetitive\n\nDone.","commit_id":"8798589f8c1482626ed4ffd06e585a44d476ff1f"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"93d912528a798d9560c677a3998b8607f03fe549","unresolved":false,"context_lines":[{"line_number":1405,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("},{"line_number":1406,"context_line":"                self.context, self.compute_name)"},{"line_number":1407,"context_line":"            self.assertProviderTree(orig_exp_ptree, ptree)"},{"line_number":1408,"context_line":"            self.assertAllocations(orig_exp_allocs, allocs)"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_4c89bc72","line":1408,"updated":"2018-08-24 20:15:55.000000000","message":"I actually think this is a good functional test. It shows reshaping to a new model and then reshaping back to the original model. That\u0027s a good test.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7fb1bdd86307739631f4fdfd3881e1bcb02b199d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3f79a3b5_f0be67ac","line":1409,"updated":"2018-08-24 12:01:25.000000000","message":"I feel that we only test the happy path here.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2463aff2c9b56ea1b8f69cde950ceb1010e42cc7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3f79a3b5_c7a4c5a8","line":1409,"in_reply_to":"3f79a3b5_0f33c115","updated":"2018-08-24 20:56:28.000000000","message":"I injected an error path check in a convenient spot above. You like?","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7f9e91ef36433e480bde7f282d960a4689d4a59f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3f79a3b5_a8e1239e","line":1409,"in_reply_to":"3f79a3b5_c7a4c5a8","updated":"2018-08-28 15:39:08.000000000","message":"Thank you. :)","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3c022223827a471053e0b4024e3aacea86def520","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3f79a3b5_0f33c115","line":1409,"in_reply_to":"3f79a3b5_f0be67ac","updated":"2018-08-24 14:56:20.000000000","message":"The existing tests for ufpt test the paths - happy and non - that don\u0027t involve reshaper. (The delta in report.py looks bigger than it really is - much of it is code cut/paste because I had to reverse the order of operations.)\n\nSo I think you\u0027re suggesting that we add a test where reshaper fails and make sure ufpt reacts appropriately?\n\nThat code path is covered by tests elsewhere in the series, but I can add a test case here if you think it\u0027s necessary.","commit_id":"f39d8dba0ad8650036658c78c47c6433e852083b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            self.client.get_allocation_candidates("},{"line_number":1036,"context_line":"                self.context, utils.ResourceRequest())"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"    def _set_up_provider_tree(self):"},{"line_number":1039,"context_line":"        \"\"\"Create two compute nodes in placement: \"this\" one, and another one."},{"line_number":1040,"context_line":""},{"line_number":1041,"context_line":"        Must be invoked from within an _interceptor() context."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_974c86c9","line":1038,"updated":"2018-08-28 14:23:52.000000000","message":"nit: it would be helpful if this docstring contained an ascii-style diagram of the provider tree it\u0027s building, because it\u0027s hard to track it in my head by just looking through the method.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"192864e7a7cd9b3c656d7d61e1ecc8933428dbf6","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            self.client.get_allocation_candidates("},{"line_number":1036,"context_line":"                self.context, utils.ResourceRequest())"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"    def _set_up_provider_tree(self):"},{"line_number":1039,"context_line":"        \"\"\"Create two compute nodes in placement: \"this\" one, and another one."},{"line_number":1040,"context_line":""},{"line_number":1041,"context_line":"        Must be invoked from within an _interceptor() context."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_c5ea080d","line":1038,"in_reply_to":"3f79a3b5_839768ec","updated":"2018-08-30 19:22:29.000000000","message":"Done","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            self.client.get_allocation_candidates("},{"line_number":1036,"context_line":"                self.context, utils.ResourceRequest())"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"    def _set_up_provider_tree(self):"},{"line_number":1039,"context_line":"        \"\"\"Create two compute nodes in placement: \"this\" one, and another one."},{"line_number":1040,"context_line":""},{"line_number":1041,"context_line":"        Must be invoked from within an _interceptor() context."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_a8e36334","line":1038,"in_reply_to":"3f79a3b5_974c86c9","updated":"2018-08-28 15:34:11.000000000","message":"Yup, good call. FUP okay?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1035,"context_line":"            self.client.get_allocation_candidates("},{"line_number":1036,"context_line":"                self.context, utils.ResourceRequest())"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"    def _set_up_provider_tree(self):"},{"line_number":1039,"context_line":"        \"\"\"Create two compute nodes in placement: \"this\" one, and another one."},{"line_number":1040,"context_line":""},{"line_number":1041,"context_line":"        Must be invoked from within an _interceptor() context."}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_839768ec","line":1038,"in_reply_to":"3f79a3b5_a8e36334","updated":"2018-08-28 16:23:56.000000000","message":"Yeah. I just wrote it down on paper before reviewing the test. :)","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"        # Part of the shared storage aggregate"},{"line_number":1132,"context_line":"        self.client.put("},{"line_number":1133,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.othercn,"},{"line_number":1134,"context_line":"            {\u0027aggregates\u0027: [uuids.ssp],"},{"line_number":1135,"context_line":"             \u0027resource_provider_generation\u0027:"},{"line_number":1136,"context_line":"                 resp.json()[\u0027resource_provider_generation\u0027]},"},{"line_number":1137,"context_line":"            version\u003d\u00271.19\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_3a2479af","line":1134,"range":{"start_line":1134,"start_character":34,"end_line":1134,"end_character":37},"updated":"2018-08-28 14:23:52.000000000","message":"agg1? I guess I already mentioned this:\n\nhttps://review.openstack.org/#/c/585034/20/nova/tests/functional/test_report_client.py@1097","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"        # Part of the shared storage aggregate"},{"line_number":1132,"context_line":"        self.client.put("},{"line_number":1133,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.othercn,"},{"line_number":1134,"context_line":"            {\u0027aggregates\u0027: [uuids.ssp],"},{"line_number":1135,"context_line":"             \u0027resource_provider_generation\u0027:"},{"line_number":1136,"context_line":"                 resp.json()[\u0027resource_provider_generation\u0027]},"},{"line_number":1137,"context_line":"            version\u003d\u00271.19\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_68cf8bb3","line":1134,"range":{"start_line":1134,"start_character":34,"end_line":1134,"end_character":37},"in_reply_to":"3f79a3b5_3a2479af","updated":"2018-08-28 15:34:11.000000000","message":"Yes; will do in a fup unless I need to respin here.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"192864e7a7cd9b3c656d7d61e1ecc8933428dbf6","unresolved":false,"context_lines":[{"line_number":1131,"context_line":"        # Part of the shared storage aggregate"},{"line_number":1132,"context_line":"        self.client.put("},{"line_number":1133,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.othercn,"},{"line_number":1134,"context_line":"            {\u0027aggregates\u0027: [uuids.ssp],"},{"line_number":1135,"context_line":"             \u0027resource_provider_generation\u0027:"},{"line_number":1136,"context_line":"                 resp.json()[\u0027resource_provider_generation\u0027]},"},{"line_number":1137,"context_line":"            version\u003d\u00271.19\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_65f1b401","line":1134,"range":{"start_line":1134,"start_character":34,"end_line":1134,"end_character":37},"in_reply_to":"3f79a3b5_68cf8bb3","updated":"2018-08-30 19:22:29.000000000","message":"Done","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1307,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("},{"line_number":1308,"context_line":"                self.context, self.compute_name)"},{"line_number":1309,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1310,"context_line":"            self.assertAllocations({}, allocs)"},{"line_number":1311,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1312,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1313,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_5a34d5ac","line":1310,"range":{"start_line":1310,"start_character":12,"end_line":1310,"end_character":46},"updated":"2018-08-28 14:23:52.000000000","message":"OK this is empty because we haven\u0027t called _set_up_provider_tree_allocs yet.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3da4b50eb6fe3f66318718390c88bb754ade34f0","unresolved":false,"context_lines":[{"line_number":1307,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("},{"line_number":1308,"context_line":"                self.context, self.compute_name)"},{"line_number":1309,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1310,"context_line":"            self.assertAllocations({}, allocs)"},{"line_number":1311,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1312,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1313,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_fa59215b","line":1310,"range":{"start_line":1310,"start_character":12,"end_line":1310,"end_character":46},"in_reply_to":"3f79a3b5_5a34d5ac","updated":"2018-08-28 14:34:52.000000000","message":"correct.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"            exp_allocs \u003d self._set_up_provider_tree_allocs()"},{"line_number":1315,"context_line":"            # Save a copy of this for later"},{"line_number":1316,"context_line":"            orig_exp_allocs \u003d copy.deepcopy(exp_allocs)"},{"line_number":1317,"context_line":"            # Another null reshape: no inv changes, no alloc changes"},{"line_number":1318,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("},{"line_number":1319,"context_line":"                self.context, self.compute_uuid)"},{"line_number":1320,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_ba15e9e7","line":1317,"range":{"start_line":1317,"start_character":12,"end_line":1317,"end_character":35},"updated":"2018-08-28 14:23:52.000000000","message":"Well, because we pass in allocations, we\u0027ll still call POST /reshape right? It just doesn\u0027t do anything because we don\u0027t have any inventory or allocation changes on any of the providers in the reshape call.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3da4b50eb6fe3f66318718390c88bb754ade34f0","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"            exp_allocs \u003d self._set_up_provider_tree_allocs()"},{"line_number":1315,"context_line":"            # Save a copy of this for later"},{"line_number":1316,"context_line":"            orig_exp_allocs \u003d copy.deepcopy(exp_allocs)"},{"line_number":1317,"context_line":"            # Another null reshape: no inv changes, no alloc changes"},{"line_number":1318,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("},{"line_number":1319,"context_line":"                self.context, self.compute_uuid)"},{"line_number":1320,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_da5c654e","line":1317,"range":{"start_line":1317,"start_character":12,"end_line":1317,"end_character":35},"in_reply_to":"3f79a3b5_ba15e9e7","updated":"2018-08-28 14:34:52.000000000","message":"correct.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"            exp_allocs \u003d self._set_up_provider_tree_allocs()"},{"line_number":1315,"context_line":"            # Save a copy of this for later"},{"line_number":1316,"context_line":"            orig_exp_allocs \u003d copy.deepcopy(exp_allocs)"},{"line_number":1317,"context_line":"            # Another null reshape: no inv changes, no alloc changes"},{"line_number":1318,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("},{"line_number":1319,"context_line":"                self.context, self.compute_uuid)"},{"line_number":1320,"context_line":"            allocs \u003d self.client.get_allocations_for_provider_tree("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_08c0577c","line":1317,"range":{"start_line":1317,"start_character":12,"end_line":1317,"end_character":35},"in_reply_to":"3f79a3b5_da5c654e","updated":"2018-08-28 15:34:11.000000000","message":"Right, \"null reshape\" meaning \"a call to reshape that doesn\u0027t do anything\" as opposed to \"not a reshape call\".","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_9aa90d7e","line":1334,"updated":"2018-08-28 14:23:52.000000000","message":"Does this also add inventory to the ssp provider? Is that part of the tree if it\u0027s shared? I\u0027m guessing not since being in a shared aggregate relationship doesn\u0027t really mean the same thing as being in the same tree as the compute node root provider.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"15f0e9ee6aeb9ea48d75a4a9499d91d09756ee2e","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_9db58735","line":1334,"in_reply_to":"3f79a3b5_5a17f555","updated":"2018-08-28 14:39:20.000000000","message":"OK I guess that makes sense and confirms how ProviderTree.has_sharing_provider works.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f94b80ca98082d9bb9b264bdda1fdf410502728b","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_fad4c1bd","line":1334,"in_reply_to":"3f79a3b5_9aa90d7e","updated":"2018-08-28 14:24:37.000000000","message":"I guess I don\u0027t know how shared providers are modeled regarding a tree:\n\n(9:22:20 AM) mriedem: is a shared storage provider in an aggregate relationship with a compute node root provider considered part of the comptue node providers \"tree\"?\n(9:23:55 AM) cdent: mriedem: efried_doc will have the details on that, but the short answer is yet\n(9:23:58 AM) cdent: yes","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3da4b50eb6fe3f66318718390c88bb754ade34f0","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_5a17f555","line":1334,"in_reply_to":"3f79a3b5_9aa90d7e","updated":"2018-08-28 14:34:52.000000000","message":"Yes, actually, providers that share any resources (via an aggregate association) with the root compute node or any of its children are included in the ProviderTree object. The sharing providers are included as separate root nodes in the ProviderTree object, and we grab the inventory/traits for these sharing providers. We do *not* grab all the allocations for those sharing providers. Instead, we only grab allocations for consumers that are on this compute node. Those allocation records *may* reference a sharing provider, however we don\u0027t do a blind \"get allocations for all sharing providers associated with this compute node\" because that could very well produce 1000s of allocation records for a large storage pool.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f3ad60b75907620a46f7f70033b0e388ad62e3b","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_bd716347","line":1334,"in_reply_to":"3f79a3b5_9db58735","updated":"2018-08-28 14:50:33.000000000","message":"Also, I needed to go read into get_provider_tree_and_ensure_root and _ensure_resource_provider and _refresh_associations to see how the ProviderTree is constructed and pulls in the shared-via-aggregates providers. I didn\u0027t know that _ensure_resource_provider did all of that now - it\u0027s much more than what that method originally did was just create the provider if it didn\u0027t exist. Now it\u0027s that plus \"grab the world\".","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1331,"context_line":"            self.assertProviderTree(exp_ptree, ptree)"},{"line_number":1332,"context_line":"            self.assertAllocations(exp_allocs, allocs)"},{"line_number":1333,"context_line":"            for rp_uuid in ptree.get_provider_uuids():"},{"line_number":1334,"context_line":"                # Add a new resource class to the inventories"},{"line_number":1335,"context_line":"                ptree.update_inventory("},{"line_number":1336,"context_line":"                    rp_uuid, dict(ptree.data(rp_uuid).inventory,"},{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_a85123dc","line":1334,"in_reply_to":"3f79a3b5_bd716347","updated":"2018-08-28 15:34:11.000000000","message":"Yeah, _ensure_resource_provider was named when there was only one resource provider (the compute node). I could get behind renaming it at this point.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"},{"line_number":1338,"context_line":"                exp_ptree[rp_uuid][\u0027inventory\u0027][\u0027CUSTOM_FOO\u0027] \u003d {"},{"line_number":1339,"context_line":"                    \u0027total\u0027: 10}"},{"line_number":1340,"context_line":"            for c_uuid, alloc in allocs.items():"},{"line_number":1341,"context_line":"                for rp_uuid, res in alloc[\u0027allocations\u0027].items():"},{"line_number":1342,"context_line":"                    res[\u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1343,"context_line":"                    exp_allocs[c_uuid][\u0027allocations\u0027][rp_uuid]["},{"line_number":1344,"context_line":"                        \u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1345,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1346,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"            # Let\u0027s do a big transform that stuffs everything back onto the"},{"line_number":1349,"context_line":"            # compute node"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_9a74ad38","line":1346,"range":{"start_line":1340,"start_character":12,"end_line":1346,"end_character":69},"updated":"2018-08-28 14:23:52.000000000","message":"OK so both consumers (inst1 and inst2) have 1 CUSTOM_FOO allocation on every provider in the tree (the compute node root, numa1, and numa2).","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"},{"line_number":1338,"context_line":"                exp_ptree[rp_uuid][\u0027inventory\u0027][\u0027CUSTOM_FOO\u0027] \u003d {"},{"line_number":1339,"context_line":"                    \u0027total\u0027: 10}"},{"line_number":1340,"context_line":"            for c_uuid, alloc in allocs.items():"},{"line_number":1341,"context_line":"                for rp_uuid, res in alloc[\u0027allocations\u0027].items():"},{"line_number":1342,"context_line":"                    res[\u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1343,"context_line":"                    exp_allocs[c_uuid][\u0027allocations\u0027][rp_uuid]["},{"line_number":1344,"context_line":"                        \u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1345,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1346,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"            # Let\u0027s do a big transform that stuffs everything back onto the"},{"line_number":1349,"context_line":"            # compute node"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_c38c80d9","line":1346,"range":{"start_line":1340,"start_character":12,"end_line":1346,"end_character":69},"in_reply_to":"3f79a3b5_88a307b2","updated":"2018-08-28 16:23:56.000000000","message":"\u003e So they both have 1 CUSTOM_FOO against the compute and the SSP; \n\nYup this part I missed because of the above misunderstanding of the shared provider being a separate root in the ProviderTree object.\n\n\u003e inst1 has 1 CUSTOM_FOO against numa1; and inst2 has 1 CUSTOM_FOO against numa2.\n\nYup, my mistake. I should have known that since I drew the tree on paper before reading this.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1337,"context_line":"                                  CUSTOM_FOO\u003d{\u0027total\u0027: 10}))"},{"line_number":1338,"context_line":"                exp_ptree[rp_uuid][\u0027inventory\u0027][\u0027CUSTOM_FOO\u0027] \u003d {"},{"line_number":1339,"context_line":"                    \u0027total\u0027: 10}"},{"line_number":1340,"context_line":"            for c_uuid, alloc in allocs.items():"},{"line_number":1341,"context_line":"                for rp_uuid, res in alloc[\u0027allocations\u0027].items():"},{"line_number":1342,"context_line":"                    res[\u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1343,"context_line":"                    exp_allocs[c_uuid][\u0027allocations\u0027][rp_uuid]["},{"line_number":1344,"context_line":"                        \u0027resources\u0027][\u0027CUSTOM_FOO\u0027] \u003d 1"},{"line_number":1345,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1346,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1347,"context_line":""},{"line_number":1348,"context_line":"            # Let\u0027s do a big transform that stuffs everything back onto the"},{"line_number":1349,"context_line":"            # compute node"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_88a307b2","line":1346,"range":{"start_line":1340,"start_character":12,"end_line":1346,"end_character":69},"in_reply_to":"3f79a3b5_9a74ad38","updated":"2018-08-28 15:34:11.000000000","message":"They have 1 CUSTOM_FOO on every provider that had any allocations before. So they both have 1 CUSTOM_FOO against the compute and the SSP; inst1 has 1 CUSTOM_FOO against numa1; and inst2 has 1 CUSTOM_FOO against numa2.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"                    cum_inv[rc][\u0027total\u0027] +\u003d inv[\u0027total\u0027]"},{"line_number":1362,"context_line":"                # Remove all the providers except the compute node and the"},{"line_number":1363,"context_line":"                # shared storage provider, which still has (and shall"},{"line_number":1364,"context_line":"                # retain (allocations from the \"other\" compute node."},{"line_number":1365,"context_line":"                # TODO(efried): But is that right? I should be able to"},{"line_number":1366,"context_line":"                # remove the SSP from *this* tree and have it continue to"},{"line_number":1367,"context_line":"                # exist in the world. But how would ufpt distinguish?"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_5a037577","line":1364,"range":{"start_line":1364,"start_character":38,"end_line":1364,"end_character":68},"updated":"2018-08-28 14:23:52.000000000","message":"Because of the bug noted above on L1134, this isn\u0027t really true, right? Because the ssp provider is in the agg1 aggregate, but the othercn is in an ssp aggregate, which is something else that doesn\u0027t actually include the ssp provider.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"                    cum_inv[rc][\u0027total\u0027] +\u003d inv[\u0027total\u0027]"},{"line_number":1362,"context_line":"                # Remove all the providers except the compute node and the"},{"line_number":1363,"context_line":"                # shared storage provider, which still has (and shall"},{"line_number":1364,"context_line":"                # retain (allocations from the \"other\" compute node."},{"line_number":1365,"context_line":"                # TODO(efried): But is that right? I should be able to"},{"line_number":1366,"context_line":"                # remove the SSP from *this* tree and have it continue to"},{"line_number":1367,"context_line":"                # exist in the world. But how would ufpt distinguish?"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_5ada951d","line":1364,"range":{"start_line":1364,"start_character":24,"end_line":1364,"end_character":26},"updated":"2018-08-28 14:23:52.000000000","message":"s/ (/) / ?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"                    cum_inv[rc][\u0027total\u0027] +\u003d inv[\u0027total\u0027]"},{"line_number":1362,"context_line":"                # Remove all the providers except the compute node and the"},{"line_number":1363,"context_line":"                # shared storage provider, which still has (and shall"},{"line_number":1364,"context_line":"                # retain (allocations from the \"other\" compute node."},{"line_number":1365,"context_line":"                # TODO(efried): But is that right? I should be able to"},{"line_number":1366,"context_line":"                # remove the SSP from *this* tree and have it continue to"},{"line_number":1367,"context_line":"                # exist in the world. But how would ufpt distinguish?"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_e89f1b4f","line":1364,"range":{"start_line":1364,"start_character":24,"end_line":1364,"end_character":26},"in_reply_to":"3f79a3b5_5ada951d","updated":"2018-08-28 15:34:11.000000000","message":"Right; when I fixed L1134 the first time it still worked as expected. I\u0027ll fix the paren in the fup, but I believe the note is still correct.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"192864e7a7cd9b3c656d7d61e1ecc8933428dbf6","unresolved":false,"context_lines":[{"line_number":1361,"context_line":"                    cum_inv[rc][\u0027total\u0027] +\u003d inv[\u0027total\u0027]"},{"line_number":1362,"context_line":"                # Remove all the providers except the compute node and the"},{"line_number":1363,"context_line":"                # shared storage provider, which still has (and shall"},{"line_number":1364,"context_line":"                # retain (allocations from the \"other\" compute node."},{"line_number":1365,"context_line":"                # TODO(efried): But is that right? I should be able to"},{"line_number":1366,"context_line":"                # remove the SSP from *this* tree and have it continue to"},{"line_number":1367,"context_line":"                # exist in the world. But how would ufpt distinguish?"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_05e00026","line":1364,"range":{"start_line":1364,"start_character":24,"end_line":1364,"end_character":26},"in_reply_to":"3f79a3b5_e89f1b4f","updated":"2018-08-30 19:22:29.000000000","message":"Done","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1386,"context_line":""},{"line_number":1387,"context_line":"            # Let\u0027s inject an error path test here: attempting to reshape"},{"line_number":1388,"context_line":"            # inventories without having moved their allocations should fail."},{"line_number":1389,"context_line":"            self.assertRaises("},{"line_number":1390,"context_line":"                exception.ReshapeFailed,"},{"line_number":1391,"context_line":"                self.client.update_from_provider_tree, self.context, ptree,"},{"line_number":1392,"context_line":"                allocations\u003dallocs)"},{"line_number":1393,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_9a458db7","line":1390,"range":{"start_line":1389,"start_character":12,"end_line":1390,"end_character":40},"updated":"2018-08-28 14:23:52.000000000","message":"Can you assert the actual error off the message? e.g.:\n\nex \u003d self.assertRaises(exception.ReshapeFailed, ...)\nself.assertIn(\u0027some error i expect in this case\u0027, six.text_type(ex))","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0345f132e547f23654966ff853040d154ee1cdb7","unresolved":false,"context_lines":[{"line_number":1386,"context_line":""},{"line_number":1387,"context_line":"            # Let\u0027s inject an error path test here: attempting to reshape"},{"line_number":1388,"context_line":"            # inventories without having moved their allocations should fail."},{"line_number":1389,"context_line":"            self.assertRaises("},{"line_number":1390,"context_line":"                exception.ReshapeFailed,"},{"line_number":1391,"context_line":"                self.client.update_from_provider_tree, self.context, ptree,"},{"line_number":1392,"context_line":"                allocations\u003dallocs)"},{"line_number":1393,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_6e4a335c","line":1390,"range":{"start_line":1389,"start_character":12,"end_line":1390,"end_character":40},"in_reply_to":"3f79a3b5_03acb833","updated":"2018-08-28 16:33:48.000000000","message":"Yes","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1386,"context_line":""},{"line_number":1387,"context_line":"            # Let\u0027s inject an error path test here: attempting to reshape"},{"line_number":1388,"context_line":"            # inventories without having moved their allocations should fail."},{"line_number":1389,"context_line":"            self.assertRaises("},{"line_number":1390,"context_line":"                exception.ReshapeFailed,"},{"line_number":1391,"context_line":"                self.client.update_from_provider_tree, self.context, ptree,"},{"line_number":1392,"context_line":"                allocations\u003dallocs)"},{"line_number":1393,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_03acb833","line":1390,"range":{"start_line":1389,"start_character":12,"end_line":1390,"end_character":40},"in_reply_to":"3f79a3b5_089db756","updated":"2018-08-28 16:23:56.000000000","message":"Follow up or...?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"192864e7a7cd9b3c656d7d61e1ecc8933428dbf6","unresolved":false,"context_lines":[{"line_number":1386,"context_line":""},{"line_number":1387,"context_line":"            # Let\u0027s inject an error path test here: attempting to reshape"},{"line_number":1388,"context_line":"            # inventories without having moved their allocations should fail."},{"line_number":1389,"context_line":"            self.assertRaises("},{"line_number":1390,"context_line":"                exception.ReshapeFailed,"},{"line_number":1391,"context_line":"                self.client.update_from_provider_tree, self.context, ptree,"},{"line_number":1392,"context_line":"                allocations\u003dallocs)"},{"line_number":1393,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_e5c70464","line":1390,"range":{"start_line":1389,"start_character":12,"end_line":1390,"end_character":40},"in_reply_to":"3f79a3b5_6e4a335c","updated":"2018-08-30 19:22:29.000000000","message":"Done","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1386,"context_line":""},{"line_number":1387,"context_line":"            # Let\u0027s inject an error path test here: attempting to reshape"},{"line_number":1388,"context_line":"            # inventories without having moved their allocations should fail."},{"line_number":1389,"context_line":"            self.assertRaises("},{"line_number":1390,"context_line":"                exception.ReshapeFailed,"},{"line_number":1391,"context_line":"                self.client.update_from_provider_tree, self.context, ptree,"},{"line_number":1392,"context_line":"                allocations\u003dallocs)"},{"line_number":1393,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_089db756","line":1390,"range":{"start_line":1389,"start_character":12,"end_line":1390,"end_character":40},"in_reply_to":"3f79a3b5_9a458db7","updated":"2018-08-28 15:34:11.000000000","message":"okay.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d28de58c7ddab0174b7e54127e908afae53633e","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                alloc[\u0027allocations\u0027] \u003d {"},{"line_number":1404,"context_line":"                    self.compute_uuid: {\u0027resources\u0027: cum_allocs}}"},{"line_number":1405,"context_line":"            exp_allocs \u003d copy.deepcopy(allocs)"},{"line_number":1406,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1407,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"            # Okay, let\u0027s transform back now"},{"line_number":1410,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_fa2121fe","line":1407,"range":{"start_line":1406,"start_character":12,"end_line":1407,"end_character":69},"updated":"2018-08-28 14:23:52.000000000","message":"Where do assert that the child providers don\u0027t have allocations now? I guess L1414 and L1415 below?","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0345f132e547f23654966ff853040d154ee1cdb7","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                alloc[\u0027allocations\u0027] \u003d {"},{"line_number":1404,"context_line":"                    self.compute_uuid: {\u0027resources\u0027: cum_allocs}}"},{"line_number":1405,"context_line":"            exp_allocs \u003d copy.deepcopy(allocs)"},{"line_number":1406,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1407,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"            # Okay, let\u0027s transform back now"},{"line_number":1410,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_4e72d7af","line":1407,"range":{"start_line":1406,"start_character":12,"end_line":1407,"end_character":69},"in_reply_to":"3f79a3b5_037118df","updated":"2018-08-28 16:33:48.000000000","message":"Okay. I was counting on \"Let\u0027s do a big transform that stuffs everything back onto the compute node\" and \"Move all the allocations off their existing providers and onto the compute node\". I\u0027ll see what else I can add.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"192864e7a7cd9b3c656d7d61e1ecc8933428dbf6","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                alloc[\u0027allocations\u0027] \u003d {"},{"line_number":1404,"context_line":"                    self.compute_uuid: {\u0027resources\u0027: cum_allocs}}"},{"line_number":1405,"context_line":"            exp_allocs \u003d copy.deepcopy(allocs)"},{"line_number":1406,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1407,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"            # Okay, let\u0027s transform back now"},{"line_number":1410,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_452118fc","line":1407,"range":{"start_line":1406,"start_character":12,"end_line":1407,"end_character":69},"in_reply_to":"3f79a3b5_4e72d7af","updated":"2018-08-30 19:22:29.000000000","message":"Done","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d135b10459318fec5d4c02524b4488ae3d0e779b","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                alloc[\u0027allocations\u0027] \u003d {"},{"line_number":1404,"context_line":"                    self.compute_uuid: {\u0027resources\u0027: cum_allocs}}"},{"line_number":1405,"context_line":"            exp_allocs \u003d copy.deepcopy(allocs)"},{"line_number":1406,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1407,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"            # Okay, let\u0027s transform back now"},{"line_number":1410,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_037118df","line":1407,"range":{"start_line":1406,"start_character":12,"end_line":1407,"end_character":69},"in_reply_to":"3f79a3b5_68030b78","updated":"2018-08-28 16:23:56.000000000","message":"OK, I definitely started to get a bit lost around L1356. More inline comments could help.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a4377055943a97abc4a42235475f5c0b138dcc96","unresolved":false,"context_lines":[{"line_number":1403,"context_line":"                alloc[\u0027allocations\u0027] \u003d {"},{"line_number":1404,"context_line":"                    self.compute_uuid: {\u0027resources\u0027: cum_allocs}}"},{"line_number":1405,"context_line":"            exp_allocs \u003d copy.deepcopy(allocs)"},{"line_number":1406,"context_line":"            self.client.update_from_provider_tree(self.context, ptree,"},{"line_number":1407,"context_line":"                                                  allocations\u003dallocs)"},{"line_number":1408,"context_line":""},{"line_number":1409,"context_line":"            # Okay, let\u0027s transform back now"},{"line_number":1410,"context_line":"            ptree \u003d self.client.get_provider_tree_and_ensure_root("}],"source_content_type":"text/x-python","patch_set":19,"id":"3f79a3b5_68030b78","line":1407,"range":{"start_line":1406,"start_character":12,"end_line":1407,"end_character":69},"in_reply_to":"3f79a3b5_fa2121fe","updated":"2018-08-28 15:34:11.000000000","message":"There are no child providers at this point. They were removed via L1369.","commit_id":"b23bf6d6ab60abde40c58cba25fc2694d3a0277e"}]}