)]}'
{"nova/scheduler/client/report.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a72a207689db815654dc577355e046c5bdfc12b8","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % compute_node.uuid"},{"line_number":242,"context_line":"        result \u003d self.get(url)"},{"line_number":243,"context_line":"        if not result:"},{"line_number":244,"context_line":"            return []"},{"line_number":245,"context_line":"        return result.json()"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_5a24f5c3","line":244,"updated":"2016-09-07 12:55:23.000000000","message":"You return an empty list here, but result.json() below will return a dict with a key \u0027inventories\u0027 that itself is a dict keyed by resource class string.","commit_id":"506f64a29da26acffb356a9bc85d921657be150b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a72a207689db815654dc577355e046c5bdfc12b8","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":276,"context_line":"        # VCPU inventory, so use that."},{"line_number":277,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":278,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][\u0027VCPU\u0027]"},{"line_number":279,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":280,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_7a2d39a7","line":277,"updated":"2016-09-07 12:55:23.000000000","message":"See note above. If you return [] from _get_inventory(), the above line will blow up.","commit_id":"506f64a29da26acffb356a9bc85d921657be150b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a72a207689db815654dc577355e046c5bdfc12b8","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        # Check to see if we need to update placement\u0027s view"},{"line_number":289,"context_line":"        if self._compare_inventory(data[\u0027inventories\u0027],"},{"line_number":290,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":291,"context_line":"            LOG.debug(\u0027No inventory change needed\u0027)"},{"line_number":292,"context_line":"            return True"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_9a2afd8e","line":291,"updated":"2016-09-07 12:55:23.000000000","message":"Not sure the above debug log line is helpful. What would be helpful would be a log message that shows what resource amounts actually changed between periodic update_available_resource() calls. That\u0027s why I had logged this information in lines 150-211 of the original set inventory patch:\n\nhttps://review.openstack.org/#/c/363583/1/nova/scheduler/client/report.py","commit_id":"506f64a29da26acffb356a9bc85d921657be150b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b5095b8f610e8f8015f1dbc4e3a19788f6902272","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        # Check to see if we need to update placement\u0027s view"},{"line_number":289,"context_line":"        if self._compare_inventory(data[\u0027inventories\u0027],"},{"line_number":290,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":291,"context_line":"            LOG.debug(\u0027No inventory change needed\u0027)"},{"line_number":292,"context_line":"            return True"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_a3cdd486","line":291,"in_reply_to":"9a89bdaa_9a2afd8e","updated":"2016-09-07 15:48:03.000000000","message":"Yeah, this was mostly just for me trying to debug things","commit_id":"506f64a29da26acffb356a9bc85d921657be150b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a72a207689db815654dc577355e046c5bdfc12b8","unresolved":false,"context_lines":[{"line_number":324,"context_line":""},{"line_number":325,"context_line":"        # Update our view of the generation for next time"},{"line_number":326,"context_line":"        updated_inventories \u003d result.json()[\u0027inventories\u0027]"},{"line_number":327,"context_line":"        new_gen \u003d updated_inventories[\u0027VCPU\u0027][\u0027resource_provider_generation\u0027]"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        self._resource_providers[compute_node.uuid].generation \u003d new_gen"},{"line_number":330,"context_line":"        LOG.debug(\u0027Updated inventory for %s at generation %i\u0027 % ("}],"source_content_type":"text/x-python","patch_set":7,"id":"9a89bdaa_ba19818a","line":327,"updated":"2016-09-07 12:55:23.000000000","message":"Please use fields.ResourceClass.VCPU constant instead of hard-coded string above.","commit_id":"506f64a29da26acffb356a9bc85d921657be150b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"d70bfc68d3a48f2190fd1618fff5b9c2e6c590a0","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":295,"context_line":"            return True"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("},{"line_number":298,"context_line":"            self._resource_providers[compute_node.uuid].generation)"},{"line_number":299,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % compute_node.uuid"},{"line_number":300,"context_line":"        result \u003d self.put(url, data)"},{"line_number":301,"context_line":"        if result.status_code \u003d\u003d 409:"},{"line_number":302,"context_line":"            LOG.info(_LI(\u0027Inventory update conflict for %s\u0027),"},{"line_number":303,"context_line":"                     compute_node.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_9c579801","line":300,"range":{"start_line":297,"start_character":8,"end_line":300,"end_character":36},"updated":"2016-09-12 08:33:54.000000000","message":"I doubt why \"resource_provider_generation\" need to be sent to placement service in order to update inventories. It makes no sense to me that an outdated \"generation\" stored in client will fail an inventory update attempt, because compute node should *always* have newer inventories records than placement service.\n\n\nThis makes me revisit the placement-side implementation and the code[1] also makes no sense to me. The generation is already validated during inventory updates in code[2],  and a \"ConcurrentUpdateDetected\" can be raised there. However, code[1] will raise another \"ConcurrentUpdateDetected\" for an external generation value, which violates my understanding that \"generation\" is used to protect placement service from concurrent *inner* updating threads.\n\n\n\n[1] https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/inventory.py#L381-L384\n\n[2] https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L196","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"68dcf342f0a9aa22297c00bd29bdd9a13ff394c4","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":295,"context_line":"            return True"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("},{"line_number":298,"context_line":"            self._resource_providers[compute_node.uuid].generation)"},{"line_number":299,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % compute_node.uuid"},{"line_number":300,"context_line":"        result \u003d self.put(url, data)"},{"line_number":301,"context_line":"        if result.status_code \u003d\u003d 409:"},{"line_number":302,"context_line":"            LOG.info(_LI(\u0027Inventory update conflict for %s\u0027),"},{"line_number":303,"context_line":"                     compute_node.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_124efa6e","line":300,"range":{"start_line":297,"start_character":8,"end_line":300,"end_character":36},"in_reply_to":"9a89bdaa_4fe45ddf","updated":"2016-09-12 14:37:58.000000000","message":"But it _is_ required. Further, we shouldn\u0027t make the compute nodes behave differently from other resource providers just because they happen to (currently) be updating their full set all the time.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"7fc7f1ace0c3b4453ce8e1763c84c739ad5db976","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":295,"context_line":"            return True"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("},{"line_number":298,"context_line":"            self._resource_providers[compute_node.uuid].generation)"},{"line_number":299,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % compute_node.uuid"},{"line_number":300,"context_line":"        result \u003d self.put(url, data)"},{"line_number":301,"context_line":"        if result.status_code \u003d\u003d 409:"},{"line_number":302,"context_line":"            LOG.info(_LI(\u0027Inventory update conflict for %s\u0027),"},{"line_number":303,"context_line":"                     compute_node.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_4fe45ddf","line":300,"range":{"start_line":297,"start_character":8,"end_line":300,"end_character":36},"in_reply_to":"9a89bdaa_827001de","updated":"2016-09-12 14:35:26.000000000","message":"Thanks, I can understand its usage where an external GET-merge-PUT cycle happens.\n\nHowever, as you said, when compute node is \"updating all the inventory for a single resource provider in a single run\",  we don\u0027t need to check the \"generation\". I mean data[\u0027resource_provider_generation\u0027] is not mandatory in all situations.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9f9419d43e89d45034f264fce222d6a4866c175b","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                   curr.get(\u0027inventories\u0027, {})):"},{"line_number":295,"context_line":"            return True"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        data[\u0027resource_provider_generation\u0027] \u003d ("},{"line_number":298,"context_line":"            self._resource_providers[compute_node.uuid].generation)"},{"line_number":299,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % compute_node.uuid"},{"line_number":300,"context_line":"        result \u003d self.put(url, data)"},{"line_number":301,"context_line":"        if result.status_code \u003d\u003d 409:"},{"line_number":302,"context_line":"            LOG.info(_LI(\u0027Inventory update conflict for %s\u0027),"},{"line_number":303,"context_line":"                     compute_node.uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_827001de","line":300,"range":{"start_line":297,"start_character":8,"end_line":300,"end_character":36},"in_reply_to":"9a89bdaa_9c579801","updated":"2016-09-12 13:38:12.000000000","message":"No, the generation field is for external concurrency. Its use is not as clear in this case where we\u0027re updating all the inventory for a single resource provider in a single run, but is more important when we may be doing updates for a given resource provider for multiple scripts where an external GET-merge-PUT cycle happens.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b62e9c51faca241409d2ddabf7c3e9ed0082a0e2","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Invalidate our cache and re-fetch the resource provider"},{"line_number":305,"context_line":"            # to be sure to get the latest generation."},{"line_number":306,"context_line":"            del self._resource_providers[compute_node.uuid]"},{"line_number":307,"context_line":"            self._ensure_resource_provider(compute_node.uuid,"},{"line_number":308,"context_line":"                                           compute_node.hypervisor_hostname)"},{"line_number":309,"context_line":"            return False"},{"line_number":310,"context_line":"        elif not result:"},{"line_number":311,"context_line":"            LOG.warning(_LW(\u0027Failed to update inventory for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_0f81d18f","line":308,"range":{"start_line":307,"start_character":12,"end_line":308,"end_character":76},"updated":"2016-09-09 11:49:18.000000000","message":"If this is just for update the generation, the new generation can be updated at line 290 in the next retry. So we can save one rest API.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4f4ddd1191c67fe550b67ff8cb6304541cef7d5b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Invalidate our cache and re-fetch the resource provider"},{"line_number":305,"context_line":"            # to be sure to get the latest generation."},{"line_number":306,"context_line":"            del self._resource_providers[compute_node.uuid]"},{"line_number":307,"context_line":"            self._ensure_resource_provider(compute_node.uuid,"},{"line_number":308,"context_line":"                                           compute_node.hypervisor_hostname)"},{"line_number":309,"context_line":"            return False"},{"line_number":310,"context_line":"        elif not result:"},{"line_number":311,"context_line":"            LOG.warning(_LW(\u0027Failed to update inventory for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_58be38d5","line":308,"range":{"start_line":307,"start_character":12,"end_line":308,"end_character":76},"in_reply_to":"9a89bdaa_0f81d18f","updated":"2016-09-09 13:43:32.000000000","message":"No, if we get here it\u0027s because the generation on 290 has already changed. The design of the API is that we shouldn\u0027t guess, but should look to see what the current generation is and retry with that.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"d70bfc68d3a48f2190fd1618fff5b9c2e6c590a0","unresolved":false,"context_lines":[{"line_number":338,"context_line":"    def _update_inventory(self, compute_node):"},{"line_number":339,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":340,"context_line":"            # This will be a no-op the first time, but will refetch"},{"line_number":341,"context_line":"            # if our cache is invalidated"},{"line_number":342,"context_line":"            self._ensure_resource_provider(compute_node.uuid,"},{"line_number":343,"context_line":"                                           compute_node.hypervisor_hostname)"},{"line_number":344,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":345,"context_line":"                return True"},{"line_number":346,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_b7bba174","line":343,"range":{"start_line":341,"start_character":14,"end_line":343,"end_character":76},"updated":"2016-09-12 08:33:54.000000000","message":"The only cache invalidation happens @306, but then a _ensure_resource_provider() is immediately called @307.\n\nAnd another call @355 ensures r-p at the first attempt.\n\nSo this refetch appears unnecessary I think, it won\u0027t update any r-ps if they exist in cache.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9f9419d43e89d45034f264fce222d6a4866c175b","unresolved":false,"context_lines":[{"line_number":338,"context_line":"    def _update_inventory(self, compute_node):"},{"line_number":339,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":340,"context_line":"            # This will be a no-op the first time, but will refetch"},{"line_number":341,"context_line":"            # if our cache is invalidated"},{"line_number":342,"context_line":"            self._ensure_resource_provider(compute_node.uuid,"},{"line_number":343,"context_line":"                                           compute_node.hypervisor_hostname)"},{"line_number":344,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":345,"context_line":"                return True"},{"line_number":346,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a89bdaa_62e85d7d","line":343,"range":{"start_line":341,"start_character":14,"end_line":343,"end_character":76},"in_reply_to":"9a89bdaa_b7bba174","updated":"2016-09-12 13:38:12.000000000","message":"You\u0027re right, I meant to clean this up after some other changes.","commit_id":"8806940ae224119d4deb36a2dc787502586eb24d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4597dfd2bb955b99c09e562d4df02faa52933e99","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                         \u0027text\u0027: result.text})"},{"line_number":316,"context_line":"            return False"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"        if result.status_code !\u003d 200:"},{"line_number":319,"context_line":"            LOG.info("},{"line_number":320,"context_line":"                _LI(\u0027Received unexpected response code %(code)i while \u0027"},{"line_number":321,"context_line":"                    \u0027trying to update inventory for compute node %(uuid)s\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_72df463a","line":318,"updated":"2016-09-12 14:44:37.000000000","message":"The response code should be 204 on success:\n\nhttp://specs.openstack.org/openstack/nova-specs/specs/newton/approved/generic-resource-pools.html#put-resource-providers-uuid-inventories","commit_id":"df35cc80b4e6680f757227ee1e764df9b9d2fec4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4597dfd2bb955b99c09e562d4df02faa52933e99","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            return False"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        # Update our view of the generation for next time"},{"line_number":329,"context_line":"        updated_inventories \u003d result.json()[\u0027inventories\u0027]"},{"line_number":330,"context_line":"        new_gen \u003d updated_inventories[VCPU][\u0027resource_provider_generation\u0027]"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        self._resource_providers[compute_node.uuid].generation \u003d new_gen"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_52e402df","line":329,"updated":"2016-09-12 14:44:37.000000000","message":"There is no body returned by the PUT /r-p/{uuid}/inventories call:\n\nhttp://specs.openstack.org/openstack/nova-specs/specs/newton/approved/generic-resource-pools.html#put-resource-providers-uuid-inventories","commit_id":"df35cc80b4e6680f757227ee1e764df9b9d2fec4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a27292dfced2c95233abe0220e533c228ca4b356","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":"    def _compute_node_inventory(self, compute_node):"},{"line_number":216,"context_line":"        inventories \u003d {"},{"line_number":217,"context_line":"            \u0027VCPU\u0027: {"},{"line_number":218,"context_line":"                \u0027total\u0027: compute_node.vcpus,"},{"line_number":219,"context_line":"                \u0027reserved\u0027: 0,"},{"line_number":220,"context_line":"                \u0027min_unit\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_ba0a3877","line":217,"range":{"start_line":217,"start_character":13,"end_line":217,"end_character":17},"updated":"2016-09-13 16:45:42.000000000","message":"nit: you could use your new constants in here.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"240968007f89f2be5e313b6d4258cb0773848731","unresolved":false,"context_lines":[{"line_number":259,"context_line":""},{"line_number":260,"context_line":"        # Snip out the generation from each of the resource class dicts"},{"line_number":261,"context_line":"        trimmed \u003d {}"},{"line_number":262,"context_line":"        for rclass, rdict in remote.items():"},{"line_number":263,"context_line":"            trimmed[rclass] \u003d copy.copy(rdict)"},{"line_number":264,"context_line":"            trimmed[rclass].pop(\u0027resource_provider_generation\u0027)"},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_9081e2c5","line":262,"range":{"start_line":262,"start_character":36,"end_line":262,"end_character":44},"updated":"2016-09-13 14:15:48.000000000","message":"nit: That could be confusing for a Python3 reviewer given it would return an iterator instead of a list of tuples, but okay, that works (fortunatly) too.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"240968007f89f2be5e313b6d4258cb0773848731","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_cdb2f593","line":281,"range":{"start_line":281,"start_character":26,"end_line":281,"end_character":51},"updated":"2016-09-13 14:15:48.000000000","message":"we would get a KeyError if we are unable to reach the placement API by the _get_inventory() call.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f26dee3c97b69af11f5720041ccd4b6ea9dcff79","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_bf0dcae9","line":281,"range":{"start_line":281,"start_character":26,"end_line":281,"end_character":51},"in_reply_to":"9a89bdaa_1f94f688","updated":"2016-09-13 16:18:45.000000000","message":"Because I spent an hour trying to reorder them and gave up. It was designed to be on top of this one, and it would be a lot of work to make it happen for little gain when time is tight.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bad0c005972809758299749e8a5f74a45ecff59e","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_1f94f688","line":281,"range":{"start_line":281,"start_character":26,"end_line":281,"end_character":51},"in_reply_to":"9a89bdaa_5b393ba3","updated":"2016-09-13 16:16:24.000000000","message":"Why isn\u0027t the next patch in the series that fixes this stuff before this change?","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4dd6549188ed2dcd2b73ca2e5ab2f1e8737d80ac","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_5b393ba3","line":281,"range":{"start_line":281,"start_character":26,"end_line":281,"end_character":51},"in_reply_to":"9a89bdaa_cdb2f593","updated":"2016-09-13 14:21:45.000000000","message":"This is removed in the next patch.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"02cee75403e8fb8f2f25fcb40ee39af0c40f1f9a","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_8497dfea","line":282,"range":{"start_line":281,"start_character":12,"end_line":282,"end_character":59},"updated":"2016-09-13 16:11:09.000000000","message":"I\u0027m confused, I thought resource_provider_generation was a sibling key to the inventories key in the results here?\n\nhttp://docs-draft.openstack.org/88/366888/2/check/gate-nova-specs-docs-ubuntu-xenial/bb1c721//doc/build/html/specs/newton/approved/generic-resource-pools.html#get-resource-providers-uuid-inventories","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4a5f26d8503c3bdfc3c2581785e498b845ecdb8","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_ff4d923e","line":282,"range":{"start_line":281,"start_character":12,"end_line":282,"end_character":59},"in_reply_to":"9a89bdaa_8497dfea","updated":"2016-09-13 16:19:03.000000000","message":"So I guess this is fixed in the next patch because we get the inventory for VCPU which the response body will include the generation:\n\nhttp://docs-draft.openstack.org/88/366888/2/check/gate-nova-specs-docs-ubuntu-xenial/bb1c721//doc/build/html/specs/newton/approved/generic-resource-pools.html#get-resource-providers-uuid-inventories-resource-class\n\nBut I\u0027m not sure why this change isn\u0027t coming after Chris\u0027 change.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0a004e330e9adbb770813d83d54d0e737101a0e0","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        # Update our generation immediately, if possible. We always report"},{"line_number":279,"context_line":"        # VCPU inventory, so use that."},{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_5f52fe35","line":282,"range":{"start_line":281,"start_character":12,"end_line":282,"end_character":59},"in_reply_to":"9a89bdaa_ff4d923e","updated":"2016-09-13 16:27:49.000000000","message":"OK I think I get it now, talked it through in IRC. This change is working with the current state of the placement API, which is not written to spec, but cdent\u0027s change after it fixes that, and also fixes this in the process.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"240968007f89f2be5e313b6d4258cb0773848731","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"},{"line_number":286,"context_line":"                          \u0027from %(old)i to %(new)i\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_f03eaeb6","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":63},"updated":"2016-09-13 14:15:48.000000000","message":"there is a possibility that it would also return a KeyError if the resource provider is not there yet.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b50ac5cc57fba8a94f55d48acdaa2ee6a8de3f55","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"},{"line_number":286,"context_line":"                          \u0027from %(old)i to %(new)i\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_f6a06692","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":63},"in_reply_to":"9a89bdaa_bb27a761","updated":"2016-09-13 14:45:32.000000000","message":"Okay, given L352 ensures that a RP is existing, that\u0027s fine by me then. There could be some races tho, but that\u0027s something we could modify later on.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4dd6549188ed2dcd2b73ca2e5ab2f1e8737d80ac","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        if curr.get(\u0027inventories\u0027):"},{"line_number":281,"context_line":"            server_gen \u003d (curr[\u0027inventories\u0027][VCPU]"},{"line_number":282,"context_line":"                          [\u0027resource_provider_generation\u0027])"},{"line_number":283,"context_line":"            my_rp \u003d self._resource_providers[compute_node.uuid]"},{"line_number":284,"context_line":"            if server_gen !\u003d my_rp.generation:"},{"line_number":285,"context_line":"                LOG.debug(\u0027Updating our resource provider generation \u0027"},{"line_number":286,"context_line":"                          \u0027from %(old)i to %(new)i\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_bb27a761","line":283,"range":{"start_line":283,"start_character":20,"end_line":283,"end_character":63},"in_reply_to":"9a89bdaa_f03eaeb6","updated":"2016-09-13 14:21:45.000000000","message":"We can\u0027t get here if _ensure_resource_provider() didn\u0027t run. How would we get a KeyError?","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a27292dfced2c95233abe0220e533c228ca4b356","unresolved":false,"context_lines":[{"line_number":286,"context_line":"                          \u0027from %(old)i to %(new)i\u0027,"},{"line_number":287,"context_line":"                          {\u0027old\u0027: my_rp.generation,"},{"line_number":288,"context_line":"                           \u0027new\u0027: server_gen})"},{"line_number":289,"context_line":"            my_rp.generation \u003d server_gen"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"        # Check to see if we need to update placement\u0027s view"},{"line_number":292,"context_line":"        if self._compare_inventory(data[\u0027inventories\u0027],"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_3a5468c8","line":289,"updated":"2016-09-13 16:45:42.000000000","message":"I guess this is updating self._resource_providers[compute_node.uuid] by reference, I was wondering why we weren\u0027t just using my_rp on line 297 below.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a27292dfced2c95233abe0220e533c228ca4b356","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        new_gen \u003d updated_inventories[VCPU][\u0027resource_provider_generation\u0027]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        self._resource_providers[compute_node.uuid].generation \u003d new_gen"},{"line_number":332,"context_line":"        LOG.debug(\u0027Updated inventory for %s at generation %i\u0027 % ("},{"line_number":333,"context_line":"            compute_node.uuid, new_gen))"},{"line_number":334,"context_line":"        return True"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    @safe_connect"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_fa7e8017","line":333,"range":{"start_line":332,"start_character":62,"end_line":333,"end_character":40},"updated":"2016-09-13 16:45:42.000000000","message":"nit: pass as args to LOG.debug.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"240968007f89f2be5e313b6d4258cb0773848731","unresolved":false,"context_lines":[{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    @safe_connect"},{"line_number":337,"context_line":"    def _update_inventory(self, compute_node):"},{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_edced958","line":338,"updated":"2016-09-13 14:15:48.000000000","message":"nit: should we have a CONF opt for this ? Shouldn\u0027t we rather use the existing periodic call to update_resources_interval() as a way to do more than one attempt ?","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f4ea978102ac3398c79cf09dccbb350f2f481802","unresolved":false,"context_lines":[{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    @safe_connect"},{"line_number":337,"context_line":"    def _update_inventory(self, compute_node):"},{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a8ec9b2_b83689b2","line":338,"in_reply_to":"9a89bdaa_5b913b46","updated":"2016-09-13 19:27:26.000000000","message":"agree with dansmith on this.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4dd6549188ed2dcd2b73ca2e5ab2f1e8737d80ac","unresolved":false,"context_lines":[{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    @safe_connect"},{"line_number":337,"context_line":"    def _update_inventory(self, compute_node):"},{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_5b913b46","line":338,"in_reply_to":"9a89bdaa_edced958","updated":"2016-09-13 14:21:45.000000000","message":"I disagree, I don\u0027t think there\u0027s any reason for an operator to change this. If it doesn\u0027t get updated after three attempts, it\u0027ll just run again the next time.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"240968007f89f2be5e313b6d4258cb0773848731","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"},{"line_number":342,"context_line":"        return False"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def update_resource_stats(self, compute_node):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_6dd80954","line":341,"updated":"2016-09-13 14:15:48.000000000","message":"nit: should we have rather a CONF opt for this ? Also, what if the operator modifies CONF.update_resources_interval down to 1 sec instead of 60 sec ?","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b50ac5cc57fba8a94f55d48acdaa2ee6a8de3f55","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"},{"line_number":342,"context_line":"        return False"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def update_resource_stats(self, compute_node):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_41eb0edc","line":341,"in_reply_to":"9a89bdaa_5b2dbbfc","updated":"2016-09-13 14:45:32.000000000","message":"Okay, I guess I was concerned by the periodic call shifting due to some long runs, but that\u0027s really something not really a problem.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4dd6549188ed2dcd2b73ca2e5ab2f1e8737d80ac","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        for attempt in (1, 2, 3):"},{"line_number":339,"context_line":"            if self._update_inventory_attempt(compute_node):"},{"line_number":340,"context_line":"                return True"},{"line_number":341,"context_line":"            time.sleep(1)"},{"line_number":342,"context_line":"        return False"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def update_resource_stats(self, compute_node):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_5b2dbbfc","line":341,"in_reply_to":"9a89bdaa_6dd80954","updated":"2016-09-13 14:21:45.000000000","message":"I don\u0027t think there\u0027s any reason for them to tweak this either. This is purely throttling so we don\u0027t hit the API too hard. After a 409 we expect the next attempt to work, so there\u0027s no reason to wait longer.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"}],"nova/tests/unit/scheduler/client/test_report.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a27292dfced2c95233abe0220e533c228ca4b356","unresolved":false,"context_lines":[{"line_number":601,"context_line":"                                      mock.call(cn)])"},{"line_number":602,"context_line":""},{"line_number":603,"context_line":"        # Slept three times"},{"line_number":604,"context_line":"        mock_sleep.assert_has_calls([mock.call(1), mock.call(1), mock.call(1)])"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"    @mock.patch(\u0027nova.scheduler.client.report.SchedulerReportClient.\u0027"},{"line_number":607,"context_line":"                \u0027_ensure_resource_provider\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_9a365401","line":604,"updated":"2016-09-13 16:45:42.000000000","message":"You already checked this above.","commit_id":"d5e03deb01cb8ad477111de2ffb7b7ec68e7aaf9"}]}