)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eric Fried \u003cefried@us.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-06 21:54:03 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP: Tighten up ReportClient use of generation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update aggregate-related scheduler report client methods to use"},{"line_number":10,"context_line":"placement microversion 1.19, which returns provider generation in GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"5f7c97a3_bed966f1","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":46},"updated":"2018-07-10 19:01:11.000000000","message":"please change to specifically state this patch addresses the *resource provider generation* when updating aggregate membership. This commit message summary makes it sound like you\u0027re going over all of the reportclient looking for any place that might be \"loose\" with regards to generation-checking.\n\nHow about this instead?\n\n placement: always ensure provider generation checked","commit_id":"5ad90213f9a661eddc1231bfb7c3c3f4bd1a69b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eric Fried \u003cefried@us.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-10 13:31:51 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Tighten up ReportClient use of generation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update aggregate-related scheduler report client methods to use"},{"line_number":10,"context_line":"placement microversion 1.19, which returns provider generation in GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"5f7c97a3_97980d76","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":41},"updated":"2018-07-10 19:01:11.000000000","message":"please change to specifically state this patch addresses the *resource provider generation* when updating aggregate membership. This commit message summary makes it sound like you\u0027re going over all of the reportclient looking for any place that might be \"loose\" with regards to generation-checking.\n\nHow about this instead?\n\n placement: always ensure provider generation checked","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eric Fried \u003cefried@us.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-07-10 13:31:51 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Tighten up ReportClient use of generation"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Update aggregate-related scheduler report client methods to use"},{"line_number":10,"context_line":"placement microversion 1.19, which returns provider generation in GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"5f7c97a3_927a5bb8","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":41},"in_reply_to":"5f7c97a3_97980d76","updated":"2018-07-10 20:57:33.000000000","message":"I don\u0027t love the \u0027placement\u0027 prefix, since this change is all on the nova side, but I reworded this.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Update aggregate-related scheduler report client methods to use"},{"line_number":10,"context_line":"placement microversion 1.19, which returns provider generation in GET"},{"line_number":11,"context_line":"/rps/{u}/aggregates and handles generation conflicts in PUT"},{"line_number":12,"context_line":"/rps/{u}/aggregates. Helper methods previously returning aggregates and"},{"line_number":13,"context_line":"traits now also return the generation, which is fed through"},{"line_number":14,"context_line":"appropriately to subsequent calls. As a result, the generation kwarg is"},{"line_number":15,"context_line":"no longer needed in _refresh_associations, so it is removed."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"5f7c97a3_3087e8b5","line":13,"range":{"start_line":12,"start_character":68,"end_line":13,"end_character":6},"updated":"2018-07-19 19:43:05.000000000","message":"I assume this is for consistency, but as noted inline, it\u0027s inconsistent with how get_allocations_for_consumer (depending on kwargs) will return the full json response body dict.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_a14e2736","line":859,"updated":"2018-07-10 19:01:11.000000000","message":"this seems like an overly broad retry decorator location. I would have thought we\u0027d want to surgically retry only certain operations from within the reportclient itself -- for instance, on the set_aggregates_for_provider() method?","commit_id":"5ad90213f9a661eddc1231bfb7c3c3f4bd1a69b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_177e3d1a","line":859,"updated":"2018-07-10 19:01:11.000000000","message":"this seems like an overly broad retry decorator location. I would have thought we\u0027d want to surgically retry only certain operations from within the reportclient itself -- for instance, inside the SchedulerReportClient.update_from_provider_tree() method maybe retrying the specific PUT /resource_providers/{uuid}/inventories call that occurs there?","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_0d1cea63","line":859,"in_reply_to":"5f7c97a3_177e3d1a","updated":"2018-07-10 20:57:33.000000000","message":"We can\u0027t go that narrow because the retry needs to include re-GETting the provider tree info from placement (get_provider_tree_and_ensure_root) and letting the virt driver swizzle it again (update_provider_tree).\n\nAt narrowest, the retry could encompass L884-898. I considered that originally, but I would have to pull that chunk out into yet another helper method (and figure out how to unwind the nested exceptions accordingly, and it didn\u0027t seem worth that. But can do if you feel strongly that that\u0027s better.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"cf20fa6cd05684f63f27ac485c884cca4fbcb6e0","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_4378657d","line":859,"updated":"2018-07-11 11:17:57.000000000","message":"After reading the discussion on the earlier patches, I think you\u0027re okay, for now, to not extract the retry handler to something granular, but only if _resource_change remains as simple as it is and that this overall method does not otherwise change.\n\nSuch change seems moderately likely and what we don\u0027t want to see is that we inadvertently repeat unnecessary writes (to wherever).\n\nAnd besides: I\u0027m always a fan of extracting a method.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"322028575a5495ccce156f2410e5de376f4590c8","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d98a7cbb","line":859,"in_reply_to":"5f7c97a3_29cae76e","updated":"2018-07-18 21:27:13.000000000","message":"I think the discussion on earlier patches wasn\u0027t about extracting a method, but rather on *applying* the retry decorator on a *smaller* code surface area. In other words, decorating reportclient.update_from_provider_tree() instead of this entire RT method.\n\nThat said, I\u0027m not going to hold this up for that. I don\u0027t think it will hurt anything to have the retry on the RT._update() method...","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"539cbcec9e83ce58a7324a63203cc60bc22e7928","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_29cae76e","line":859,"in_reply_to":"5f7c97a3_4378657d","updated":"2018-07-11 13:22:44.000000000","message":"\u003e And besides: I\u0027m always a fan of extracting a method.\n\nWhat tipped the balance for me was the number of shim layers already in play here.\n\ncompute manager\u0027s update_available_resource is just a loop around\n\n_update_available_resource_for_node, which is just an exception handler wrapped around\n\nresource tracker\u0027s update_available_resource, which adds very little to\n\n_update_available_resource, which adds some actual substance before calling\n\nthis _update method.\n\nThat\u0027s probably not a good reason not to extract.  What do others think?","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"126ec569bd44037e208f12933f457ec91c600b6a","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            return True"},{"line_number":857,"context_line":"        return False"},{"line_number":858,"context_line":""},{"line_number":859,"context_line":"    @utils.retry_on_exception(exception.ResourceProviderUpdateConflict)"},{"line_number":860,"context_line":"    def _update(self, context, compute_node):"},{"line_number":861,"context_line":"        \"\"\"Update partial stats locally and populate them to Scheduler.\"\"\""},{"line_number":862,"context_line":"        if self._resource_change(compute_node):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_74f00ffd","line":859,"in_reply_to":"5f7c97a3_d98a7cbb","updated":"2018-07-18 21:40:09.000000000","message":"Well, we definitely can\u0027t do it *just* on update_from_provider_tree, because the retry needs to re-GET the things we\u0027re passing into that guy as parameters. That\u0027s why we\u0027re talking about extracting - in order to scope it perfectly, we would have to peel out the stuff on L884-898 (which is harder than it sounds because we\u0027re wound up in NotImplementedError to fall back to get_inventory, etc.).\n\nIf you\u0027re good with it being on _update for now, great.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":864,"context_line":"            # NOTE(jianghuaw): Once we completely move to use get_inventory()"},{"line_number":865,"context_line":"            # for all resource provider\u0027s inv data. We can remove this check."},{"line_number":866,"context_line":"            # At the moment we still need this check and save compute_node."},{"line_number":867,"context_line":"            compute_node.save()"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        # NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the"},{"line_number":870,"context_line":"        # object of compute_node; instead the inventory data for these"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_10d42ce9","line":867,"range":{"start_line":867,"start_character":12,"end_line":867,"end_character":31},"updated":"2018-07-19 19:43:05.000000000","message":"So if we retry I guess this could be redundant since we already saved once, and this will always save because it updates the updated_at field.\n\nhttps://github.com/openstack/nova/blob/50f40854b04351fb622fd8b68b374a8fe8ca2070/nova/db/sqlalchemy/api.py#L705\n\nBut that\u0027s probably not a huge problem. If it became a problem, we could move the UPT stuff into an inner method and put the retry decorator on that...","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":864,"context_line":"            # NOTE(jianghuaw): Once we completely move to use get_inventory()"},{"line_number":865,"context_line":"            # for all resource provider\u0027s inv data. We can remove this check."},{"line_number":866,"context_line":"            # At the moment we still need this check and save compute_node."},{"line_number":867,"context_line":"            compute_node.save()"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        # NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the"},{"line_number":870,"context_line":"        # object of compute_node; instead the inventory data for these"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_84e47d0f","line":867,"range":{"start_line":867,"start_character":12,"end_line":867,"end_character":31},"in_reply_to":"5f7c97a3_10d42ce9","updated":"2018-07-19 20:55:22.000000000","message":"Right, see discussion above.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e10a80d4c2ad9ff08906b2f683a0517d2d1d7ff","unresolved":false,"context_lines":[{"line_number":864,"context_line":"            # NOTE(jianghuaw): Once we completely move to use get_inventory()"},{"line_number":865,"context_line":"            # for all resource provider\u0027s inv data. We can remove this check."},{"line_number":866,"context_line":"            # At the moment we still need this check and save compute_node."},{"line_number":867,"context_line":"            compute_node.save()"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        # NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the"},{"line_number":870,"context_line":"        # object of compute_node; instead the inventory data for these"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_fdb944eb","line":867,"range":{"start_line":867,"start_character":12,"end_line":867,"end_character":31},"in_reply_to":"5f7c97a3_84e47d0f","updated":"2018-07-19 21:44:39.000000000","message":"Yup, and I did, I\u0027m just saying, an easy compromise is putting the update_provider_tree and friends into an inner method in here and putting the retry decorator on that, but not a huge deal.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3395e6bfea51d8fa22e3532a957522914bb228f2","unresolved":false,"context_lines":[{"line_number":864,"context_line":"            # NOTE(jianghuaw): Once we completely move to use get_inventory()"},{"line_number":865,"context_line":"            # for all resource provider\u0027s inv data. We can remove this check."},{"line_number":866,"context_line":"            # At the moment we still need this check and save compute_node."},{"line_number":867,"context_line":"            compute_node.save()"},{"line_number":868,"context_line":""},{"line_number":869,"context_line":"        # NOTE(jianghuaw): Some resources(e.g. VGPU) are not saved in the"},{"line_number":870,"context_line":"        # object of compute_node; instead the inventory data for these"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_98905e98","line":867,"range":{"start_line":867,"start_character":12,"end_line":867,"end_character":31},"in_reply_to":"5f7c97a3_fdb944eb","updated":"2018-07-19 22:19:41.000000000","message":"It\u0027s not quiite that easy, cause we have to figure out how to unwind the exception paths, but yeah.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":915,"context_line":"                                                          compute_node)"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":"        if self.pci_tracker:"},{"line_number":918,"context_line":"            self.pci_tracker.save(context)"},{"line_number":919,"context_line":""},{"line_number":920,"context_line":"    def _update_usage(self, usage, nodename, sign\u003d1):"},{"line_number":921,"context_line":"        mem_usage \u003d usage[\u0027memory_mb\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_702000f9","line":918,"range":{"start_line":918,"start_character":12,"end_line":918,"end_character":42},"updated":"2018-07-19 19:43:05.000000000","message":"This is idempotent because PciDevice.save() will call _from_db_object() which calls obj_reset_changes().","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":915,"context_line":"                                                          compute_node)"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":"        if self.pci_tracker:"},{"line_number":918,"context_line":"            self.pci_tracker.save(context)"},{"line_number":919,"context_line":""},{"line_number":920,"context_line":"    def _update_usage(self, usage, nodename, sign\u003d1):"},{"line_number":921,"context_line":"        mem_usage \u003d usage[\u0027memory_mb\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_64e5a106","line":918,"range":{"start_line":918,"start_character":12,"end_line":918,"end_character":42},"in_reply_to":"5f7c97a3_702000f9","updated":"2018-07-19 20:55:22.000000000","message":"But we\u0027ll only get here once even if we retry, because the retry is triggered by having raised ResourceProviderUpdateConflict from L898.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2f4964fbc250496604a2678c27414d8d8b4bfd2f","unresolved":false,"context_lines":[{"line_number":20,"context_line":"\"\"\""},{"line_number":21,"context_line":"import collections"},{"line_number":22,"context_line":"import copy"},{"line_number":23,"context_line":"import retrying"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from oslo_log import log as logging"},{"line_number":26,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_b3be86ac","line":23,"updated":"2018-07-20 18:22:50.000000000","message":"nit: 3rd party so should go with the oslo imports","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"27ad73ccb0a0e4a1d1592a836c5d4655e59ba957","unresolved":false,"context_lines":[{"line_number":20,"context_line":"\"\"\""},{"line_number":21,"context_line":"import collections"},{"line_number":22,"context_line":"import copy"},{"line_number":23,"context_line":"import retrying"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from oslo_log import log as logging"},{"line_number":26,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f79a3b5_a99882bf","line":23,"in_reply_to":"5f7c97a3_b3be86ac","updated":"2018-09-06 16:12:51.000000000","message":"Done","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"        if generation is None and not use_cache:"},{"line_number":1277,"context_line":"            raise ValueError(_(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        # Allow specified generation to override what\u0027s in the cache"},{"line_number":1280,"context_line":"        if use_cache and generation is None:"},{"line_number":1281,"context_line":"            generation \u003d self._provider_tree.data(rp_uuid).generation"},{"line_number":1282,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_3381d14e","line":1279,"range":{"start_line":1279,"start_character":8,"end_line":1279,"end_character":68},"updated":"2018-06-18 13:23:03.000000000","message":"This isn\u0027t what the next two lines do, which makes this code comment a bit odd. It might be more logical to do this and the above validation in a single conditional block like so?\n\n if generation is None:\n     if not use_cache:\n         # Either cache or generation (override) is required\n         raise ValueError(...)\n\n     else:\n         # Grab the generation from the cached provider\n         generation \u003d self._provider_tree.data(rp_uuid).generation","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0a025d88a928677b35dc1030dc06ed25efb049e1","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"        if generation is None and not use_cache:"},{"line_number":1277,"context_line":"            raise ValueError(_(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        # Allow specified generation to override what\u0027s in the cache"},{"line_number":1280,"context_line":"        if use_cache and generation is None:"},{"line_number":1281,"context_line":"            generation \u003d self._provider_tree.data(rp_uuid).generation"},{"line_number":1282,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_9a0a3225","line":1279,"range":{"start_line":1279,"start_character":8,"end_line":1279,"end_character":68},"in_reply_to":"5f7c97a3_3381d14e","updated":"2018-06-18 16:31:36.000000000","message":"I could refactor the conditional if you think it\u0027s clearer that way.\n\nAgree the comment is a tad odd, but it does relate: if use_cache and generation is not None, we\u0027ll use the specified generation (i.e. it overrides the one from the cache).","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8693be9390681cd710610ff47dc6701f875022af","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"        if generation is None and not use_cache:"},{"line_number":1277,"context_line":"            raise ValueError(_(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        # Allow specified generation to override what\u0027s in the cache"},{"line_number":1280,"context_line":"        if use_cache and generation is None:"},{"line_number":1281,"context_line":"            generation \u003d self._provider_tree.data(rp_uuid).generation"},{"line_number":1282,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_351691a9","line":1279,"range":{"start_line":1279,"start_character":8,"end_line":1279,"end_character":68},"in_reply_to":"5f7c97a3_9a0a3225","updated":"2018-06-18 17:02:57.000000000","message":"Done.  But I also left the comment.  But I added a bit to connect the missing link in the thought process.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"                        global_request_id\u003dcontext.global_id)"},{"line_number":1289,"context_line":""},{"line_number":1290,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":1291,"context_line":"            json \u003d resp.json()"},{"line_number":1292,"context_line":"            if use_cache:"},{"line_number":1293,"context_line":"                self._provider_tree.update_aggregates("},{"line_number":1294,"context_line":"                    rp_uuid, json[\u0027aggregates\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_33cf91b4","line":1291,"range":{"start_line":1291,"start_character":12,"end_line":1291,"end_character":30},"updated":"2018-06-18 13:23:03.000000000","message":"not necessary (see below)","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":1291,"context_line":"            json \u003d resp.json()"},{"line_number":1292,"context_line":"            if use_cache:"},{"line_number":1293,"context_line":"                self._provider_tree.update_aggregates("},{"line_number":1294,"context_line":"                    rp_uuid, json[\u0027aggregates\u0027],"},{"line_number":1295,"context_line":"                    generation\u003djson[\u0027resource_provider_generation\u0027])"},{"line_number":1296,"context_line":"            return"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b347211f","line":1295,"range":{"start_line":1294,"start_character":0,"end_line":1295,"end_character":68},"updated":"2018-06-18 13:23:03.000000000","message":"we know that if we got a 200 OK, then we didn\u0027t get a generation conflict and generation + 1 is what the resource provider\u0027s generation will be. we also know that we overwrote the provider\u0027s collection of aggregates with exactly what we supplied in the aggregates param to this function.\n\nSo, we don\u0027t need to unpack the JSON response here and can just do:\n\n self._provider_tree.update_aggregates(\n     rp_uuid, aggregates, generation\u003dgeneration + 1)\n\nOr were you being super-paranoid about the generation being \"opaque\"?","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6076e8183e40494e510f5fa00ef20f8a3674e87e","unresolved":false,"context_lines":[{"line_number":1291,"context_line":"            json \u003d resp.json()"},{"line_number":1292,"context_line":"            if use_cache:"},{"line_number":1293,"context_line":"                self._provider_tree.update_aggregates("},{"line_number":1294,"context_line":"                    rp_uuid, json[\u0027aggregates\u0027],"},{"line_number":1295,"context_line":"                    generation\u003djson[\u0027resource_provider_generation\u0027])"},{"line_number":1296,"context_line":"            return"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f542590f","line":1295,"range":{"start_line":1294,"start_character":0,"end_line":1295,"end_character":68},"in_reply_to":"5f7c97a3_5560edcf","updated":"2018-06-18 16:39:18.000000000","message":"ack, that\u0027s fine, thanks for the answer. BTW, \"paranoid\" wasn\u0027t meant as a dis.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0a025d88a928677b35dc1030dc06ed25efb049e1","unresolved":false,"context_lines":[{"line_number":1291,"context_line":"            json \u003d resp.json()"},{"line_number":1292,"context_line":"            if use_cache:"},{"line_number":1293,"context_line":"                self._provider_tree.update_aggregates("},{"line_number":1294,"context_line":"                    rp_uuid, json[\u0027aggregates\u0027],"},{"line_number":1295,"context_line":"                    generation\u003djson[\u0027resource_provider_generation\u0027])"},{"line_number":1296,"context_line":"            return"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5560edcf","line":1295,"range":{"start_line":1294,"start_character":0,"end_line":1295,"end_character":68},"in_reply_to":"5f7c97a3_b347211f","updated":"2018-06-18 16:31:36.000000000","message":"I wouldn\u0027t call it paranoid; it\u0027s what we made such a big deal about in Dublin and was the reason behind several blueprints, including returning the payload from GET /rps and doing inventory deletions with PUT/POST instead of DELETE.  Those allowed us to get rid of code that did stuff like generation + 1.\n\nSo yeah, I could get rid of the `json` variable and use the aggregates from above, but I would still use generation\u003dresp.json()[\u0027resource_provider_generation\u0027].\n\nHowever, I like keeping the thing where we get the aggregates from the response payload instead of the input.  It just feels... righter.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8693be9390681cd710610ff47dc6701f875022af","unresolved":false,"context_lines":[{"line_number":1291,"context_line":"            json \u003d resp.json()"},{"line_number":1292,"context_line":"            if use_cache:"},{"line_number":1293,"context_line":"                self._provider_tree.update_aggregates("},{"line_number":1294,"context_line":"                    rp_uuid, json[\u0027aggregates\u0027],"},{"line_number":1295,"context_line":"                    generation\u003djson[\u0027resource_provider_generation\u0027])"},{"line_number":1296,"context_line":"            return"},{"line_number":1297,"context_line":""},{"line_number":1298,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_751ba9c9","line":1295,"range":{"start_line":1294,"start_character":0,"end_line":1295,"end_character":68},"in_reply_to":"5f7c97a3_f542590f","updated":"2018-06-18 17:02:57.000000000","message":"I didn\u0027t take it that way at all.  Paranoid is good for coding IMO.  But in this case we\u0027re just being well-behaved little API consumers by not exploiting our knowledge of generation internals.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a308ee108af5e98f474ea0780c551b8b0e524cc6","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"        LOG.error(msg, args)"},{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"        # If a conflict, raise special conflict exception"},{"line_number":1312,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1313,"context_line":"            raise exception.ResourceProviderUpdateConflict("},{"line_number":1314,"context_line":"                uuid\u003drp_uuid, generation\u003dgeneration, error\u003dresp.text)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        # Otherwise, raise generic exception"},{"line_number":1317,"context_line":"        raise exception.ResourceProviderUpdateFailed(url\u003durl, error\u003dresp.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_8928b297","line":1314,"range":{"start_line":1312,"start_character":8,"end_line":1314,"end_character":69},"updated":"2018-06-18 14:38:15.000000000","message":"When I was thinking about implementing the TODO, I figured I\u0027d used the Retry exception and decorator to try to automatically retry the allocation request since these are already idempotent methods - if adding an aggregate and it\u0027s already there, we don\u0027t do anything, same with removing an rp from an aggregate, if it\u0027s already gone we\u0027re done.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6c4c5ab3499b809847b45d0b9958e247bb7d4cb3","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"        LOG.error(msg, args)"},{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"        # If a conflict, raise special conflict exception"},{"line_number":1312,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1313,"context_line":"            raise exception.ResourceProviderUpdateConflict("},{"line_number":1314,"context_line":"                uuid\u003drp_uuid, generation\u003dgeneration, error\u003dresp.text)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        # Otherwise, raise generic exception"},{"line_number":1317,"context_line":"        raise exception.ResourceProviderUpdateFailed(url\u003durl, error\u003dresp.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_55bc6d05","line":1314,"range":{"start_line":1312,"start_character":8,"end_line":1314,"end_character":69},"in_reply_to":"5f7c97a3_5a7d7ab0","updated":"2018-06-18 16:34:11.000000000","message":"I\u0027m thinking about the case on L1981 below, and in aggregate_remove_host, but I\u0027m OK with handling retries automatically in those methods separately.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6076e8183e40494e510f5fa00ef20f8a3674e87e","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"        LOG.error(msg, args)"},{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"        # If a conflict, raise special conflict exception"},{"line_number":1312,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1313,"context_line":"            raise exception.ResourceProviderUpdateConflict("},{"line_number":1314,"context_line":"                uuid\u003drp_uuid, generation\u003dgeneration, error\u003dresp.text)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        # Otherwise, raise generic exception"},{"line_number":1317,"context_line":"        raise exception.ResourceProviderUpdateFailed(url\u003durl, error\u003dresp.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_b51b811a","line":1314,"range":{"start_line":1312,"start_character":8,"end_line":1314,"end_character":69},"in_reply_to":"5f7c97a3_5a7d7ab0","updated":"2018-06-18 16:39:18.000000000","message":"Yeah, agreed that picking through the methods and identifying ones that can/should be retried would be a separate patch. But, definitely we should invalidate the cached provider in the tree if we get a 409. +1 on that.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0a025d88a928677b35dc1030dc06ed25efb049e1","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"        LOG.error(msg, args)"},{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"        # If a conflict, raise special conflict exception"},{"line_number":1312,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1313,"context_line":"            raise exception.ResourceProviderUpdateConflict("},{"line_number":1314,"context_line":"                uuid\u003drp_uuid, generation\u003dgeneration, error\u003dresp.text)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        # Otherwise, raise generic exception"},{"line_number":1317,"context_line":"        raise exception.ResourceProviderUpdateFailed(url\u003durl, error\u003dresp.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_5a7d7ab0","line":1314,"range":{"start_line":1312,"start_character":8,"end_line":1314,"end_character":69},"in_reply_to":"5f7c97a3_8928b297","updated":"2018-06-18 16:31:36.000000000","message":"The way things work now is, if one of these things fails, we\u0027ll log the exception and try again on the next periodic.  (That said, we should definitely react to this one by invalidating the cache entry, so we\u0027re sure to get the updated generation next time around.  -1 for that.)\n\nIf we want to pick through all the report client methods and identify certain ones to retry \"in band\", we could do that.  But I think that\u0027s a separate effort from what we\u0027re doing here, and should be done in a different (set of) patch(es).","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8693be9390681cd710610ff47dc6701f875022af","unresolved":false,"context_lines":[{"line_number":1309,"context_line":"        LOG.error(msg, args)"},{"line_number":1310,"context_line":""},{"line_number":1311,"context_line":"        # If a conflict, raise special conflict exception"},{"line_number":1312,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1313,"context_line":"            raise exception.ResourceProviderUpdateConflict("},{"line_number":1314,"context_line":"                uuid\u003drp_uuid, generation\u003dgeneration, error\u003dresp.text)"},{"line_number":1315,"context_line":""},{"line_number":1316,"context_line":"        # Otherwise, raise generic exception"},{"line_number":1317,"context_line":"        raise exception.ResourceProviderUpdateFailed(url\u003durl, error\u003dresp.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_15ed958d","line":1314,"range":{"start_line":1312,"start_character":8,"end_line":1314,"end_character":69},"in_reply_to":"5f7c97a3_b51b811a","updated":"2018-06-18 17:02:57.000000000","message":"Did the cache invalidation thing (and concomitant testing)","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8814588db6eda0970c3909d7cc47144c8af9d8e8","unresolved":false,"context_lines":[{"line_number":1978,"context_line":"            return"},{"line_number":1979,"context_line":""},{"line_number":1980,"context_line":"        new_aggs \u003d existing_aggs | set([agg_uuid])"},{"line_number":1981,"context_line":"        self.set_aggregates_for_provider("},{"line_number":1982,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse,"},{"line_number":1983,"context_line":"            generation\u003drp[\u0027generation\u0027])"},{"line_number":1984,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_a40fd52d","line":1981,"updated":"2018-06-18 14:39:29.000000000","message":"We should retry on 409 here I think, see my comments above. Similar to claim_resources.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6076e8183e40494e510f5fa00ef20f8a3674e87e","unresolved":false,"context_lines":[{"line_number":1978,"context_line":"            return"},{"line_number":1979,"context_line":""},{"line_number":1980,"context_line":"        new_aggs \u003d existing_aggs | set([agg_uuid])"},{"line_number":1981,"context_line":"        self.set_aggregates_for_provider("},{"line_number":1982,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse,"},{"line_number":1983,"context_line":"            generation\u003drp[\u0027generation\u0027])"},{"line_number":1984,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_d5d95d56","line":1981,"in_reply_to":"5f7c97a3_a40fd52d","updated":"2018-06-18 16:39:18.000000000","message":"We could, sure, but not sure it\u0027s necessary to do that in this patch. Keep in mind that this particular method is called from the nova-api, not the nova-compute, and the report client does *not* have a cached provider tree in that case.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":1980,"context_line":"        new_aggs \u003d existing_aggs | set([agg_uuid])"},{"line_number":1981,"context_line":"        self.set_aggregates_for_provider("},{"line_number":1982,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse,"},{"line_number":1983,"context_line":"            generation\u003drp[\u0027generation\u0027])"},{"line_number":1984,"context_line":""},{"line_number":1985,"context_line":"    def aggregate_remove_host(self, context, agg_uuid, host_name):"},{"line_number":1986,"context_line":"        \"\"\"Looks up a resource provider by the supplied host name, and removes"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_53d56d1a","line":1983,"updated":"2018-06-18 13:23:03.000000000","message":"✓","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":2022,"context_line":"        new_aggs \u003d existing_aggs - set([agg_uuid])"},{"line_number":2023,"context_line":"        self.set_aggregates_for_provider("},{"line_number":2024,"context_line":"            context, rp_uuid, new_aggs, use_cache\u003dFalse,"},{"line_number":2025,"context_line":"            generation\u003drp[\u0027generation\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_f3d91956","line":2025,"updated":"2018-06-18 13:23:03.000000000","message":"✓","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4aece9cbf04cc81b1883682fe8b11ce9bbddf19","unresolved":false,"context_lines":[{"line_number":796,"context_line":"            # NOTE(efried): This will blow up if called for a RP that doesn\u0027t"},{"line_number":797,"context_line":"            # exist in our _provider_tree."},{"line_number":798,"context_line":"            self._provider_tree.update_traits("},{"line_number":799,"context_line":"                rp_uuid, traits, generation\u003dgeneration)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"            if refresh_sharing:"},{"line_number":802,"context_line":"                # Refresh providers associated by aggregate"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_91577402","line":799,"range":{"start_line":799,"start_character":33,"end_line":799,"end_character":54},"updated":"2018-06-18 20:15:56.000000000","message":"This will be a different generation now won\u0027t it?","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a86bcfffba133bcf12d0824bff1bbfbac41ecf15","unresolved":false,"context_lines":[{"line_number":796,"context_line":"            # NOTE(efried): This will blow up if called for a RP that doesn\u0027t"},{"line_number":797,"context_line":"            # exist in our _provider_tree."},{"line_number":798,"context_line":"            self._provider_tree.update_traits("},{"line_number":799,"context_line":"                rp_uuid, traits, generation\u003dgeneration)"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"            if refresh_sharing:"},{"line_number":802,"context_line":"                # Refresh providers associated by aggregate"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_ac9813cc","line":799,"range":{"start_line":799,"start_character":33,"end_line":799,"end_character":54},"in_reply_to":"5f7c97a3_91577402","updated":"2018-06-18 20:59:20.000000000","message":"It really shouldn\u0027t have been - all the above operations are reads.  But the next PS fixes all this stuff up so that generations flow more properly from one op to the next.","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e74efdaaa2f8932b99841a8d6fc8ea1c2a80f43","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"                generation \u003d self._provider_tree.data(rp_uuid).generation"},{"line_number":1280,"context_line":"            else:"},{"line_number":1281,"context_line":"                # Either cache or generation is required"},{"line_number":1282,"context_line":"                raise ValueError("},{"line_number":1283,"context_line":"                    _(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"        url \u003d \u0027/resource_providers/%s/aggregates\u0027 % rp_uuid"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_e31e1eba","line":1282,"range":{"start_line":1282,"start_character":22,"end_line":1282,"end_character":32},"updated":"2018-06-18 19:28:55.000000000","message":"Could add a simple unit test for this.","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a86bcfffba133bcf12d0824bff1bbfbac41ecf15","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"                generation \u003d self._provider_tree.data(rp_uuid).generation"},{"line_number":1280,"context_line":"            else:"},{"line_number":1281,"context_line":"                # Either cache or generation is required"},{"line_number":1282,"context_line":"                raise ValueError("},{"line_number":1283,"context_line":"                    _(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"        url \u003d \u0027/resource_providers/%s/aggregates\u0027 % rp_uuid"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_46c9204f","line":1282,"range":{"start_line":1282,"start_character":22,"end_line":1282,"end_character":32},"in_reply_to":"5f7c97a3_e31e1eba","updated":"2018-06-18 20:59:20.000000000","message":"Done","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8ba98d543b00c983bfa08ff185dabfd54637aba0","unresolved":false,"context_lines":[{"line_number":1314,"context_line":"        if resp.status_code \u003d\u003d 409:"},{"line_number":1315,"context_line":"            # No reason to condition cache invalidation on use_cache - if we"},{"line_number":1316,"context_line":"            # got a 409, the cache entry is still bogus if it exists; and the"},{"line_number":1317,"context_line":"            # below is a no-op if it doesn\u0027t."},{"line_number":1318,"context_line":"            try:"},{"line_number":1319,"context_line":"                self._provider_tree.remove(rp_uuid)"},{"line_number":1320,"context_line":"            except ValueError:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_bd8a8717","line":1317,"updated":"2018-06-18 17:42:11.000000000","message":"ack, makes sense.","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e74efdaaa2f8932b99841a8d6fc8ea1c2a80f43","unresolved":false,"context_lines":[{"line_number":1963,"context_line":"        :param agg_uuid: UUID of the aggregate being modified"},{"line_number":1964,"context_line":"        :param host_name: Name of the nova-compute service worker to look up a"},{"line_number":1965,"context_line":"                          resource provider for"},{"line_number":1966,"context_line":"        :raises: `exceptions.ResourceProviderNotFound` if no resource provider"},{"line_number":1967,"context_line":"                  matching the host name could be found from the placement API"},{"line_number":1968,"context_line":"        :raises: `exception.ResourceProviderAggregateRetrievalFailed` when"},{"line_number":1969,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":1970,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":1971,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":1972,"context_line":"        \"\"\""},{"line_number":1973,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"},{"line_number":1974,"context_line":"        # NOTE(jaypipes): Unfortunately, due to @safe_connect,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_8335ea47","line":1971,"range":{"start_line":1966,"start_character":8,"end_line":1971,"end_character":67},"updated":"2018-06-18 19:28:55.000000000","message":"nit: could add ResourceProviderUpdateConflict to this list now","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a86bcfffba133bcf12d0824bff1bbfbac41ecf15","unresolved":false,"context_lines":[{"line_number":1963,"context_line":"        :param agg_uuid: UUID of the aggregate being modified"},{"line_number":1964,"context_line":"        :param host_name: Name of the nova-compute service worker to look up a"},{"line_number":1965,"context_line":"                          resource provider for"},{"line_number":1966,"context_line":"        :raises: `exceptions.ResourceProviderNotFound` if no resource provider"},{"line_number":1967,"context_line":"                  matching the host name could be found from the placement API"},{"line_number":1968,"context_line":"        :raises: `exception.ResourceProviderAggregateRetrievalFailed` when"},{"line_number":1969,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":1970,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":1971,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":1972,"context_line":"        \"\"\""},{"line_number":1973,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"},{"line_number":1974,"context_line":"        # NOTE(jaypipes): Unfortunately, due to @safe_connect,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_06ec88b9","line":1971,"range":{"start_line":1966,"start_character":8,"end_line":1971,"end_character":67},"in_reply_to":"5f7c97a3_8335ea47","updated":"2018-06-18 20:59:20.000000000","message":"Done","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e74efdaaa2f8932b99841a8d6fc8ea1c2a80f43","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        :param agg_uuid: UUID of the aggregate being modified"},{"line_number":2006,"context_line":"        :param host_name: Name of the nova-compute service worker to look up a"},{"line_number":2007,"context_line":"                          resource provider for"},{"line_number":2008,"context_line":"        :raises: `exceptions.ResourceProviderNotFound` if no resource provider"},{"line_number":2009,"context_line":"                  matching the host name could be found from the placement API"},{"line_number":2010,"context_line":"        :raises: `exception.ResourceProviderAggregateRetrievalFailed` when"},{"line_number":2011,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":2012,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":2013,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":2014,"context_line":"        \"\"\""},{"line_number":2015,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"},{"line_number":2016,"context_line":"        # NOTE(jaypipes): Unfortunately, due to @safe_connect,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_4337d24c","line":2013,"range":{"start_line":2008,"start_character":8,"end_line":2013,"end_character":67},"updated":"2018-06-18 19:28:55.000000000","message":"same","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a86bcfffba133bcf12d0824bff1bbfbac41ecf15","unresolved":false,"context_lines":[{"line_number":2005,"context_line":"        :param agg_uuid: UUID of the aggregate being modified"},{"line_number":2006,"context_line":"        :param host_name: Name of the nova-compute service worker to look up a"},{"line_number":2007,"context_line":"                          resource provider for"},{"line_number":2008,"context_line":"        :raises: `exceptions.ResourceProviderNotFound` if no resource provider"},{"line_number":2009,"context_line":"                  matching the host name could be found from the placement API"},{"line_number":2010,"context_line":"        :raises: `exception.ResourceProviderAggregateRetrievalFailed` when"},{"line_number":2011,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":2012,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":2013,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":2014,"context_line":"        \"\"\""},{"line_number":2015,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"},{"line_number":2016,"context_line":"        # NOTE(jaypipes): Unfortunately, due to @safe_connect,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_c62bf0e7","line":2013,"range":{"start_line":2008,"start_character":8,"end_line":2013,"end_character":67},"in_reply_to":"5f7c97a3_4337d24c","updated":"2018-06-18 20:59:20.000000000","message":"Done","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8559e2a2620643b8f99ccdcb52f2f7cedbff1408","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":405,"context_line":"        :return: A set() of string trait names, which may be empty if the"},{"line_number":406,"context_line":"                 specified provider has no traits."},{"line_number":407,"context_line":"        :return: The resource provider generation."},{"line_number":408,"context_line":"        :raise: ResourceProviderTraitRetrievalFailed on errors.  In particular,"},{"line_number":409,"context_line":"                we raise this exception (as opposed to returning None or the"},{"line_number":410,"context_line":"                empty set()) if the specified resource provider does not exist."}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_b336b8e6","line":407,"updated":"2018-06-25 10:39:39.000000000","message":"We appear to mix \"return\" and \"returns\" in this file. I\u0027m not sure which is right, also not clear if multiple \"return\" means multiple values in the return. Did you do this because you know this is right and you can point me to a reference so I can learn a thing, or was it more a gesture of hope?","commit_id":"882ba5d203393ab86b507e451483c9e7bebd8710"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"08c8edba0b140d13d811acbb55920efd1dd729f5","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":405,"context_line":"        :return: A set() of string trait names, which may be empty if the"},{"line_number":406,"context_line":"                 specified provider has no traits."},{"line_number":407,"context_line":"        :return: The resource provider generation."},{"line_number":408,"context_line":"        :raise: ResourceProviderTraitRetrievalFailed on errors.  In particular,"},{"line_number":409,"context_line":"                we raise this exception (as opposed to returning None or the"},{"line_number":410,"context_line":"                empty set()) if the specified resource provider does not exist."}],"source_content_type":"text/x-python","patch_set":11,"id":"5f7c97a3_47c19ae3","line":407,"in_reply_to":"5f7c97a3_b336b8e6","updated":"2018-06-25 13:56:55.000000000","message":"AFAIK there\u0027s no difference between :return: and :returns:.  My IDE recognizes both forms, but when it autogenerates docstring templates, it uses :return:.  The difference, if there is one, would show up if we generated docs from our docstrings, which we don\u0027t in Nova.  Stephen might know more.\n\nAs for multiple return values, I use this form because it\u0027s what I\u0027ve seen elsewhere for multiple return values.  I think it\u0027s just as clear as\n\n :return: A tuple comprising:\n    - first thing\n    - second thing\n\nbut I don\u0027t have a strong opinion.  Again, I don\u0027t know how it renders.","commit_id":"882ba5d203393ab86b507e451483c9e7bebd8710"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        \"\"\"Queries the placement API for a resource provider\u0027s aggregates."},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A set() of aggregate UUIDs, which may be empty if the"},{"line_number":369,"context_line":"                 specified provider has no aggregate associations."},{"line_number":370,"context_line":"        :return: The resource provider generation."},{"line_number":371,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"},{"line_number":372,"context_line":"                particular, we raise this exception (as opposed to returning"},{"line_number":373,"context_line":"                None or the empty set()) if the specified resource provider"}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_410cf3e9","line":370,"range":{"start_line":368,"start_character":8,"end_line":370,"end_character":50},"updated":"2018-07-10 19:01:11.000000000","message":"I\u0027d prefer to update the existing :return: docstring to say we\u0027re returning a two-tuple containing the set() of aggregates UUIDs and the provider\u0027s generation.","commit_id":"5ad90213f9a661eddc1231bfb7c3c3f4bd1a69b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":403,"context_line":"        :return: A set() of string trait names, which may be empty if the"},{"line_number":404,"context_line":"                 specified provider has no traits."},{"line_number":405,"context_line":"        :return: The resource provider generation."},{"line_number":406,"context_line":"        :raise: ResourceProviderTraitRetrievalFailed on errors.  In particular,"},{"line_number":407,"context_line":"                we raise this exception (as opposed to returning None or the"},{"line_number":408,"context_line":"                empty set()) if the specified resource provider does not exist."}],"source_content_type":"text/x-python","patch_set":14,"id":"5f7c97a3_21221755","line":405,"updated":"2018-07-10 19:01:11.000000000","message":"ditto","commit_id":"5ad90213f9a661eddc1231bfb7c3c3f4bd1a69b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A set() of aggregate UUIDs, which may be empty if the"},{"line_number":369,"context_line":"                 specified provider has no aggregate associations."},{"line_number":370,"context_line":"        :return: The resource provider generation."},{"line_number":371,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"},{"line_number":372,"context_line":"                particular, we raise this exception (as opposed to returning"},{"line_number":373,"context_line":"                None or the empty set()) if the specified resource provider"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_d77be525","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":50},"updated":"2018-07-10 19:01:11.000000000","message":"I\u0027d prefer to update the existing :return: docstring to say we\u0027re returning a two-tuple containing the set() of aggregates UUIDs and the provider\u0027s generation.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A set() of aggregate UUIDs, which may be empty if the"},{"line_number":369,"context_line":"                 specified provider has no aggregate associations."},{"line_number":370,"context_line":"        :return: The resource provider generation."},{"line_number":371,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"},{"line_number":372,"context_line":"                particular, we raise this exception (as opposed to returning"},{"line_number":373,"context_line":"                None or the empty set()) if the specified resource provider"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8db9da16","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":50},"in_reply_to":"5f7c97a3_d77be525","updated":"2018-07-10 20:57:33.000000000","message":"Okay.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":403,"context_line":"        :return: A set() of string trait names, which may be empty if the"},{"line_number":404,"context_line":"                 specified provider has no traits."},{"line_number":405,"context_line":"        :return: The resource provider generation."},{"line_number":406,"context_line":"        :raise: ResourceProviderTraitRetrievalFailed on errors.  In particular,"},{"line_number":407,"context_line":"                we raise this exception (as opposed to returning None or the"},{"line_number":408,"context_line":"                empty set()) if the specified resource provider does not exist."}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_f776213f","line":405,"range":{"start_line":405,"start_character":9,"end_line":405,"end_character":15},"updated":"2018-07-10 19:01:11.000000000","message":"ditto","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":403,"context_line":"        :return: A set() of string trait names, which may be empty if the"},{"line_number":404,"context_line":"                 specified provider has no traits."},{"line_number":405,"context_line":"        :return: The resource provider generation."},{"line_number":406,"context_line":"        :raise: ResourceProviderTraitRetrievalFailed on errors.  In particular,"},{"line_number":407,"context_line":"                we raise this exception (as opposed to returning None or the"},{"line_number":408,"context_line":"                empty set()) if the specified resource provider does not exist."}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_adb4d61d","line":405,"range":{"start_line":405,"start_character":9,"end_line":405,"end_character":15},"in_reply_to":"5f7c97a3_f776213f","updated":"2018-07-10 20:57:33.000000000","message":"Done","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                raise ValueError("},{"line_number":1277,"context_line":"                    _(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        # Check whether aggregates need updating.  We can only do this if we"},{"line_number":1280,"context_line":"        # have a cache entry with a matching generation."},{"line_number":1281,"context_line":"        try:"},{"line_number":1282,"context_line":"            if (self._provider_tree.data(rp_uuid).generation \u003d\u003d generation"},{"line_number":1283,"context_line":"                    and not self._provider_tree.have_aggregates_changed("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_b71669a0","line":1280,"range":{"start_line":1279,"start_character":0,"end_line":1280,"end_character":56},"updated":"2018-07-10 19:01:11.000000000","message":"I don\u0027t really understand why you\u0027re doing this here? Is this just an optimization? Because a) lines 2005 and 2049 already make this check and b) we\u0027re protected from concurrent update of aggregates (or anything else related to the provider) by using the generation in the payload to PUT below.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":1276,"context_line":"                raise ValueError("},{"line_number":1277,"context_line":"                    _(\"generation is required with use_cache\u003dFalse\"))"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"        # Check whether aggregates need updating.  We can only do this if we"},{"line_number":1280,"context_line":"        # have a cache entry with a matching generation."},{"line_number":1281,"context_line":"        try:"},{"line_number":1282,"context_line":"            if (self._provider_tree.data(rp_uuid).generation \u003d\u003d generation"},{"line_number":1283,"context_line":"                    and not self._provider_tree.have_aggregates_changed("}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_8dc31a58","line":1280,"range":{"start_line":1279,"start_character":0,"end_line":1280,"end_character":56},"in_reply_to":"5f7c97a3_b71669a0","updated":"2018-07-10 20:57:33.000000000","message":"- Yes, the checks in aggregate_{add|remove}_host will make this a no-op for those code paths; but we\u0027re *not* doing such a check in update_from_provider_tree - see L1506.\n- This method\u0027s brethren (_set_inventory_for_provider @L1080, set_traits_for_provider @L1201) have the check, so this is good for symmetry.\n- Yes, it\u0027s an optimization. When I originally discovered this race in PS8 (search for ResourceProviderUpdateConflict in [1][2][3][4]) I figured out that some of the times we were hitting it we were \"updating\" the aggregates to what they already were via the update_from_provider_tree path. If we bounce there, we have to refresh the cache for that provider and try again. Before the retry decorator, that would have to wait until the next periodic interval; but even now, it entails a lot of extra placement traffic, which ought to be unnecessary.\n\n[1] http://logs.openstack.org/69/556669/8/check/nova-cells-v1/60e2de1/logs/screen-n-cpu.txt.gz\n[2] http://logs.openstack.org/69/556669/8/check/nova-multiattach/80507a3/logs/screen-n-api.txt.gz\n[3] http://logs.openstack.org/69/556669/8/check/nova-multiattach/80507a3/logs/screen-n-cpu.txt.gz\n[4] http://logs.openstack.org/69/556669/8/check/nova-next/e8b4907/logs/screen-n-cpu.txt.gz","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":1306,"context_line":"                if use_cache:"},{"line_number":1307,"context_line":"                    # The entry should\u0027ve been there"},{"line_number":1308,"context_line":"                    raise"},{"line_number":1309,"context_line":"                LOG.debug("},{"line_number":1310,"context_line":"                    \"Ignoring failure to update the cache in \""},{"line_number":1311,"context_line":"                    \"set_aggregates_for_provider because use_cache\u003dFalse\")"},{"line_number":1312,"context_line":"            return"},{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_17e4fda6","line":1311,"range":{"start_line":1309,"start_character":0,"end_line":1311,"end_character":74},"updated":"2018-07-10 19:01:11.000000000","message":"This doesn\u0027t seem like all that useful of a DEBUG message.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":1306,"context_line":"                if use_cache:"},{"line_number":1307,"context_line":"                    # The entry should\u0027ve been there"},{"line_number":1308,"context_line":"                    raise"},{"line_number":1309,"context_line":"                LOG.debug("},{"line_number":1310,"context_line":"                    \"Ignoring failure to update the cache in \""},{"line_number":1311,"context_line":"                    \"set_aggregates_for_provider because use_cache\u003dFalse\")"},{"line_number":1312,"context_line":"            return"},{"line_number":1313,"context_line":""},{"line_number":1314,"context_line":"        # Some error occurred; log it"}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_2d5766cb","line":1311,"range":{"start_line":1309,"start_character":0,"end_line":1311,"end_character":74},"in_reply_to":"5f7c97a3_17e4fda6","updated":"2018-07-10 20:57:33.000000000","message":"Lets us know that the cache wasn\u0027t populated, which I thought would be useful to know.  But I can remove it.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":733,"context_line":"        if curr is None:"},{"line_number":734,"context_line":"            return None"},{"line_number":735,"context_line":""},{"line_number":736,"context_line":"        cur_gen \u003d curr[\u0027resource_provider_generation\u0027]"},{"line_number":737,"context_line":"        # TODO(efried): This condition banks on the generation for a new RP"},{"line_number":738,"context_line":"        # starting at zero, which isn\u0027t part of the API.  It also is only"},{"line_number":739,"context_line":"        # useful as an optimization on a freshly-created RP to which nothing"},{"line_number":740,"context_line":"        # has ever been done.  And it\u0027s not much of an optimization, because"},{"line_number":741,"context_line":"        # updating the cache is super cheap.  We should remove the condition."},{"line_number":742,"context_line":"        if cur_gen:"},{"line_number":743,"context_line":"            curr_inv \u003d curr[\u0027inventories\u0027]"},{"line_number":744,"context_line":"            self._provider_tree.update_inventory(rp_uuid, curr_inv,"},{"line_number":745,"context_line":"                                                 generation\u003dcur_gen)"},{"line_number":746,"context_line":"        return curr"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"    def _refresh_associations(self, context, rp_uuid, generation\u003dNone,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d02eb4a9","side":"PARENT","line":745,"range":{"start_line":736,"start_character":8,"end_line":745,"end_character":68},"updated":"2018-07-19 19:43:05.000000000","message":"Why does this need to change as part of this patch? How is it related?","commit_id":"fcf406003bcde2b60a515f694f2640e27a9d65fd"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":733,"context_line":"        if curr is None:"},{"line_number":734,"context_line":"            return None"},{"line_number":735,"context_line":""},{"line_number":736,"context_line":"        cur_gen \u003d curr[\u0027resource_provider_generation\u0027]"},{"line_number":737,"context_line":"        # TODO(efried): This condition banks on the generation for a new RP"},{"line_number":738,"context_line":"        # starting at zero, which isn\u0027t part of the API.  It also is only"},{"line_number":739,"context_line":"        # useful as an optimization on a freshly-created RP to which nothing"},{"line_number":740,"context_line":"        # has ever been done.  And it\u0027s not much of an optimization, because"},{"line_number":741,"context_line":"        # updating the cache is super cheap.  We should remove the condition."},{"line_number":742,"context_line":"        if cur_gen:"},{"line_number":743,"context_line":"            curr_inv \u003d curr[\u0027inventories\u0027]"},{"line_number":744,"context_line":"            self._provider_tree.update_inventory(rp_uuid, curr_inv,"},{"line_number":745,"context_line":"                                                 generation\u003dcur_gen)"},{"line_number":746,"context_line":"        return curr"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"    def _refresh_associations(self, context, rp_uuid, generation\u003dNone,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_4498a574","side":"PARENT","line":745,"range":{"start_line":736,"start_character":8,"end_line":745,"end_character":68},"in_reply_to":"5f7c97a3_d02eb4a9","updated":"2018-07-19 20:55:22.000000000","message":"Related because I\u0027m tightening up stuff related to generations. Want me to move it out?","commit_id":"fcf406003bcde2b60a515f694f2640e27a9d65fd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fb52b748065497a1b65306e6bcff8b39285af5a","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.reason \u003d reason"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def retries(f):"},{"line_number":98,"context_line":"    \"\"\"Decorator to retry a call three times if it raises Retry"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    Note that this returns the actual value of the inner call on success"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_6475a1c8","line":97,"updated":"2018-07-19 20:09:13.000000000","message":"We have this also, but it\u0027s not the right fit probably since it returns False to the caller rather than re-raising which is what we want (and also what RetryDecorator in oslo does).","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        \"\"\"Queries the placement API for a resource provider\u0027s aggregates."},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_70d320d4","line":368,"range":{"start_line":368,"start_character":17,"end_line":368,"end_character":37},"updated":"2018-07-19 19:43:05.000000000","message":"I personally prefer we just return the dict response and let the callers handle pulling the keys out that they need, like in get_allocations_for_consumer and _get_inventory, but I\u0027m not married to it. But if _get_provider_traits is being updated in this otherwise aggregate-specific change, it seems like we\u0027re splitting in two different directions which defeats the purpose of trying to be consistent.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"04385b83931f3eb4a92ada1503453c9a2298d66c","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        \"\"\"Queries the placement API for a resource provider\u0027s aggregates."},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_c498351b","line":368,"range":{"start_line":368,"start_character":17,"end_line":368,"end_character":37},"in_reply_to":"5f7c97a3_6493615a","updated":"2018-07-19 20:06:36.000000000","message":"OK, personal preference. The documentation is in the API reference, but arguably people can do a shitty job of saying that\u0027s how the internal interfaces on stuff like this works. Named tuples would help if we\u0027re going a tuple-everywhere route, but that\u0027s a later refactor also.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5e72ce969c06620dcc399c19ba3374ca82b54897","unresolved":false,"context_lines":[{"line_number":365,"context_line":"        \"\"\"Queries the placement API for a resource provider\u0027s aggregates."},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_6493615a","line":368,"range":{"start_line":368,"start_character":17,"end_line":368,"end_character":37},"in_reply_to":"5f7c97a3_70d320d4","updated":"2018-07-19 19:52:38.000000000","message":"-1 to just returning the response dict. I\u0027d asked Eric to do this. I don\u0027t like just dumping the returned response JSON object back to callers because it\u0027s a jelly-like interface that tends to change over time and change in ways that are neither documented nor recognized.\n\nI\u0027ve asked that the get_allocations_for_consumer() return value be changed to a tuple instead of just dumping the response JSON object back out of the function.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."},{"line_number":372,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_3044c89e","line":369,"range":{"start_line":369,"start_character":40,"end_line":369,"end_character":51},"updated":"2018-07-19 19:43:05.000000000","message":"aggregate uuids?","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."},{"line_number":372,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_64d921ef","line":369,"range":{"start_line":369,"start_character":40,"end_line":369,"end_character":51},"in_reply_to":"5f7c97a3_3044c89e","updated":"2018-07-19 20:55:22.000000000","message":"Done","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."},{"line_number":372,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"},{"line_number":373,"context_line":"                particular, we raise this exception (as opposed to returning"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d0c47410","line":370,"range":{"start_line":370,"start_character":48,"end_line":370,"end_character":54},"updated":"2018-07-19 19:43:05.000000000","message":"aggregates","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        :param rp_uuid: UUID of the resource provider to grab aggregates for."},{"line_number":368,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":369,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":370,"context_line":"                      specified provider has no traits."},{"line_number":371,"context_line":"                    * The resource provider generation."},{"line_number":372,"context_line":"        :raise: ResourceProviderAggregateRetrievalFailed on errors.  In"},{"line_number":373,"context_line":"                particular, we raise this exception (as opposed to returning"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_04ee2d8a","line":370,"range":{"start_line":370,"start_character":48,"end_line":370,"end_character":54},"in_reply_to":"5f7c97a3_d0c47410","updated":"2018-07-19 20:55:22.000000000","message":"Done","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":401,"context_line":""},{"line_number":402,"context_line":"        :param context: The security context"},{"line_number":403,"context_line":"        :param rp_uuid: UUID of the resource provider to grab traits for."},{"line_number":404,"context_line":"        :return: A 2-tuple comprising:"},{"line_number":405,"context_line":"                    * A set() of string trait names, which may be empty if the"},{"line_number":406,"context_line":"                      specified provider has no traits."},{"line_number":407,"context_line":"                    * The resource provider generation."}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_f095f0e3","line":404,"range":{"start_line":404,"start_character":17,"end_line":404,"end_character":38},"updated":"2018-07-19 19:43:05.000000000","message":"same","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":774,"context_line":"        \"\"\""},{"line_number":775,"context_line":"        if force or self._associations_stale(rp_uuid):"},{"line_number":776,"context_line":"            # Refresh aggregates"},{"line_number":777,"context_line":"            aggs, generation \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":778,"context_line":"            msg \u003d (\"Refreshing aggregate associations for resource provider \""},{"line_number":779,"context_line":"                   \"%s, aggregates: %s\")"},{"line_number":780,"context_line":"            LOG.debug(msg, rp_uuid, \u0027,\u0027.join(aggs or [\u0027None\u0027]))"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_d0179453","line":777,"range":{"start_line":777,"start_character":12,"end_line":777,"end_character":78},"updated":"2018-07-19 19:43:05.000000000","message":"Because of @safe_connect on _get_provider_aggregates (and _get_provider_traits), we could get a NoneType error here since @safe_connect can return None and None isn\u0027t iterable. Something to think about wrt returning the full response body dict rather than a 2-item tuple, because it looks like the code before this could handle safe_connect returning None.\n\nMaybe at this point if safe_connect does return None we don\u0027t care and are OK with a system error, I haven\u0027t audited that far.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":774,"context_line":"        \"\"\""},{"line_number":775,"context_line":"        if force or self._associations_stale(rp_uuid):"},{"line_number":776,"context_line":"            # Refresh aggregates"},{"line_number":777,"context_line":"            aggs, generation \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":778,"context_line":"            msg \u003d (\"Refreshing aggregate associations for resource provider \""},{"line_number":779,"context_line":"                   \"%s, aggregates: %s\")"},{"line_number":780,"context_line":"            LOG.debug(msg, rp_uuid, \u0027,\u0027.join(aggs or [\u0027None\u0027]))"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_847b1d27","line":777,"range":{"start_line":777,"start_character":12,"end_line":777,"end_character":78},"in_reply_to":"5f7c97a3_d0179453","updated":"2018-07-19 20:55:22.000000000","message":"That was exactly my thought here. It\u0027s silly that we ever attempt to continue operating \"normally\" when @safe_connect returns None. I *want* that condition to raise a horrendous unhandled exception here. And I want @safe_connect to diaf.\n\nNote that the way this was before, we would have gotten a similarly horrendous unhandled error at L794 anyway. So can I not fix this here, since I didn\u0027t break it here?","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            \u0027status_code\u0027: resp.status_code,"},{"line_number":1322,"context_line":"            \u0027err_text\u0027: resp.text,"},{"line_number":1323,"context_line":"        }"},{"line_number":1324,"context_line":"        LOG.error(msg, args)"},{"line_number":1325,"context_line":""},{"line_number":1326,"context_line":"        # If a conflict, invalidate the cache and raise special exception"},{"line_number":1327,"context_line":"        if resp.status_code \u003d\u003d 409:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_c4ecb529","line":1324,"range":{"start_line":1324,"start_character":12,"end_line":1324,"end_character":17},"updated":"2018-07-19 19:43:05.000000000","message":"We probably don\u0027t need to log an error on 409 do we? Especially if we\u0027re going to automatically retry. Maybe we should make this a warning in the 409 case and move the LOG.error above the generic raise ResourceProviderUpdateFailed.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            \u0027status_code\u0027: resp.status_code,"},{"line_number":1322,"context_line":"            \u0027err_text\u0027: resp.text,"},{"line_number":1323,"context_line":"        }"},{"line_number":1324,"context_line":"        LOG.error(msg, args)"},{"line_number":1325,"context_line":""},{"line_number":1326,"context_line":"        # If a conflict, invalidate the cache and raise special exception"},{"line_number":1327,"context_line":"        if resp.status_code \u003d\u003d 409:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_c7027708","line":1324,"range":{"start_line":1324,"start_character":12,"end_line":1324,"end_character":17},"in_reply_to":"5f7c97a3_c4ecb529","updated":"2018-07-19 20:55:22.000000000","message":"Done, I think.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":1984,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":1985,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":1986,"context_line":"        :raises: `exception.ResourceProviderUpdateConflict` if a concurrent"},{"line_number":1987,"context_line":"                 update to the provider was detected."},{"line_number":1988,"context_line":"        \"\"\""},{"line_number":1989,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_042ced47","line":1986,"updated":"2018-07-19 19:43:05.000000000","message":"We should handle this in the calling code in the API now too:\n\nhttps://github.com/openstack/nova/blob/50f40854b04351fb622fd8b68b374a8fe8ca2070/nova/compute/api.py#L5307","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":1983,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":1984,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":1985,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":1986,"context_line":"        :raises: `exception.ResourceProviderUpdateConflict` if a concurrent"},{"line_number":1987,"context_line":"                 update to the provider was detected."},{"line_number":1988,"context_line":"        \"\"\""},{"line_number":1989,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_472f4788","line":1986,"in_reply_to":"5f7c97a3_042ced47","updated":"2018-07-19 20:55:22.000000000","message":"Done","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":2027,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":2028,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":2029,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":2030,"context_line":"        :raises: `exception.ResourceProviderUpdateConflict` if a concurrent"},{"line_number":2031,"context_line":"                 update to the provider was detected."},{"line_number":2032,"context_line":"        \"\"\""},{"line_number":2033,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_c4211551","line":2030,"updated":"2018-07-19 19:43:05.000000000","message":"And here:\n\nhttps://github.com/openstack/nova/blob/50f40854b04351fb622fd8b68b374a8fe8ca2070/nova/compute/api.py#L5372","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":2027,"context_line":"                 failing to get a provider\u0027s existing aggregates"},{"line_number":2028,"context_line":"        :raises: `exception.ResourceProviderUpdateFailed` if there was a"},{"line_number":2029,"context_line":"                 failure attempting to save the provider aggregates"},{"line_number":2030,"context_line":"        :raises: `exception.ResourceProviderUpdateConflict` if a concurrent"},{"line_number":2031,"context_line":"                 update to the provider was detected."},{"line_number":2032,"context_line":"        \"\"\""},{"line_number":2033,"context_line":"        rp \u003d self._get_provider_by_name(context, host_name)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_672a8378","line":2030,"in_reply_to":"5f7c97a3_c4211551","updated":"2018-07-19 20:55:22.000000000","message":"Done","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e10a80d4c2ad9ff08906b2f683a0517d2d1d7ff","unresolved":false,"context_lines":[{"line_number":784,"context_line":"            # Refresh aggregates"},{"line_number":785,"context_line":"            agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":786,"context_line":"            # If @safe_connect makes the above return None, this will raise"},{"line_number":787,"context_line":"            # TypeError. Good."},{"line_number":788,"context_line":"            aggs, generation \u003d agg_info.aggregates, agg_info.generation"},{"line_number":789,"context_line":"            msg \u003d (\"Refreshing aggregate associations for resource provider \""},{"line_number":790,"context_line":"                   \"%s, aggregates: %s\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_dd8008f9","line":787,"range":{"start_line":787,"start_character":25,"end_line":787,"end_character":30},"updated":"2018-07-19 21:44:39.000000000","message":"Good because...it\u0027s a system error and we want to halt?","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3395e6bfea51d8fa22e3532a957522914bb228f2","unresolved":false,"context_lines":[{"line_number":784,"context_line":"            # Refresh aggregates"},{"line_number":785,"context_line":"            agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":786,"context_line":"            # If @safe_connect makes the above return None, this will raise"},{"line_number":787,"context_line":"            # TypeError. Good."},{"line_number":788,"context_line":"            aggs, generation \u003d agg_info.aggregates, agg_info.generation"},{"line_number":789,"context_line":"            msg \u003d (\"Refreshing aggregate associations for resource provider \""},{"line_number":790,"context_line":"                   \"%s, aggregates: %s\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_38e02a27","line":787,"range":{"start_line":787,"start_character":25,"end_line":787,"end_character":30},"in_reply_to":"5f7c97a3_3dbfbcab","updated":"2018-07-19 22:19:41.000000000","message":"I wanted to follow up and make safe_connect actually raise PlacementAPIConnectFailure whenever it... um... has a Failure to Connect to the Placement API. But I saw a comment from Jay that we want to do that in Stein - not sure why we\u0027re waiting.","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d52946b6bad293f4563bb9a39f5669ec88644746","unresolved":false,"context_lines":[{"line_number":784,"context_line":"            # Refresh aggregates"},{"line_number":785,"context_line":"            agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":786,"context_line":"            # If @safe_connect makes the above return None, this will raise"},{"line_number":787,"context_line":"            # TypeError. Good."},{"line_number":788,"context_line":"            aggs, generation \u003d agg_info.aggregates, agg_info.generation"},{"line_number":789,"context_line":"            msg \u003d (\"Refreshing aggregate associations for resource provider \""},{"line_number":790,"context_line":"                   \"%s, aggregates: %s\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_3dbfbcab","line":787,"range":{"start_line":787,"start_character":25,"end_line":787,"end_character":30},"in_reply_to":"5f7c97a3_dd8008f9","updated":"2018-07-19 21:46:04.000000000","message":"Might be friendlier to debug if we raised PlacementAPIConnectFailure like below, but could also be a follow up.","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e10a80d4c2ad9ff08906b2f683a0517d2d1d7ff","unresolved":false,"context_lines":[{"line_number":2020,"context_line":"        agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":2021,"context_line":"        # @safe_connect can make the above return None"},{"line_number":2022,"context_line":"        if agg_info is None:"},{"line_number":2023,"context_line":"            raise exception.PlacementAPIConnectFailure()"},{"line_number":2024,"context_line":"        existing_aggs, gen \u003d agg_info.aggregates, agg_info.generation"},{"line_number":2025,"context_line":"        if agg_uuid in existing_aggs:"},{"line_number":2026,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_bd92ec48","line":2023,"range":{"start_line":2023,"start_character":28,"end_line":2023,"end_character":54},"updated":"2018-07-19 21:44:39.000000000","message":"nit: this could go in the docstring now\n\nAnd we\u0027re OK with this b/c it\u0027s handled by the API:\n\nhttps://github.com/openstack/nova/blob/50f40854b04351fb622fd8b68b374a8fe8ca2070/nova/compute/api.py#L5297","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3395e6bfea51d8fa22e3532a957522914bb228f2","unresolved":false,"context_lines":[{"line_number":2020,"context_line":"        agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":2021,"context_line":"        # @safe_connect can make the above return None"},{"line_number":2022,"context_line":"        if agg_info is None:"},{"line_number":2023,"context_line":"            raise exception.PlacementAPIConnectFailure()"},{"line_number":2024,"context_line":"        existing_aggs, gen \u003d agg_info.aggregates, agg_info.generation"},{"line_number":2025,"context_line":"        if agg_uuid in existing_aggs:"},{"line_number":2026,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_98be1efe","line":2023,"range":{"start_line":2023,"start_character":28,"end_line":2023,"end_character":54},"in_reply_to":"5f7c97a3_bd92ec48","updated":"2018-07-19 22:19:41.000000000","message":"True, it could have gone in the docstring before (L2012) too.","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e10a80d4c2ad9ff08906b2f683a0517d2d1d7ff","unresolved":false,"context_lines":[{"line_number":2070,"context_line":"        agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":2071,"context_line":"        # @safe_connect can make the above return None"},{"line_number":2072,"context_line":"        if agg_info is None:"},{"line_number":2073,"context_line":"            raise exception.PlacementAPIConnectFailure()"},{"line_number":2074,"context_line":"        existing_aggs, gen \u003d agg_info.aggregates, agg_info.generation"},{"line_number":2075,"context_line":"        if agg_uuid not in existing_aggs:"},{"line_number":2076,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"5f7c97a3_7da0d491","line":2073,"range":{"start_line":2073,"start_character":28,"end_line":2073,"end_character":54},"updated":"2018-07-19 21:44:39.000000000","message":"same\n\nAnd we\u0027re OK:\n\nhttps://github.com/openstack/nova/blob/50f40854b04351fb622fd8b68b374a8fe8ca2070/nova/compute/api.py#L5362","commit_id":"31de9d9a32033c93815e5c3a119eb0d1d69a2acc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2f4964fbc250496604a2678c27414d8d8b4bfd2f","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import functools"},{"line_number":20,"context_line":"import random"},{"line_number":21,"context_line":"import re"},{"line_number":22,"context_line":"import retrying"},{"line_number":23,"context_line":"import time"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from keystoneauth1 import exceptions as ks_exc"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_93dd8a14","line":22,"updated":"2018-07-20 18:22:50.000000000","message":"same","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"27ad73ccb0a0e4a1d1592a836c5d4655e59ba957","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import functools"},{"line_number":20,"context_line":"import random"},{"line_number":21,"context_line":"import re"},{"line_number":22,"context_line":"import retrying"},{"line_number":23,"context_line":"import time"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from keystoneauth1 import exceptions as ks_exc"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f79a3b5_698eea79","line":22,"in_reply_to":"5f7c97a3_93dd8a14","updated":"2018-09-06 16:12:51.000000000","message":"Done","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a20c4ba963f61a8c1eeb242fc715fcd342c44f1d","unresolved":false,"context_lines":[{"line_number":2035,"context_line":"        agg_info \u003d self._get_provider_aggregates(context, rp_uuid)"},{"line_number":2036,"context_line":"        # @safe_connect can make the above return None"},{"line_number":2037,"context_line":"        if agg_info is None:"},{"line_number":2038,"context_line":"            raise exception.PlacementAPIConnectFailure()"},{"line_number":2039,"context_line":"        existing_aggs, gen \u003d agg_info.aggregates, agg_info.generation"},{"line_number":2040,"context_line":"        if agg_uuid in existing_aggs:"},{"line_number":2041,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_d93c812e","line":2038,"updated":"2018-07-20 19:49:13.000000000","message":"✓","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"}],"nova/tests/functional/test_report_client.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":942,"context_line":""},{"line_number":943,"context_line":"    @mock.patch(\u0027nova.compute.provider_tree.ProviderTree.update_aggregates\u0027)"},{"line_number":944,"context_line":"    def test_non_tree_aggregate_membership(self, upd_aggs_mock):"},{"line_number":945,"context_line":"        \"\"\"There are some methods of the reportclient that interact with the"},{"line_number":946,"context_line":"        reportclient\u0027s provider_tree cache of information on a best-effort"},{"line_number":947,"context_line":"        basis. These methods are called to add and remove members from a nova"},{"line_number":948,"context_line":"        host aggregate and ensure that the placement API has a mirrored record"},{"line_number":949,"context_line":"        of the resource provider\u0027s aggregate associations."},{"line_number":950,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_f76f01f7","line":947,"range":{"start_line":945,"start_character":59,"end_line":947,"end_character":14},"updated":"2018-07-10 19:01:11.000000000","message":"I don\u0027t see why you changed this wording. The SchedulerReportClient.aggregate_add_host() and aggregate_remove_host() were specifically designed not to touch the provider_tree attribute (the client\u0027s cache) because we do not want each nova-api process to balloon with a bunch of memory for each compute node in the entire deployment going into the reportclient\u0027s provider_tree cache.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":942,"context_line":""},{"line_number":943,"context_line":"    @mock.patch(\u0027nova.compute.provider_tree.ProviderTree.update_aggregates\u0027)"},{"line_number":944,"context_line":"    def test_non_tree_aggregate_membership(self, upd_aggs_mock):"},{"line_number":945,"context_line":"        \"\"\"There are some methods of the reportclient that interact with the"},{"line_number":946,"context_line":"        reportclient\u0027s provider_tree cache of information on a best-effort"},{"line_number":947,"context_line":"        basis. These methods are called to add and remove members from a nova"},{"line_number":948,"context_line":"        host aggregate and ensure that the placement API has a mirrored record"},{"line_number":949,"context_line":"        of the resource provider\u0027s aggregate associations."},{"line_number":950,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_b55ae01b","line":947,"range":{"start_line":945,"start_character":59,"end_line":947,"end_character":14},"in_reply_to":"5f7c97a3_f76f01f7","updated":"2018-07-10 20:57:33.000000000","message":"Maybe I\u0027m being too anal, but it is no longer true to say that the reportclient methods in question (aggregate_{add|remove}_host) do not interact with the cache. They try to do so, and ignore errors arising from it not being populated (\"best-effort\"). One of the main purposes of the test case is to make sure of this. We were originally doing that with checks like the one on L978 (making sure the added agg doesn\u0027t end up in the cache). This patch adds validation for the best-effort-ness via the ValueError side_effect on L953.\n\nIt *is* true (hopefully) that the *API* service is careful to avoid populating the cache when it *uses* these methods, for the reason you stated; but this test case is in the scope of the report client, not in the scope of the API service.\n\nIn fact, we could make this even closer to the n-api use case by blowing away the cache after get_provider_tree_and_ensure_root (L958-9) and changing the .data() checks to assert that the cache is never repopulated with the compute_uuid entry.  \u003c\u003d\u003d Done.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d0e79cabf3cb7f7f6d4f40e30c9c4b0db15ebe93","unresolved":false,"context_lines":[{"line_number":993,"context_line":"            #  Try removing the same host and verify no error"},{"line_number":994,"context_line":"            self.client.aggregate_remove_host("},{"line_number":995,"context_line":"                self.context, agg_uuid, self.compute_name)"},{"line_number":996,"context_line":"        self.assertEqual(2, upd_aggs_mock.call_count)"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"    def test_alloc_cands_smoke(self):"},{"line_number":999,"context_line":"        \"\"\"Simple call to get_allocation_candidates for version checking.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_373819eb","line":996,"range":{"start_line":996,"start_character":8,"end_line":996,"end_character":53},"updated":"2018-07-10 19:01:11.000000000","message":"this should not be hit with either aggregate_add_host() or aggregate_remove_host()","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"24e5e77c08c833d57ad094c8cf31859605890a9d","unresolved":false,"context_lines":[{"line_number":993,"context_line":"            #  Try removing the same host and verify no error"},{"line_number":994,"context_line":"            self.client.aggregate_remove_host("},{"line_number":995,"context_line":"                self.context, agg_uuid, self.compute_name)"},{"line_number":996,"context_line":"        self.assertEqual(2, upd_aggs_mock.call_count)"},{"line_number":997,"context_line":""},{"line_number":998,"context_line":"    def test_alloc_cands_smoke(self):"},{"line_number":999,"context_line":"        \"\"\"Simple call to get_allocation_candidates for version checking.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"5f7c97a3_95bfa4aa","line":996,"range":{"start_line":996,"start_character":8,"end_line":996,"end_character":53},"in_reply_to":"5f7c97a3_373819eb","updated":"2018-07-10 20:57:33.000000000","message":"True before, but now we\u0027re attempting to hit the cache and ignoring ValueError when use_cache\u003dFalse.\n\nAnyway, I removed the upd_aggs_mock to make the test deeper as described above.","commit_id":"3a8544a8b6b1d669b498a7196ec83c093bff8cf8"}],"nova/tests/unit/compute/test_resource_tracker.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"322028575a5495ccce156f2410e5de376f4590c8","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"        exp_inv[rc_fields.ResourceClass.DISK_GB][\u0027reserved\u0027] \u003d 1"},{"line_number":1387,"context_line":"        self.assertEqual(exp_inv, ptree.data(new_compute.uuid).inventory)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"    @mock.patch(\u0027nova.objects.ComputeNode.save\u0027, new\u003dmock.Mock())"},{"line_number":1390,"context_line":"    def test_update_retry_success(self):"},{"line_number":1391,"context_line":"        self._setup_rt()"},{"line_number":1392,"context_line":"        orig_compute \u003d _COMPUTE_NODE_FIXTURES[0].obj_clone()"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_b91b007e","line":1389,"range":{"start_line":1389,"start_character":47,"end_line":1389,"end_character":64},"updated":"2018-07-18 21:27:13.000000000","message":"you sure like doing this, don\u0027t you? :) even though the remainder of the test file doesn\u0027t do it...","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"126ec569bd44037e208f12933f457ec91c600b6a","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"        exp_inv[rc_fields.ResourceClass.DISK_GB][\u0027reserved\u0027] \u003d 1"},{"line_number":1387,"context_line":"        self.assertEqual(exp_inv, ptree.data(new_compute.uuid).inventory)"},{"line_number":1388,"context_line":""},{"line_number":1389,"context_line":"    @mock.patch(\u0027nova.objects.ComputeNode.save\u0027, new\u003dmock.Mock())"},{"line_number":1390,"context_line":"    def test_update_retry_success(self):"},{"line_number":1391,"context_line":"        self._setup_rt()"},{"line_number":1392,"context_line":"        orig_compute \u003d _COMPUTE_NODE_FIXTURES[0].obj_clone()"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_5439539f","line":1389,"range":{"start_line":1389,"start_character":47,"end_line":1389,"end_character":64},"in_reply_to":"5f7c97a3_b91b007e","updated":"2018-07-18 21:40:09.000000000","message":"I definitely prefer this to unused parameters on the test method, yes.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"}],"nova/tests/unit/scheduler/client/test_report.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"760999537f001c5c09456abf2f4770f00add2bde","unresolved":false,"context_lines":[{"line_number":1957,"context_line":"        self.client.set_aggregates_for_provider(self.context, uuids.rp, aggs)"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"        exp_payload \u003d {\u0027aggregates\u0027: aggs,"},{"line_number":1960,"context_line":"                       \u0027resource_provider_generation\u0027: 0}"},{"line_number":1961,"context_line":"        self.ks_adap_mock.put.assert_called_once_with("},{"line_number":1962,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.rp, json\u003dexp_payload,"},{"line_number":1963,"context_line":"            raise_exc\u003dFalse, microversion\u003d\u00271.19\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_d3e09dfb","line":1960,"range":{"start_line":1960,"start_character":55,"end_line":1960,"end_character":56},"updated":"2018-06-18 13:23:03.000000000","message":"wouldn\u0027t this be 1? you set it to 1 above on line 1948...","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6076e8183e40494e510f5fa00ef20f8a3674e87e","unresolved":false,"context_lines":[{"line_number":1957,"context_line":"        self.client.set_aggregates_for_provider(self.context, uuids.rp, aggs)"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"        exp_payload \u003d {\u0027aggregates\u0027: aggs,"},{"line_number":1960,"context_line":"                       \u0027resource_provider_generation\u0027: 0}"},{"line_number":1961,"context_line":"        self.ks_adap_mock.put.assert_called_once_with("},{"line_number":1962,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.rp, json\u003dexp_payload,"},{"line_number":1963,"context_line":"            raise_exc\u003dFalse, microversion\u003d\u00271.19\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_35be91d4","line":1960,"range":{"start_line":1960,"start_character":55,"end_line":1960,"end_character":56},"in_reply_to":"5f7c97a3_15e6f54b","updated":"2018-06-18 16:39:18.000000000","message":"Ah, gotcha.","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0a025d88a928677b35dc1030dc06ed25efb049e1","unresolved":false,"context_lines":[{"line_number":1957,"context_line":"        self.client.set_aggregates_for_provider(self.context, uuids.rp, aggs)"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"        exp_payload \u003d {\u0027aggregates\u0027: aggs,"},{"line_number":1960,"context_line":"                       \u0027resource_provider_generation\u0027: 0}"},{"line_number":1961,"context_line":"        self.ks_adap_mock.put.assert_called_once_with("},{"line_number":1962,"context_line":"            \u0027/resource_providers/%s/aggregates\u0027 % uuids.rp, json\u003dexp_payload,"},{"line_number":1963,"context_line":"            raise_exc\u003dFalse, microversion\u003d\u00271.19\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_15e6f54b","line":1960,"range":{"start_line":1960,"start_character":55,"end_line":1960,"end_character":56},"in_reply_to":"5f7c97a3_d3e09dfb","updated":"2018-06-18 16:31:36.000000000","message":"This is the payload we expect to call PUT with.  The one on L1948 is what we expect to get *back* from that PUT call; and we check that that generation value made it into the cache on L1968.  (So the generations are actually correct in terms of what they would be in real life, though I could have made them totally random as long as L1948 matches L1968.)","commit_id":"30f199ce9101e8eef18626b99ed1ca7bc3a3edbc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e74efdaaa2f8932b99841a8d6fc8ea1c2a80f43","unresolved":false,"context_lines":[{"line_number":1982,"context_line":"    def test_set_aggregates_for_provider_conflict(self):"},{"line_number":1983,"context_line":"        # Prime the provider tree cache"},{"line_number":1984,"context_line":"        self.client._provider_tree.new_root(\u0027rp\u0027, uuids.rp, 0)"},{"line_number":1985,"context_line":"        self.ks_adap_mock.put.return_value \u003d mock.Mock(status_code\u003d409)"},{"line_number":1986,"context_line":"        self.assertRaises("},{"line_number":1987,"context_line":"            exception.ResourceProviderUpdateConflict,"},{"line_number":1988,"context_line":"            self.client.set_aggregates_for_provider,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_830e4a8a","line":1985,"range":{"start_line":1985,"start_character":45,"end_line":1985,"end_character":71},"updated":"2018-06-18 19:28:55.000000000","message":"nit: why not use your FakeResponse class here?","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a86bcfffba133bcf12d0824bff1bbfbac41ecf15","unresolved":false,"context_lines":[{"line_number":1982,"context_line":"    def test_set_aggregates_for_provider_conflict(self):"},{"line_number":1983,"context_line":"        # Prime the provider tree cache"},{"line_number":1984,"context_line":"        self.client._provider_tree.new_root(\u0027rp\u0027, uuids.rp, 0)"},{"line_number":1985,"context_line":"        self.ks_adap_mock.put.return_value \u003d mock.Mock(status_code\u003d409)"},{"line_number":1986,"context_line":"        self.assertRaises("},{"line_number":1987,"context_line":"            exception.ResourceProviderUpdateConflict,"},{"line_number":1988,"context_line":"            self.client.set_aggregates_for_provider,"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_4637e04c","line":1985,"range":{"start_line":1985,"start_character":45,"end_line":1985,"end_character":71},"in_reply_to":"5f7c97a3_830e4a8a","updated":"2018-06-18 20:59:20.000000000","message":"Copy/paste.  Done.","commit_id":"786861b6e783795aa0671425bf7fc80edbb22905"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a20c4ba963f61a8c1eeb242fc715fcd342c44f1d","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            self.client._provider_tree.has_traits(uuids.compute_node,"},{"line_number":1220,"context_line":"                                                  [\u0027CUSTOM_SILVER\u0027]))"},{"line_number":1221,"context_line":"        get_shr_mock.assert_called_once_with(self.context, set([uuids.agg1]))"},{"line_number":1222,"context_line":"        self.assertEqual("},{"line_number":1223,"context_line":"            43,"},{"line_number":1224,"context_line":"            self.client._provider_tree.data(uuids.compute_node).generation)"},{"line_number":1225,"context_line":"        self.assertFalse(create_rp_mock.called)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":"    @mock.patch(\u0027nova.scheduler.client.report.SchedulerReportClient.\u0027"}],"source_content_type":"text/x-python","patch_set":21,"id":"5f7c97a3_79768d0e","line":1224,"range":{"start_line":1222,"start_character":8,"end_line":1224,"end_character":75},"updated":"2018-07-20 19:49:13.000000000","message":"note for future Eric...\n\nmight be a tad more pleasant to look at like so:\n\n gen \u003d self.client._provider_tree.data(uuids.compute_node).generation\n self.assertEqual(43, gen)","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"27ad73ccb0a0e4a1d1592a836c5d4655e59ba957","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"            self.client._provider_tree.has_traits(uuids.compute_node,"},{"line_number":1220,"context_line":"                                                  [\u0027CUSTOM_SILVER\u0027]))"},{"line_number":1221,"context_line":"        get_shr_mock.assert_called_once_with(self.context, set([uuids.agg1]))"},{"line_number":1222,"context_line":"        self.assertEqual("},{"line_number":1223,"context_line":"            43,"},{"line_number":1224,"context_line":"            self.client._provider_tree.data(uuids.compute_node).generation)"},{"line_number":1225,"context_line":"        self.assertFalse(create_rp_mock.called)"},{"line_number":1226,"context_line":""},{"line_number":1227,"context_line":"    @mock.patch(\u0027nova.scheduler.client.report.SchedulerReportClient.\u0027"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f79a3b5_89890670","line":1224,"range":{"start_line":1222,"start_character":8,"end_line":1224,"end_character":75},"in_reply_to":"5f7c97a3_79768d0e","updated":"2018-09-06 16:12:51.000000000","message":"Done","commit_id":"3518ccb665b9b6374c476b4c2e63fa966aee1f3a"}],"nova/utils.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c34f35f655d97c8c41daa91146a2433fa4681575","unresolved":false,"context_lines":[{"line_number":1298,"context_line":"    return \"\""},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"def retry_on_exception(exc):"},{"line_number":1302,"context_line":"    \"\"\"Decorator to retry a method if it raises specific exceptions."},{"line_number":1303,"context_line":""},{"line_number":1304,"context_line":"    Used as:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_e43d318d","line":1301,"updated":"2018-07-19 19:43:05.000000000","message":"Why write our own when we already have the RetryDecorator in oslo.service which is already in use in some places within nova?\n\nhttps://github.com/openstack/oslo.service/blob/master/oslo_service/loopingcall.py#L401\n\nIt\u0027s like the exact same thing, except more configurable.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2aa8b40e50c733ec85ff1c14d038d824acf3a3a3","unresolved":false,"context_lines":[{"line_number":1298,"context_line":"    return \"\""},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"def retry_on_exception(exc):"},{"line_number":1302,"context_line":"    \"\"\"Decorator to retry a method if it raises specific exceptions."},{"line_number":1303,"context_line":""},{"line_number":1304,"context_line":"    Used as:"}],"source_content_type":"text/x-python","patch_set":17,"id":"5f7c97a3_474987b5","line":1301,"in_reply_to":"5f7c97a3_e43d318d","updated":"2018-07-19 20:55:22.000000000","message":"Done. Though I preferred my logging.","commit_id":"e92db8250724f1f233d452fed49317fc26e5f43d"}]}