)]}'
{"nova/compute/resource_tracker.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":588,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":589,"context_line":"    def _update_available_resource(self, context, resources):"},{"line_number":590,"context_line":"        # We short-circuit entirely if the virt driver says the node isn\u0027t"},{"line_number":591,"context_line":"        # available. This is slightly different that the disabled() method,"},{"line_number":592,"context_line":"        # which also takes into account whether the compute node has been"},{"line_number":593,"context_line":"        # created and stored in the cache of compute node objects after"},{"line_number":594,"context_line":"        # _init_compute_node() is called."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_e266aeb9","line":591,"range":{"start_line":591,"start_character":48,"end_line":591,"end_character":52},"updated":"2017-03-06 01:15:27.000000000","message":"than","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":593,"context_line":"        # created and stored in the cache of compute node objects after"},{"line_number":594,"context_line":"        # _init_compute_node() is called."},{"line_number":595,"context_line":"        nodename \u003d resources[\u0027hypervisor_hostname\u0027]"},{"line_number":596,"context_line":"        if not self.driver.node_is_available(nodename):"},{"line_number":597,"context_line":"            return"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"        # initialize the compute node object, creating it"},{"line_number":600,"context_line":"        # if it does not already exist."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_c2028ab3","line":597,"range":{"start_line":596,"start_character":8,"end_line":597,"end_character":18},"updated":"2017-03-06 01:15:27.000000000","message":"This wasn\u0027t mentioned in the commit message. What\u0027s driving the need for this in this change?","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":593,"context_line":"        # created and stored in the cache of compute node objects after"},{"line_number":594,"context_line":"        # _init_compute_node() is called."},{"line_number":595,"context_line":"        nodename \u003d resources[\u0027hypervisor_hostname\u0027]"},{"line_number":596,"context_line":"        if not self.driver.node_is_available(nodename):"},{"line_number":597,"context_line":"            return"},{"line_number":598,"context_line":""},{"line_number":599,"context_line":"        # initialize the compute node object, creating it"},{"line_number":600,"context_line":"        # if it does not already exist."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_f1b9fa7a","line":597,"range":{"start_line":596,"start_character":8,"end_line":597,"end_character":18},"in_reply_to":"ba2be162_c2028ab3","updated":"2017-03-06 18:30:14.000000000","message":"Yeah, sorry, it\u0027s confusing. This patch changes _init_compute_node() to call _update() (which is where we call the new get_inventory() method). Because of this, I was getting the following failure in a unit test:\n\nnova.tests.unit.compute.test_resource_tracker.TestUpdateAvailableResources.test_disabled\n----------------------------------------------------------------------------------------\n\n Captured traceback:\n ~~~~~~~~~~~~~~~~~~~\n    Traceback (most recent call last):\n      File \"/home/jaypipes/src/git.openstack.org/openstack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 1305, in patched\n        return func(*args, **keywargs)\n      File \"nova/tests/unit/compute/test_resource_tracker.py\", line 504, in test_disabled\n        self.assertFalse(update_mock.called)\n      File \"/home/jaypipes/src/git.openstack.org/openstack/nova/.tox/py27/local/lib/python2.7/site-packages/unittest2/case.py\", line 696, in assertFalse\n        raise self.failureException(msg)\n    AssertionError: True is not false\n\nIt turns out that the test_disabled() test method was verifying that when calling update_available_resource() we didn\u0027t call _update() if the compute node was disabled or didn\u0027t exist in the RT\u0027s _compute_nodes dict.\n\nI\u0027ve removed the above change and instead modified the test_disabled() test method to check that InstanceList.get_all_by_host_and_node() is not called and not _update(). This seems a better assertion than the previous one, since what we want to ensure is *not* that the inventory for a compute node isn\u0027t accounted for when a node is disabled, but that the allocation/usage of a node isn\u0027t considered...\n\nWe should probably get rid of the whole notion of disabled node state entirely in the future and leave that to a servicegroup API or similar. The tracking of resources really shouldn\u0027t be affected by whether a node is disabled or not...","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"            )"},{"line_number":760,"context_line":"        except NotImplementedError:"},{"line_number":761,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        if self.pci_tracker:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_82d17225","line":760,"updated":"2017-03-06 01:15:27.000000000","message":"nit: a comment about what this means might be nice. Will all virt drivers be expected to implement get_inventory at some point? I assume so.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"            )"},{"line_number":760,"context_line":"        except NotImplementedError:"},{"line_number":761,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"},{"line_number":762,"context_line":""},{"line_number":763,"context_line":"        if self.pci_tracker:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_31ac7235","line":760,"in_reply_to":"ba2be162_82d17225","updated":"2017-03-06 18:30:14.000000000","message":"Done","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"}],"nova/exception.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"935793b9b6b589e8329a1a6401f2147e74ae2b69","unresolved":false,"context_lines":[{"line_number":2054,"context_line":"    msg_fmt \u003d _(\"No inventory of class %(resource_class)s found.\")"},{"line_number":2055,"context_line":""},{"line_number":2056,"context_line":""},{"line_number":2057,"context_line":"class InvalidResourceClass(Invalid):"},{"line_number":2058,"context_line":"    msg_fmt \u003d _(\"Resource class \u0027%(resource_class)s\u0027 invalid.\")"},{"line_number":2059,"context_line":""},{"line_number":2060,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_e6577bc8","line":2057,"updated":"2017-03-07 03:12:40.000000000","message":"oh it\u0027s a good thing I asked for that test. :)","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"935793b9b6b589e8329a1a6401f2147e74ae2b69","unresolved":false,"context_lines":[{"line_number":2055,"context_line":""},{"line_number":2056,"context_line":""},{"line_number":2057,"context_line":"class InvalidResourceClass(Invalid):"},{"line_number":2058,"context_line":"    msg_fmt \u003d _(\"Resource class \u0027%(resource_class)s\u0027 invalid.\")"},{"line_number":2059,"context_line":""},{"line_number":2060,"context_line":""},{"line_number":2061,"context_line":"class ResourceClassExists(NovaException):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_264b532d","line":2058,"range":{"start_line":2058,"start_character":53,"end_line":2058,"end_character":60},"updated":"2017-03-07 03:12:40.000000000","message":"nit: is invalid?","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3c2908e6a3dc9f0724e5c4e0a1036a01e7c66589","unresolved":false,"context_lines":[{"line_number":553,"context_line":"            # record already exists. We\u0027re just reloading the record here."},{"line_number":554,"context_line":"            self._ensure_resource_provider(rp_uuid)"},{"line_number":555,"context_line":"            return False"},{"line_number":556,"context_line":"        elif not result:"},{"line_number":557,"context_line":"            placement_req_id \u003d get_placement_request_id(result)"},{"line_number":558,"context_line":"            LOG.warning(_LW(\u0027[%(placement_req_id)s] Failed to update \u0027"},{"line_number":559,"context_line":"                            \u0027inventory for resource provider \u0027"},{"line_number":560,"context_line":"                            \u0027%(uuid)s: %(status)i %(text)s\u0027),"},{"line_number":561,"context_line":"                        {\u0027placement_req_id\u0027: placement_req_id,"},{"line_number":562,"context_line":"                         \u0027uuid\u0027: rp_uuid,"},{"line_number":563,"context_line":"                         \u0027status\u0027: result.status_code,"},{"line_number":564,"context_line":"                         \u0027text\u0027: result.text})"},{"line_number":565,"context_line":"            # log the body at debug level"},{"line_number":566,"context_line":"            LOG.debug(\u0027[%(placement_req_id)s] Failed inventory update request \u0027"},{"line_number":567,"context_line":"                      \u0027for resource provider %(uuid)s with body: %(payload)s\u0027,"},{"line_number":568,"context_line":"                      {\u0027placement_req_id\u0027: placement_req_id,"},{"line_number":569,"context_line":"                       \u0027uuid\u0027: rp_uuid,"},{"line_number":570,"context_line":"                       \u0027payload\u0027: payload})"},{"line_number":571,"context_line":"            return False"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"        if result.status_code !\u003d 200:"},{"line_number":574,"context_line":"            placement_req_id \u003d get_placement_request_id(result)"},{"line_number":575,"context_line":"            LOG.info("},{"line_number":576,"context_line":"                _LI(\u0027[%(placement_req_id)s] Received unexpected response code \u0027"},{"line_number":577,"context_line":"                    \u0027%(code)i while trying to update inventory for resource \u0027"},{"line_number":578,"context_line":"                    \u0027provider %(uuid)s: %(text)s\u0027),"},{"line_number":579,"context_line":"                {\u0027placement_req_id\u0027: placement_req_id,"},{"line_number":580,"context_line":"                 \u0027uuid\u0027: rp_uuid,"},{"line_number":581,"context_line":"                 \u0027code\u0027: result.status_code,"},{"line_number":582,"context_line":"                 \u0027text\u0027: result.text})"},{"line_number":583,"context_line":"            return False"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"        # Update our view of the generation for next time"},{"line_number":586,"context_line":"        updated_inventories_result \u003d result.json()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_86b6bf3b","line":583,"range":{"start_line":556,"start_character":0,"end_line":583,"end_character":24},"updated":"2017-03-07 03:07:16.000000000","message":"If the virt driver passes garbage for the resource class name, i.e. \"foo\" discussed below, then we\u0027ll end up failing the PUT call with a 400 and logging it here somewhere.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e5e1d48ff4939a215583872b8187a82ad89ece6e","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_ec0231b3","line":692,"updated":"2017-03-04 16:08:48.000000000","message":"If the client does return a garbage resource class like \"foo\" we\u0027ll try to create it here, right? i think we probably want to check that the class we\u0027re about to create is properly namespaced under CUSTOM right?","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7701f3e2992849ff3d7f32ee2d5ffb8f9fe67eed","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_e6b3fb08","line":692,"in_reply_to":"9a30ddce_2672737d","updated":"2017-03-07 03:15:24.000000000","message":"Actually I just realized, after looking at the diff between PS1 and PS2, that _get_or_create_resource_class will check to see if the resource class exists and get a 404, so then it will call _create_resource_class and that will get the 400 and then _create_resource_class will raise InvalidResourceClass. This wasn\u0027t clear to me before because there was no test for that scenario. Actually once Jay added the test it became clear that InvalidResourceClass wasn\u0027t even defined...\n\nAnyway, my concern over this is gone now since it wasn\u0027t going to do what I thought it was.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3c2908e6a3dc9f0724e5c4e0a1036a01e7c66589","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_661d4b66","line":692,"in_reply_to":"9a30ddce_56e71a40","updated":"2017-03-07 03:07:16.000000000","message":"_get_or_create_resource_class will log an error on L727 below when we get a 400 back from the placement API. Now that you mention it though, it does seem like we should filter out any busted resource class names from new_inv, i.e. if _get_or_create_resource_class(rc_name) is None.\n\nThe reason I mention the filtering is with how _update_inventory_attempt works, it\u0027s an all-or-nothing shot and if one of the resource classes are wrong, then we don\u0027t update anything, even ones that might be legitimate.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bfd0b111eb180955712cb08a09cdefa126dce922","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_2672737d","line":692,"in_reply_to":"9a30ddce_661d4b66","updated":"2017-03-07 03:10:13.000000000","message":"Exactly my point. I\u0027d expect us to check what the virt driver returns and log the errors there if needed. Since I don\u0027t think this is actively broken, I\u0027m still +2 on the latest PS, but I do think we could benefit from more care here in a follow-up.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8c61fc48365da0e41a6ac67f1b142ad3b6a83658","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a30ddce_56e71a40","line":692,"in_reply_to":"ba2be162_7c5ec113","updated":"2017-03-07 00:16:45.000000000","message":"Right but you\u0027re not barfing errors here. Maybe you mean placement will do the barfing for us?","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":689,"context_line":""},{"line_number":690,"context_line":"        new_inv \u003d {}"},{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_7c5ec113","line":692,"in_reply_to":"ba2be162_ec0231b3","updated":"2017-03-06 18:30:14.000000000","message":"Yep, we will try to create it. I believe we agreed that we will just barf back errors about bad resource class names and rely on the drivers to ask for proper resource names.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":727,"context_line":"            LOG.error(msg, args)"},{"line_number":728,"context_line":"            return None"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"    def _create_resource_class(self, name):"},{"line_number":731,"context_line":"        \"\"\"Calls the placement API to create a new resource class."},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"        :param name: String name of the resource class to create."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_22952632","line":730,"range":{"start_line":730,"start_character":8,"end_line":730,"end_character":30},"updated":"2017-03-06 01:15:27.000000000","message":"Doesn\u0027t this need @safe_connect on it since it\u0027s calling post?","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":727,"context_line":"            LOG.error(msg, args)"},{"line_number":728,"context_line":"            return None"},{"line_number":729,"context_line":""},{"line_number":730,"context_line":"    def _create_resource_class(self, name):"},{"line_number":731,"context_line":"        \"\"\"Calls the placement API to create a new resource class."},{"line_number":732,"context_line":""},{"line_number":733,"context_line":"        :param name: String name of the resource class to create."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_3c9289a0","line":730,"range":{"start_line":730,"start_character":8,"end_line":730,"end_character":30},"in_reply_to":"ba2be162_22952632","updated":"2017-03-06 18:30:14.000000000","message":"It\u0027s always wrapped by _get_or_create_resource_class(), which is decorated with @safe_connect.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":733,"context_line":"        :param name: String name of the resource class to create."},{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        :returns: None on successful creation."},{"line_number":736,"context_line":"        :raises: `exception.ResourceClassInvalid` upon error."},{"line_number":737,"context_line":"        \"\"\""},{"line_number":738,"context_line":"        url \u003d \"/resource_classes\""},{"line_number":739,"context_line":"        payload \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_a27af621","line":736,"range":{"start_line":736,"start_character":28,"end_line":736,"end_character":48},"updated":"2017-03-06 01:15:27.000000000","message":"InvalidResourceClass","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":733,"context_line":"        :param name: String name of the resource class to create."},{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        :returns: None on successful creation."},{"line_number":736,"context_line":"        :raises: `exception.ResourceClassInvalid` upon error."},{"line_number":737,"context_line":"        \"\"\""},{"line_number":738,"context_line":"        url \u003d \"/resource_classes\""},{"line_number":739,"context_line":"        payload \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_fcf631f8","line":736,"range":{"start_line":736,"start_character":28,"end_line":736,"end_character":48},"in_reply_to":"ba2be162_a27af621","updated":"2017-03-06 18:30:14.000000000","message":"Done","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":759,"context_line":"                \u0027err_text\u0027: resp.text,"},{"line_number":760,"context_line":"            }"},{"line_number":761,"context_line":"            LOG.error(msg, args)"},{"line_number":762,"context_line":"            raise exception.InvalidResourceClass(resource_class\u003dname)"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":765,"context_line":"        \"\"\"Creates or updates stats for the supplied compute node."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_0215829a","line":762,"updated":"2017-03-06 01:15:27.000000000","message":"Don\u0027t see a test for this case.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":759,"context_line":"                \u0027err_text\u0027: resp.text,"},{"line_number":760,"context_line":"            }"},{"line_number":761,"context_line":"            LOG.error(msg, args)"},{"line_number":762,"context_line":"            raise exception.InvalidResourceClass(resource_class\u003dname)"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":765,"context_line":"        \"\"\"Creates or updates stats for the supplied compute node."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_dcc88dab","line":762,"in_reply_to":"ba2be162_0215829a","updated":"2017-03-06 18:30:14.000000000","message":"Done","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76b7a41803fbc73da99ffe78a2d22ae2196a9bb0","unresolved":false,"context_lines":[{"line_number":691,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":692,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD:"},{"line_number":693,"context_line":"                # Auto-create custom resource classes coming from a virt driver"},{"line_number":694,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"            new_inv[rc_name] \u003d inv"},{"line_number":697,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_86aa3f6f","line":694,"updated":"2017-03-07 03:25:29.000000000","message":"As noted in PS1, we might want to handle InvalidResourceClass or None and filter those out since we don\u0027t fail to update any inventory, but that could also be a follow up change if it becomes an issue.","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"}],"nova/tests/functional/api/openstack/placement/test_report_client.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            self.assertIsNone(rp)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"            # Now let\u0027s update status for our compute node."},{"line_number":105,"context_line":"            self.client.update_compute_node(self.compute_node)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            # So now we have a resource provider"},{"line_number":108,"context_line":"            rp \u003d self.client._get_resource_provider(self.compute_uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_02e022cc","line":105,"updated":"2017-03-06 01:15:27.000000000","message":"I\u0027d think at some point we\u0027re going to want to change the FakeVirtDriver we use in functional tests over to implementing get_inventory() so we use the new hotness in the functional tests.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ce2d32ed6a2095e8a16f11619333f43462353cca","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            self.assertIsNone(rp)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"            # Now let\u0027s update status for our compute node."},{"line_number":105,"context_line":"            self.client.update_compute_node(self.compute_node)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            # So now we have a resource provider"},{"line_number":108,"context_line":"            rp \u003d self.client._get_resource_provider(self.compute_uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_a299f2d2","line":105,"in_reply_to":"ba2be162_02e022cc","updated":"2017-03-06 18:30:14.000000000","message":"agreed.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"}],"nova/tests/unit/compute/test_resource_tracker.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5ef4e138857afab38db7016950fb4acad19a9c3b","unresolved":false,"context_lines":[{"line_number":420,"context_line":"    \"\"\""},{"line_number":421,"context_line":"    sched_client_mock \u003d mock.MagicMock()"},{"line_number":422,"context_line":"    notifier_mock \u003d mock.MagicMock()"},{"line_number":423,"context_line":"    vd \u003d mock.MagicMock()"},{"line_number":424,"context_line":"    # Make sure we don\u0027t change any global fixtures during tests"},{"line_number":425,"context_line":"    virt_resources \u003d copy.deepcopy(virt_resources)"},{"line_number":426,"context_line":"    vd.get_available_resource.return_value \u003d virt_resources"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_e2eceebc","line":423,"range":{"start_line":423,"start_character":14,"end_line":423,"end_character":23},"updated":"2017-03-06 01:15:27.000000000","message":"ew, we should do autospec\u003d\u0027nova.virt.driver.ComputeDriver\u0027 at some point.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76b7a41803fbc73da99ffe78a2d22ae2196a9bb0","unresolved":false,"context_lines":[{"line_number":420,"context_line":"    \"\"\""},{"line_number":421,"context_line":"    sched_client_mock \u003d mock.MagicMock()"},{"line_number":422,"context_line":"    notifier_mock \u003d mock.MagicMock()"},{"line_number":423,"context_line":"    vd \u003d mock.MagicMock()"},{"line_number":424,"context_line":"    # Make sure we don\u0027t change any global fixtures during tests"},{"line_number":425,"context_line":"    virt_resources \u003d copy.deepcopy(virt_resources)"},{"line_number":426,"context_line":"    vd.get_available_resource.return_value \u003d virt_resources"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_a62ae3e5","line":423,"range":{"start_line":423,"start_character":4,"end_line":423,"end_character":25},"updated":"2017-03-07 03:25:29.000000000","message":"nit: again, we should autospec this, but could be a separate change.","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"}],"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":"76b7a41803fbc73da99ffe78a2d22ae2196a9bb0","unresolved":false,"context_lines":[{"line_number":1238,"context_line":"        \"\"\"Tests that inventory record containing a badly-formatted resource"},{"line_number":1239,"context_line":"        class results in raising an appropriate exception."},{"line_number":1240,"context_line":"        \"\"\""},{"line_number":1241,"context_line":"        exc \u003d exception.InvalidResourceClass(resource_class\u003d\u0027BAD_FOO\u0027)"},{"line_number":1242,"context_line":"        mock_gocr.side_effect \u003d exc"},{"line_number":1243,"context_line":"        inv_data \u003d {"},{"line_number":1244,"context_line":"            \u0027BAD_FOO\u0027: {"},{"line_number":1245,"context_line":"                \u0027total\u0027: 100,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_66770b03","line":1242,"range":{"start_line":1241,"start_character":8,"end_line":1242,"end_character":35},"updated":"2017-03-07 03:25:29.000000000","message":"I\u0027ll officially say I\u0027m a bit concerned about the level of reliance on mock for testing the negative scenario here. This mocks out _get_or_create_resource_class to raise the exception that _create_resource_class would raise, but at least _create_resource_class is tested below. I just worry about not having a solid functional test to hit this entire flow with the fake virt driver. We could build onto this change with some functional testing with a fake virt driver, or a subclass of the fake virt driver that also has a bad resource class name to test the negative scenario.","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1271059525aa820590916a6934c69e7382719941","unresolved":false,"context_lines":[{"line_number":1238,"context_line":"        \"\"\"Tests that inventory record containing a badly-formatted resource"},{"line_number":1239,"context_line":"        class results in raising an appropriate exception."},{"line_number":1240,"context_line":"        \"\"\""},{"line_number":1241,"context_line":"        exc \u003d exception.InvalidResourceClass(resource_class\u003d\u0027BAD_FOO\u0027)"},{"line_number":1242,"context_line":"        mock_gocr.side_effect \u003d exc"},{"line_number":1243,"context_line":"        inv_data \u003d {"},{"line_number":1244,"context_line":"            \u0027BAD_FOO\u0027: {"},{"line_number":1245,"context_line":"                \u0027total\u0027: 100,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a30ddce_29ca4450","line":1242,"range":{"start_line":1241,"start_character":8,"end_line":1242,"end_character":35},"in_reply_to":"9a30ddce_66770b03","updated":"2017-03-07 03:52:26.000000000","message":"This was much easier than I thought:\n\nhttps://review.openstack.org/#/c/442254/","commit_id":"fbc5a67de96396e7270705f2a97c454d9a9b653c"}],"nova/virt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7d8017501b2b219c9e7f966b4b9085c36fd59ac9","unresolved":false,"context_lines":[{"line_number":774,"context_line":"        \"\"\""},{"line_number":775,"context_line":"        raise NotImplementedError()"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    def get_inventory(self, nodename):"},{"line_number":778,"context_line":"        \"\"\"Return a dict, keyed by resource class, of inventory information for"},{"line_number":779,"context_line":"        the supplied node."},{"line_number":780,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba2be162_02666229","line":777,"updated":"2017-03-06 01:23:01.000000000","message":"We typically have a test in nova.tests.unit.virt.test_virt_drivers for every method in ComputeDriver, but I\u0027m not sure how valuable they are since you\u0027d have to handle NotImplementedError and not fail the test. They were originally meant as a way to fill in a support matrix but that never happened and probably won\u0027t happen, so I\u0027ll leave it up to you if you see the need to add a test there.","commit_id":"352dd79248fa71da389340def6e42c88621c39c9"}]}