)]}'
{"nova/compute/resource_tracker.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae5baf72e0e5f335d130b09f75259ebbca95b4dc","unresolved":false,"context_lines":[{"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:"},{"line_number":764,"context_line":"            self.pci_tracker.save(context)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_e21f6a59","line":761,"updated":"2017-02-28 15:23:12.000000000","message":"I wonder if we should also catch exception here, log obviously, and then bail cleanly out of here? I haven\u0027t gone to trace where all this is coming from, but if we had one bad ironic node we wouldn\u0027t want to hold up resource reporting for all the other ones. Maybe?","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30e3639ea061d68e8b9b920de1ff2db9eff57a7e","unresolved":false,"context_lines":[{"line_number":756,"context_line":"                compute_node.uuid,"},{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"                compute_node.hypervisor_type \u003d\u003d \u0027ironic\u0027,"},{"line_number":760,"context_line":"            )"},{"line_number":761,"context_line":"        except NotImplementedError:"},{"line_number":762,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_0d02710b","line":759,"updated":"2017-03-02 18:36:43.000000000","message":"I know you don\u0027t want to auto-create if we\u0027re not on ironic, but:\n\n1. I hate the special case here\n\n2. Lets say libvirt is going to expose a CUSTOM_GPU\u003d10 resource. The operator has to pre-create that instead of libvirt just exposing it. I\u0027m not really sure why that\u0027s better. If we had out of tree drivers then we might want to be super careful here, but .. I dunno.. doesn\u0027t seem worth it.\n\nThis particular hack has no sunset period right? Meaning, we\u0027re always going to need this?","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0b41dff8a66892fa511fc464dace82a9e56b1b34","unresolved":false,"context_lines":[{"line_number":756,"context_line":"                compute_node.uuid,"},{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"                compute_node.hypervisor_type \u003d\u003d \u0027ironic\u0027,"},{"line_number":760,"context_line":"            )"},{"line_number":761,"context_line":"        except NotImplementedError:"},{"line_number":762,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_6d7be3f5","line":759,"in_reply_to":"ba2be162_0d02710b","updated":"2017-03-02 22:36:42.000000000","message":"I\u0027m happy to auto-create the world if that\u0027s what you want, but I was under the impression you wanted to prevent drivers from accidentally passing in things like MEMORY_GB and having that auto-created by mistake?","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71d339951149d90a6af7980d4e462387785c17cb","unresolved":false,"context_lines":[{"line_number":756,"context_line":"                compute_node.uuid,"},{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"                compute_node.hypervisor_type \u003d\u003d \u0027ironic\u0027,"},{"line_number":760,"context_line":"            )"},{"line_number":761,"context_line":"        except NotImplementedError:"},{"line_number":762,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_ad395bf4","line":759,"in_reply_to":"ba2be162_6d7be3f5","updated":"2017-03-02 22:42:20.000000000","message":"I definitely am against us trying to create MEMORY_GB. But if they return a CUSTOM_FOO we should create that no? What I was trying to say when we talked at length was that we should expect drivers to return values that fit our schema. That means either:\n\n a. Valid standard resource classes, or\n b. Validly-formatted CUSTOM_FOO names that we can create","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb6f2b23d5ae9282eb3f709b4baac56c31c7b9f1","unresolved":false,"context_lines":[{"line_number":756,"context_line":"                compute_node.uuid,"},{"line_number":757,"context_line":"                compute_node.hypervisor_hostname,"},{"line_number":758,"context_line":"                inv_data,"},{"line_number":759,"context_line":"                compute_node.hypervisor_type \u003d\u003d \u0027ironic\u0027,"},{"line_number":760,"context_line":"            )"},{"line_number":761,"context_line":"        except NotImplementedError:"},{"line_number":762,"context_line":"            self.scheduler_client.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_70d2d65d","line":759,"in_reply_to":"ba2be162_ad395bf4","updated":"2017-03-02 22:47:15.000000000","message":"got it. k, will change in next patch.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"}],"nova/objects/fields.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8677d42365338aa827e113b6e959f724fecb0681","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        if rc_name is None:"},{"line_number":476,"context_line":"            return None"},{"line_number":477,"context_line":"        norm_name \u003d rc_name.upper()"},{"line_number":478,"context_line":"        cust_prefix \u003d nova.objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":479,"context_line":"        norm_name \u003d cust_prefix + norm_name"},{"line_number":480,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":481,"context_line":"        norm_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, norm_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_b22700c9","line":478,"updated":"2017-03-02 18:20:10.000000000","message":"I hadn\u0027t remembered that we have both the objects and fields version of ResourceClass. Here does seem better.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"}],"nova/scheduler/client/__init__.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae5baf72e0e5f335d130b09f75259ebbca95b4dc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            rp_uuid,"},{"line_number":62,"context_line":"            rp_name,"},{"line_number":63,"context_line":"            inv_data,"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":67,"context_line":"        self.reportclient.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_c29fc6b2","line":64,"updated":"2017-02-28 15:23:12.000000000","message":"What\u0027s up with all these ugly trailing naked parens in this patch? :)","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"eebcad319c4f7ce2d43706765765de2167e8e669","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            rp_uuid,"},{"line_number":62,"context_line":"            rp_name,"},{"line_number":63,"context_line":"            inv_data,"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":67,"context_line":"        self.reportclient.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_65f1c1cd","line":64,"in_reply_to":"ba2be162_055c8524","updated":"2017-02-28 16:30:49.000000000","message":"\"lots\" is any list of parameters which exceeds a single line call. :)","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e122369171de60541a44a347ac40f791dac8facc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            rp_uuid,"},{"line_number":62,"context_line":"            rp_name,"},{"line_number":63,"context_line":"            inv_data,"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":67,"context_line":"        self.reportclient.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_055c8524","line":64,"in_reply_to":"ba2be162_32254dc2","updated":"2017-02-28 16:23:07.000000000","message":"Three is lots?\n\nWell, whatever, it\u0027s wrong and I hate it, but.. oh well :)","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"72fa0f2b1ce17b71c11a23c641098e6f0ea850c0","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            rp_uuid,"},{"line_number":62,"context_line":"            rp_name,"},{"line_number":63,"context_line":"            inv_data,"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":67,"context_line":"        self.reportclient.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_e808282b","line":64,"in_reply_to":"ba2be162_65f1c1cd","updated":"2017-02-28 16:44:45.000000000","message":"Jay is partial to C formatting.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0169129af97735685b2f5788668ae22d3de80e6b","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            rp_uuid,"},{"line_number":62,"context_line":"            rp_name,"},{"line_number":63,"context_line":"            inv_data,"},{"line_number":64,"context_line":"        )"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update_compute_node(self, compute_node):"},{"line_number":67,"context_line":"        self.reportclient.update_compute_node(compute_node)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_32254dc2","line":64,"in_reply_to":"ba2be162_c29fc6b2","updated":"2017-02-28 16:20:33.000000000","message":"that\u0027s just how I like to structure method/function calls with lots of parameters. It means if in future we add params, that there is only a single line of code changed instead of \u003e1 because the paren is on the line with the last param...","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"2c68ba77fc1350f8124d15375cdd88e5a5dc8e86","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.queryclient.delete_aggregate(context, aggregate)"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data,"},{"line_number":60,"context_line":"            is_ironic\u003dFalse):"},{"line_number":61,"context_line":"        self.reportclient.set_inventory_for_provider("},{"line_number":62,"context_line":"            rp_uuid,"},{"line_number":63,"context_line":"            rp_name,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba2be162_c420a1fa","line":60,"range":{"start_line":60,"start_character":12,"end_line":60,"end_character":21},"updated":"2017-03-01 00:01:09.000000000","message":"A little too snowflakey. How about \"normalize_name\" instead? This way when we run into another RP that we have to treat special for naming, we don\u0027t have to add \u0027is_snowflake2\u0027, \u0027is_snowflake3\u0027, etc.","commit_id":"3d5f4ab0f45d250cf002a5b5251c7c260c0da4b0"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c6ac56c7096dd8f44f94d05a90e0ee8a4cada16d","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                resource classes that would be deleted by an update to the"},{"line_number":674,"context_line":"                placement API."},{"line_number":675,"context_line":"        \"\"\""},{"line_number":676,"context_line":"        compute_node.save()"},{"line_number":677,"context_line":"        self._ensure_resource_provider(compute_node.uuid,"},{"line_number":678,"context_line":"                                       compute_node.hypervisor_hostname)"},{"line_number":679,"context_line":"        inv_data \u003d _compute_node_to_inventory_dict(compute_node)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_8627f6c8","side":"PARENT","line":676,"updated":"2017-02-23 20:37:44.000000000","message":"\\o/","commit_id":"221bc57dee073e4c9d48611ff6eb0ae25e2a9b7d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"103fa6cb9504cafd42597a497b9c27b8473b6c25","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        return None"},{"line_number":144,"context_line":"    rc_name \u003d rc_name.upper()"},{"line_number":145,"context_line":"    cust_prefix \u003d objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":146,"context_line":"    if not rc_name.startswith(cust_prefix):"},{"line_number":147,"context_line":"        rc_name \u003d cust_prefix + rc_name"},{"line_number":148,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":149,"context_line":"        rc_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, rc_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_2d4a9026","line":146,"updated":"2017-02-24 13:59:20.000000000","message":"Isn\u0027t this just encouraging drivers to return non-conforming resource names? I think this means if I return \"foo\" this turns it into CUSTOM_FOO right?\n\nWhy not just check that the driver returns sane things and fail if not?","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a2fcd833407f5b5e48fe6de9ec7d62b60fafd46d","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        return None"},{"line_number":144,"context_line":"    rc_name \u003d rc_name.upper()"},{"line_number":145,"context_line":"    cust_prefix \u003d objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":146,"context_line":"    if not rc_name.startswith(cust_prefix):"},{"line_number":147,"context_line":"        rc_name \u003d cust_prefix + rc_name"},{"line_number":148,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":149,"context_line":"        rc_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, rc_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_f8482fce","line":146,"in_reply_to":"da36d5c6_0deb0ce0","updated":"2017-02-24 18:00:30.000000000","message":"Okay, I just think that it should be an error for a virt driver to return an invalid class. If you return a standard one, then cool, and if you return a properly-formatted custom one, then cool. Anything else should be \"I refuse to pollute the placement api with your garbage\".","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"806f7a92610d9c917aca7c935d2639edc105e59e","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        return None"},{"line_number":144,"context_line":"    rc_name \u003d rc_name.upper()"},{"line_number":145,"context_line":"    cust_prefix \u003d objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":146,"context_line":"    if not rc_name.startswith(cust_prefix):"},{"line_number":147,"context_line":"        rc_name \u003d cust_prefix + rc_name"},{"line_number":148,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":149,"context_line":"        rc_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, rc_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_0deb0ce0","line":146,"in_reply_to":"da36d5c6_2d4a9026","updated":"2017-02-24 14:03:43.000000000","message":"Partly this was done because there\u0027s no requirement in Ironic to make the Ironic node\u0027s resource_class attribute follow the Nova custom resource class naming pattern. We figured it was more user-friendly to just normalize to the placement API\u0027s requirements here, inline.","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"103fa6cb9504cafd42597a497b9c27b8473b6c25","unresolved":false,"context_lines":[{"line_number":693,"context_line":"            norm_name \u003d _normalize_resource_class_name(rc_name)"},{"line_number":694,"context_line":"            if norm_name !\u003d rc_name:"},{"line_number":695,"context_line":"                inv_data[norm_name] \u003d inv_data[rc_name]"},{"line_number":696,"context_line":"                inv_data.pop(rc_name)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        if inv_data:"},{"line_number":699,"context_line":"            self._update_inventory(rp_uuid, inv_data)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_0d76ac6f","line":696,"updated":"2017-02-24 13:59:20.000000000","message":"Doesn\u0027t this try to create the default resource classes as well? Also, how does this not turn standard things like VCPUS into CUSTOM_VCPUS?","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"806f7a92610d9c917aca7c935d2639edc105e59e","unresolved":false,"context_lines":[{"line_number":693,"context_line":"            norm_name \u003d _normalize_resource_class_name(rc_name)"},{"line_number":694,"context_line":"            if norm_name !\u003d rc_name:"},{"line_number":695,"context_line":"                inv_data[norm_name] \u003d inv_data[rc_name]"},{"line_number":696,"context_line":"                inv_data.pop(rc_name)"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"        if inv_data:"},{"line_number":699,"context_line":"            self._update_inventory(rp_uuid, inv_data)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_6dec88f3","line":696,"in_reply_to":"da36d5c6_0d76ac6f","updated":"2017-02-24 14:03:43.000000000","message":"Doh, great catch, thank you Dan.","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a83c06764b85d2d4a594c46c192cd58ad186671a","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        return None"},{"line_number":144,"context_line":"    norm_name \u003d rc_name.upper()"},{"line_number":145,"context_line":"    cust_prefix \u003d objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":146,"context_line":"    if not norm_name.startswith(cust_prefix):"},{"line_number":147,"context_line":"        norm_name \u003d cust_prefix + norm_name"},{"line_number":148,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":149,"context_line":"        norm_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, norm_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_7be354d9","line":146,"updated":"2017-02-24 21:03:57.000000000","message":"Again, I don\u0027t think we should do this here and should require the driver to return valid resource names.\n\nImagine that we have a typo in a driver that returns a resource of \"MEMORY_GB\" then we will create \"CUSTOM_MEMORY_GB\" instead of failing immediately and warning that we refuse to create a custom class from something that was clearly supposed to be a standard one (i.e. MEMORY_MB).\n\nFurther, imagine the case where someone in ironic calls the node class \"custom_silver\", which means we\u0027ll not create CUSTOM_$ironic_class, we\u0027ll just assume the driver meant to namespace it, but in fact the custom resource class won\u0027t properly select the ironic node class. We should have created CUSTOM_CUSTOM_SILVER but we didn\u0027t because we allow this ambiguous result from the virt driver.","commit_id":"512c11fb539c33969a7c12a5eecb6f04ca025b76"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae5baf72e0e5f335d130b09f75259ebbca95b4dc","unresolved":false,"context_lines":[{"line_number":699,"context_line":"        else:"},{"line_number":700,"context_line":"            self._delete_inventory(rp_uuid)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def _ensure_resource_class(self, node_class):"},{"line_number":703,"context_line":"        \"\"\"Given a custom resource class, ensure that the resource class exists"},{"line_number":704,"context_line":"        in the placement API and return it. If the resource class cannot be"},{"line_number":705,"context_line":"        normalized or created, return None."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_82522edc","line":702,"range":{"start_line":702,"start_character":37,"end_line":702,"end_character":47},"updated":"2017-02-28 15:23:12.000000000","message":"Assume this is leftover from when this was ironic specific, but this should be \"resource_class\"","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a7f617b80a4c7850ac145002476bbe853b846a54","unresolved":false,"context_lines":[{"line_number":699,"context_line":"        else:"},{"line_number":700,"context_line":"            self._delete_inventory(rp_uuid)"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"    def _ensure_resource_class(self, node_class):"},{"line_number":703,"context_line":"        \"\"\"Given a custom resource class, ensure that the resource class exists"},{"line_number":704,"context_line":"        in the placement API and return it. If the resource class cannot be"},{"line_number":705,"context_line":"        normalized or created, return None."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_f13e21b7","line":702,"range":{"start_line":702,"start_character":37,"end_line":702,"end_character":47},"in_reply_to":"ba2be162_82522edc","updated":"2017-03-01 15:46:14.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae5baf72e0e5f335d130b09f75259ebbca95b4dc","unresolved":false,"context_lines":[{"line_number":708,"context_line":"                           API for a node\u0027s resource class, or None."},{"line_number":709,"context_line":"        \"\"\""},{"line_number":710,"context_line":"        if node_class is None:"},{"line_number":711,"context_line":"            return None"},{"line_number":712,"context_line":"        try:"},{"line_number":713,"context_line":"            # NOTE(jaypipes): Here, we auto-create the Ironic bare-metal"},{"line_number":714,"context_line":"            # resource class as a custom resource class in the placement API."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_e2dcca48","line":711,"updated":"2017-02-28 15:23:12.000000000","message":"This is mirrored in the other stuff, so I\u0027m hoping this is just leftover. Why should we take None here and quietly ignore it? If the virt driver returns a custom type in the dict, I would assume that means it has something to report, otherwise it wouldn\u0027t be there. I\u0027d prefer not to have all these silent ignore conditionals.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a7f617b80a4c7850ac145002476bbe853b846a54","unresolved":false,"context_lines":[{"line_number":708,"context_line":"                           API for a node\u0027s resource class, or None."},{"line_number":709,"context_line":"        \"\"\""},{"line_number":710,"context_line":"        if node_class is None:"},{"line_number":711,"context_line":"            return None"},{"line_number":712,"context_line":"        try:"},{"line_number":713,"context_line":"            # NOTE(jaypipes): Here, we auto-create the Ironic bare-metal"},{"line_number":714,"context_line":"            # resource class as a custom resource class in the placement API."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_b15ca926","line":711,"in_reply_to":"ba2be162_e2dcca48","updated":"2017-03-01 15:46:14.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ae5baf72e0e5f335d130b09f75259ebbca95b4dc","unresolved":false,"context_lines":[{"line_number":710,"context_line":"        if node_class is None:"},{"line_number":711,"context_line":"            return None"},{"line_number":712,"context_line":"        try:"},{"line_number":713,"context_line":"            # NOTE(jaypipes): Here, we auto-create the Ironic bare-metal"},{"line_number":714,"context_line":"            # resource class as a custom resource class in the placement API."},{"line_number":715,"context_line":"            # We do this to avoid the need for the deployer to set up custom"},{"line_number":716,"context_line":"            # resource classes in \u003e1 place (they have already set the resource"},{"line_number":717,"context_line":"            # class in their Ironic nodes)."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_a2b9f2ee","line":714,"range":{"start_line":713,"start_character":55,"end_line":714,"end_character":28},"updated":"2017-02-28 15:23:12.000000000","message":"Remove ironic references.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a7f617b80a4c7850ac145002476bbe853b846a54","unresolved":false,"context_lines":[{"line_number":710,"context_line":"        if node_class is None:"},{"line_number":711,"context_line":"            return None"},{"line_number":712,"context_line":"        try:"},{"line_number":713,"context_line":"            # NOTE(jaypipes): Here, we auto-create the Ironic bare-metal"},{"line_number":714,"context_line":"            # resource class as a custom resource class in the placement API."},{"line_number":715,"context_line":"            # We do this to avoid the need for the deployer to set up custom"},{"line_number":716,"context_line":"            # resource classes in \u003e1 place (they have already set the resource"},{"line_number":717,"context_line":"            # class in their Ironic nodes)."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_31499960","line":714,"range":{"start_line":713,"start_character":55,"end_line":714,"end_character":28},"in_reply_to":"ba2be162_a2b9f2ee","updated":"2017-03-01 15:46:14.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":732,"context_line":"        :param name: String name of the resource class to check/create."},{"line_number":733,"context_line":"        \"\"\""},{"line_number":734,"context_line":"        resp \u003d self.get(\"/resource_classes/%s\" % name, version\u003d\"1.2\")"},{"line_number":735,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":736,"context_line":"            return name"},{"line_number":737,"context_line":"        elif resp.status_code \u003d\u003d 404:"},{"line_number":738,"context_line":"            self._create_resource_class(name)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_a88d4e2c","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"updated":"2017-02-27 17:48:00.000000000","message":"Nit: we just had a lot of discussion in the API-WG about this, and the recommendation is to always check for 2xx, not a specific 2xx value. So if you respin, update to:\n\n    if 200 \u003c\u003d resp.status_code \u003c 300:","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":732,"context_line":"        :param name: String name of the resource class to check/create."},{"line_number":733,"context_line":"        \"\"\""},{"line_number":734,"context_line":"        resp \u003d self.get(\"/resource_classes/%s\" % name, version\u003d\"1.2\")"},{"line_number":735,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":736,"context_line":"            return name"},{"line_number":737,"context_line":"        elif resp.status_code \u003d\u003d 404:"},{"line_number":738,"context_line":"            self._create_resource_class(name)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_f1bada6d","line":735,"range":{"start_line":735,"start_character":8,"end_line":735,"end_character":35},"in_reply_to":"da36d5c6_a88d4e2c","updated":"2017-02-28 14:52:37.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":740,"context_line":"        else:"},{"line_number":741,"context_line":"            msg \u003d _LE(\"Failed to retrieve resource class record from \""},{"line_number":742,"context_line":"                      \"placement API for resource class %(rc_name)s. \""},{"line_number":743,"context_line":"                                \"Got %(status_code)d: %(err_text)s.\")"},{"line_number":744,"context_line":"            args \u003d {"},{"line_number":745,"context_line":"                \u0027rc_name\u0027: name,"},{"line_number":746,"context_line":"                \u0027status_code\u0027: resp.status_code,"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_c8b9a2cd","line":743,"range":{"start_line":743,"start_character":22,"end_line":743,"end_character":32},"updated":"2017-02-27 17:48:00.000000000","message":"weird extra indentation?","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":740,"context_line":"        else:"},{"line_number":741,"context_line":"            msg \u003d _LE(\"Failed to retrieve resource class record from \""},{"line_number":742,"context_line":"                      \"placement API for resource class %(rc_name)s. \""},{"line_number":743,"context_line":"                                \"Got %(status_code)d: %(err_text)s.\")"},{"line_number":744,"context_line":"            args \u003d {"},{"line_number":745,"context_line":"                \u0027rc_name\u0027: name,"},{"line_number":746,"context_line":"                \u0027status_code\u0027: resp.status_code,"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_b1c062e2","line":743,"range":{"start_line":743,"start_character":22,"end_line":743,"end_character":32},"in_reply_to":"da36d5c6_c8b9a2cd","updated":"2017-02-28 14:52:37.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":762,"context_line":"            \u0027name\u0027: name,"},{"line_number":763,"context_line":"        }"},{"line_number":764,"context_line":"        resp \u003d self.post(url, payload, version\u003d\"1.2\")"},{"line_number":765,"context_line":"        if resp.status_code \u003d\u003d 201:"},{"line_number":766,"context_line":"            msg \u003d _LI(\"Created resource class record via placement API \""},{"line_number":767,"context_line":"                      \"for resource class %s.\")"},{"line_number":768,"context_line":"            LOG.info(msg, name)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_68c35639","line":765,"range":{"start_line":765,"start_character":28,"end_line":765,"end_character":34},"updated":"2017-02-27 17:48:00.000000000","message":"see nit above","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":762,"context_line":"            \u0027name\u0027: name,"},{"line_number":763,"context_line":"        }"},{"line_number":764,"context_line":"        resp \u003d self.post(url, payload, version\u003d\"1.2\")"},{"line_number":765,"context_line":"        if resp.status_code \u003d\u003d 201:"},{"line_number":766,"context_line":"            msg \u003d _LI(\"Created resource class record via placement API \""},{"line_number":767,"context_line":"                      \"for resource class %s.\")"},{"line_number":768,"context_line":"            LOG.info(msg, name)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_3c3e09b9","line":765,"range":{"start_line":765,"start_character":28,"end_line":765,"end_character":34},"in_reply_to":"da36d5c6_68c35639","updated":"2017-02-28 14:52:37.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"2c68ba77fc1350f8124d15375cdd88e5a5dc8e86","unresolved":false,"context_lines":[{"line_number":721,"context_line":"        else:"},{"line_number":722,"context_line":"            self._delete_inventory(rp_uuid)"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"    def _ensure_resource_class(self, node_class):"},{"line_number":725,"context_line":"        \"\"\"Given a custom resource class, ensure that the resource class exists"},{"line_number":726,"context_line":"        in the placement API and return it. If the resource class cannot be"},{"line_number":727,"context_line":"        normalized or created, return None."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba2be162_a46acd18","line":724,"range":{"start_line":724,"start_character":37,"end_line":724,"end_character":47},"updated":"2017-03-01 00:01:09.000000000","message":"Many of Dan\u0027s comments in PS5 don\u0027t seem to have been addressed. FWIW, I agree with the idea of removing Ironic specialness from the code.","commit_id":"3d5f4ab0f45d250cf002a5b5251c7c260c0da4b0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bf3f45b3697fe7d287d6111488beccd5e4565a87","unresolved":false,"context_lines":[{"line_number":721,"context_line":"        else:"},{"line_number":722,"context_line":"            self._delete_inventory(rp_uuid)"},{"line_number":723,"context_line":""},{"line_number":724,"context_line":"    def _ensure_resource_class(self, node_class):"},{"line_number":725,"context_line":"        \"\"\"Given a custom resource class, ensure that the resource class exists"},{"line_number":726,"context_line":"        in the placement API and return it. If the resource class cannot be"},{"line_number":727,"context_line":"        normalized or created, return None."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba2be162_960cfb95","line":724,"range":{"start_line":724,"start_character":37,"end_line":724,"end_character":47},"in_reply_to":"ba2be162_a46acd18","updated":"2017-03-01 15:35:03.000000000","message":"crap, yeah... lemme address those quickly.","commit_id":"3d5f4ab0f45d250cf002a5b5251c7c260c0da4b0"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a6ff2a8da55048ffa90c5d481955260c703fff5f","unresolved":false,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"            # Auto-create custom resource classes for only those providers for"},{"line_number":718,"context_line":"            # which we have made a contract to do so, such as Ironic."},{"line_number":719,"context_line":"            new_inv[rc_name] \u003d inv"},{"line_number":720,"context_line":"            if create_class:"},{"line_number":721,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":722,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_8c29073a","line":719,"updated":"2017-03-02 17:49:47.000000000","message":"can we have lines 714 and 719 deduped and be before the conditional in line 713?","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f00e37c9b95bccdbd76ecc0918b30ebe2d45d845","unresolved":false,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"            # Auto-create custom resource classes for only those providers for"},{"line_number":718,"context_line":"            # which we have made a contract to do so, such as Ironic."},{"line_number":719,"context_line":"            new_inv[rc_name] \u003d inv"},{"line_number":720,"context_line":"            if create_class:"},{"line_number":721,"context_line":"                self._get_or_create_resource_class(rc_name)"},{"line_number":722,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_a76fc429","line":719,"in_reply_to":"ba2be162_8c29073a","updated":"2017-03-02 17:59:52.000000000","message":"Done","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a6ff2a8da55048ffa90c5d481955260c703fff5f","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        :param name: String name of the resource class to check/create."},{"line_number":736,"context_line":"        \"\"\""},{"line_number":737,"context_line":"        resp \u003d self.get(\"/resource_classes/%s\" % name, version\u003d\"1.2\")"},{"line_number":738,"context_line":"        if 200 \u003c\u003d resp.status_code \u003c 300:"},{"line_number":739,"context_line":"            return name"},{"line_number":740,"context_line":"        elif resp.status_code \u003d\u003d 404:"},{"line_number":741,"context_line":"            self._create_resource_class(name)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_4c9c6f0f","line":738,"updated":"2017-03-02 17:49:47.000000000","message":"If you\u0027re feeling saucy you should be able to do:\n\n    if resp:\n\nas the equivalent of what you\u0027ve got there.","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f00e37c9b95bccdbd76ecc0918b30ebe2d45d845","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        :param name: String name of the resource class to check/create."},{"line_number":736,"context_line":"        \"\"\""},{"line_number":737,"context_line":"        resp \u003d self.get(\"/resource_classes/%s\" % name, version\u003d\"1.2\")"},{"line_number":738,"context_line":"        if 200 \u003c\u003d resp.status_code \u003c 300:"},{"line_number":739,"context_line":"            return name"},{"line_number":740,"context_line":"        elif resp.status_code \u003d\u003d 404:"},{"line_number":741,"context_line":"            self._create_resource_class(name)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_c753d869","line":738,"in_reply_to":"ba2be162_4c9c6f0f","updated":"2017-03-02 17:59:52.000000000","message":"I\u0027d prefer not, since it\u0027s not explicit.","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a6ff2a8da55048ffa90c5d481955260c703fff5f","unresolved":false,"context_lines":[{"line_number":752,"context_line":"            LOG.error(msg, args)"},{"line_number":753,"context_line":"            return None"},{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def _create_resource_class(self, name):"},{"line_number":756,"context_line":"        \"\"\"Calls the placement API to create a new resource class."},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        :param name: String name of the resource class to create."}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_e7e53c38","line":755,"updated":"2017-03-02 17:49:47.000000000","message":"After looking at the server-side code, I\u0027m wondering if maybe it would be easier if we changed PUT /resource_classes/{name} to always work, both create and update. Except that the current PUT is effectively a sort of weird rename, not an update.\n\nIf we did that, this method can just be a single PUT rather than a GET and a POST. I assume that probably violates the desire to avoid a write of the database, but we could avoid that write server side and thus avoid (sometimes) two http requests.","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f00e37c9b95bccdbd76ecc0918b30ebe2d45d845","unresolved":false,"context_lines":[{"line_number":752,"context_line":"            LOG.error(msg, args)"},{"line_number":753,"context_line":"            return None"},{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def _create_resource_class(self, name):"},{"line_number":756,"context_line":"        \"\"\"Calls the placement API to create a new resource class."},{"line_number":757,"context_line":""},{"line_number":758,"context_line":"        :param name: String name of the resource class to create."}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_e7561c29","line":755,"in_reply_to":"ba2be162_e7e53c38","updated":"2017-03-02 17:59:52.000000000","message":"Something to consider in a future patch, since that would require a microversion bump.","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30e3639ea061d68e8b9b920de1ff2db9eff57a7e","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":697,"context_line":"            # Strip out any 0-valued inventory items (Ironic, for instance, can"},{"line_number":698,"context_line":"            # report 0-valued inventory for \"disabled\" nodes"},{"line_number":699,"context_line":"            if inv[\u0027total\u0027] \u003d\u003d 0:"},{"line_number":700,"context_line":"                continue"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD and create_class:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_6de68db1","line":699,"updated":"2017-03-02 18:36:43.000000000","message":"Why not just have ironic not report that? I mean, I guess it\u0027s good to have the logic here too to prevent \"didn\u0027t think of that\" bugs from the virt driver, but.. I dunno, feels weird.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0b41dff8a66892fa511fc464dace82a9e56b1b34","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        for rc_name, inv in inv_data.items():"},{"line_number":697,"context_line":"            # Strip out any 0-valued inventory items (Ironic, for instance, can"},{"line_number":698,"context_line":"            # report 0-valued inventory for \"disabled\" nodes"},{"line_number":699,"context_line":"            if inv[\u0027total\u0027] \u003d\u003d 0:"},{"line_number":700,"context_line":"                continue"},{"line_number":701,"context_line":""},{"line_number":702,"context_line":"            if rc_name not in fields.ResourceClass.STANDARD and create_class:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_cdb92f7b","line":699,"in_reply_to":"ba2be162_6de68db1","updated":"2017-03-02 22:36:42.000000000","message":"We had this discussion on a previous patch that was bombing out on 0-valued inventory items. vdrok actually had a patch up to have Ironic not return vcpus for disabled nodes in get_available_resource() but that broke a bunch of assumptions in the resource tracker. I suppose now that this patch introduces a get_inventory() method that is only implemented in the Ironic virt driver and doesn\u0027t affect anything in the resource tracker we could do that, sure.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"}],"nova/tests/functional/compute/test_resource_tracker.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    def placement_get_inventory(self, rp_uuid):"},{"line_number":178,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % rp_uuid"},{"line_number":179,"context_line":"        resp \u003d self.report_client.get(url)"},{"line_number":180,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":181,"context_line":"            return resp.json()[\u0027inventories\u0027]"},{"line_number":182,"context_line":"        else:"},{"line_number":183,"context_line":"            return resp.status_code"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_681fd6b7","line":180,"range":{"start_line":180,"start_character":11,"end_line":180,"end_character":35},"updated":"2017-02-27 17:48:00.000000000","message":"My nitpicky comment about 2xx status codes applies to all such comparisons in this file.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":177,"context_line":"    def placement_get_inventory(self, rp_uuid):"},{"line_number":178,"context_line":"        url \u003d \u0027/resource_providers/%s/inventories\u0027 % rp_uuid"},{"line_number":179,"context_line":"        resp \u003d self.report_client.get(url)"},{"line_number":180,"context_line":"        if resp.status_code \u003d\u003d 200:"},{"line_number":181,"context_line":"            return resp.json()[\u0027inventories\u0027]"},{"line_number":182,"context_line":"        else:"},{"line_number":183,"context_line":"            return resp.status_code"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_3ccb6974","line":180,"range":{"start_line":180,"start_character":11,"end_line":180,"end_character":35},"in_reply_to":"da36d5c6_681fd6b7","updated":"2017-02-28 14:52:37.000000000","message":"Done","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"            # Verify the auto-creation of the custom resource class, normalized"},{"line_number":368,"context_line":"            # to what the placement API expects"},{"line_number":369,"context_line":"            self.assertIn(\u0027CUSTOM_SMALL_IRON\u0027, self.placement_get_custom_rcs())"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"            allocs \u003d self.placement_get_allocations(inst.uuid)"},{"line_number":372,"context_line":"            self.assertEqual(1, len(allocs))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_eb8100c5","line":369,"range":{"start_line":369,"start_character":12,"end_line":369,"end_character":79},"updated":"2017-02-27 17:48:00.000000000","message":"It would be illustrative to also assert that this custom class is *not* present before update_available_resource() is called.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"            # Verify the auto-creation of the custom resource class, normalized"},{"line_number":368,"context_line":"            # to what the placement API expects"},{"line_number":369,"context_line":"            self.assertIn(\u0027CUSTOM_SMALL_IRON\u0027, self.placement_get_custom_rcs())"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"            allocs \u003d self.placement_get_allocations(inst.uuid)"},{"line_number":372,"context_line":"            self.assertEqual(1, len(allocs))"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_3c928958","line":369,"range":{"start_line":369,"start_character":12,"end_line":369,"end_character":79},"in_reply_to":"da36d5c6_eb8100c5","updated":"2017-02-28 14:52:37.000000000","message":"good point. done.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"2c68ba77fc1350f8124d15375cdd88e5a5dc8e86","unresolved":false,"context_lines":[{"line_number":283,"context_line":""},{"line_number":284,"context_line":"            self.assertEqual(3, len(self.rt.compute_nodes))"},{"line_number":285,"context_line":"            # A canary just to make sure the assertion below about the custom"},{"line_number":286,"context_line":"            # resource class being added wasn\u0027t already added somehow..."},{"line_number":287,"context_line":"            self.assertNotIn(\u0027CUSTOM_SMALL_IRON\u0027, self.placement_get_custom_rcs())"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"            # Verify that the placement API has the \"old-style\" resources in"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba2be162_2465fd44","line":286,"updated":"2017-03-01 00:01:09.000000000","message":"Thanks; this makes a lot more sense now.","commit_id":"3d5f4ab0f45d250cf002a5b5251c7c260c0da4b0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30e3639ea061d68e8b9b920de1ff2db9eff57a7e","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                return_value\u003d{\u0027x-auth-token\u0027: \u0027admin\u0027})"},{"line_number":206,"context_line":"    @mock.patch(\u0027keystoneauth1.session.Session.get_endpoint\u0027,"},{"line_number":207,"context_line":"                return_value\u003d\u0027http://localhost/placement\u0027)"},{"line_number":208,"context_line":"    def test_ironic_newton_to_ocata(self, mock_vbi, mock_endpoint, mock_auth,"},{"line_number":209,"context_line":"            mock_cn):"},{"line_number":210,"context_line":"        \"\"\"Check that when going from a Newton installation with Ironic having"},{"line_number":211,"context_line":"        node\u0027s resource class attributes set, that we properly \"auto-heal\" the"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_0d4a71ac","line":208,"updated":"2017-03-02 18:36:43.000000000","message":"These really feel like unit tests with all the mocks. Functional tests should, IMHO, rely on few if any mocks and largely be hitting things from the API and not directly to self.resource_tracker, etc. Just..sayin.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0b41dff8a66892fa511fc464dace82a9e56b1b34","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                return_value\u003d{\u0027x-auth-token\u0027: \u0027admin\u0027})"},{"line_number":206,"context_line":"    @mock.patch(\u0027keystoneauth1.session.Session.get_endpoint\u0027,"},{"line_number":207,"context_line":"                return_value\u003d\u0027http://localhost/placement\u0027)"},{"line_number":208,"context_line":"    def test_ironic_newton_to_ocata(self, mock_vbi, mock_endpoint, mock_auth,"},{"line_number":209,"context_line":"            mock_cn):"},{"line_number":210,"context_line":"        \"\"\"Check that when going from a Newton installation with Ironic having"},{"line_number":211,"context_line":"        node\u0027s resource class attributes set, that we properly \"auto-heal\" the"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_ed97d345","line":208,"in_reply_to":"ba2be162_0d4a71ac","updated":"2017-03-02 22:36:42.000000000","message":"We\u0027re testing the integration of the resource tracker and the scheduling client in a functional way. We\u0027re not mocking out the resource tracker or the scheduling client. If I could do this test by hitting the API layer, I would, but it would be 12000 lines long because of all the coupling... :(","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb6f2b23d5ae9282eb3f709b4baac56c31c7b9f1","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                return_value\u003d{\u0027x-auth-token\u0027: \u0027admin\u0027})"},{"line_number":206,"context_line":"    @mock.patch(\u0027keystoneauth1.session.Session.get_endpoint\u0027,"},{"line_number":207,"context_line":"                return_value\u003d\u0027http://localhost/placement\u0027)"},{"line_number":208,"context_line":"    def test_ironic_newton_to_ocata(self, mock_vbi, mock_endpoint, mock_auth,"},{"line_number":209,"context_line":"            mock_cn):"},{"line_number":210,"context_line":"        \"\"\"Check that when going from a Newton installation with Ironic having"},{"line_number":211,"context_line":"        node\u0027s resource class attributes set, that we properly \"auto-heal\" the"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_308f7e76","line":208,"in_reply_to":"ba2be162_b0552efb","updated":"2017-03-02 22:47:15.000000000","message":"Mainly these are here because I copied the existing test from tests/functional/api/openstack/placement/test_report_client.py :)","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71d339951149d90a6af7980d4e462387785c17cb","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                return_value\u003d{\u0027x-auth-token\u0027: \u0027admin\u0027})"},{"line_number":206,"context_line":"    @mock.patch(\u0027keystoneauth1.session.Session.get_endpoint\u0027,"},{"line_number":207,"context_line":"                return_value\u003d\u0027http://localhost/placement\u0027)"},{"line_number":208,"context_line":"    def test_ironic_newton_to_ocata(self, mock_vbi, mock_endpoint, mock_auth,"},{"line_number":209,"context_line":"            mock_cn):"},{"line_number":210,"context_line":"        \"\"\"Check that when going from a Newton installation with Ironic having"},{"line_number":211,"context_line":"        node\u0027s resource class attributes set, that we properly \"auto-heal\" the"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_b0552efb","line":208,"in_reply_to":"ba2be162_ed97d345","updated":"2017-03-02 22:42:20.000000000","message":"I have no problem with the way these work, they just don\u0027t really feel like \"functional\" tests to me. I.e. it seems they should be in the unit area. But, if I\u0027m the only one then ignore me.","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"}],"nova/tests/unit/scheduler/test_client.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c6ac56c7096dd8f44f94d05a90e0ee8a4cada16d","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    @mock.patch.object(scheduler_report_client.SchedulerReportClient,"},{"line_number":100,"context_line":"                       \u0027set_inventory_for_provider\u0027)"},{"line_number":101,"context_line":"    def test_set_inventory_for_provider(self, mock_set):"},{"line_number":102,"context_line":"        self.client.set_inventory_for_provider("},{"line_number":103,"context_line":"            mock.sentinel.rp_uuid,"},{"line_number":104,"context_line":"            mock.sentinel.rp_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_c9601fd2","line":101,"updated":"2017-02-23 20:37:44.000000000","message":"Okay, I guess this is good for completeness and provides a nice barrier to prevent regressions, but this kind of test makes me sad somehow.","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d4d9ae02d0d2962a94d98551170fc96a9165b071","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    @mock.patch.object(scheduler_report_client.SchedulerReportClient,"},{"line_number":100,"context_line":"                       \u0027set_inventory_for_provider\u0027)"},{"line_number":101,"context_line":"    def test_set_inventory_for_provider(self, mock_set):"},{"line_number":102,"context_line":"        self.client.set_inventory_for_provider("},{"line_number":103,"context_line":"            mock.sentinel.rp_uuid,"},{"line_number":104,"context_line":"            mock.sentinel.rp_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_c991fff2","line":101,"in_reply_to":"da36d5c6_c9601fd2","updated":"2017-02-23 20:43:29.000000000","message":"oh, me too, trust me...","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            mock.sentinel.inv_data,"},{"line_number":106,"context_line":"        )"},{"line_number":107,"context_line":"        mock_set.assert_called_once_with("},{"line_number":108,"context_line":"            mock.sentinel.rp_uuid,"},{"line_number":109,"context_line":"            mock.sentinel.rp_name,"},{"line_number":110,"context_line":"            mock.sentinel.inv_data,"},{"line_number":111,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_ab61c813","line":111,"range":{"start_line":108,"start_character":7,"end_line":111,"end_character":9},"updated":"2017-02-27 17:48:00.000000000","message":"I\u0027m not seeing anything being tested here. You\u0027re mocking a method, calling it, and then asserting that the mock was called?","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            mock.sentinel.inv_data,"},{"line_number":106,"context_line":"        )"},{"line_number":107,"context_line":"        mock_set.assert_called_once_with("},{"line_number":108,"context_line":"            mock.sentinel.rp_uuid,"},{"line_number":109,"context_line":"            mock.sentinel.rp_name,"},{"line_number":110,"context_line":"            mock.sentinel.inv_data,"},{"line_number":111,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_9c037576","line":111,"range":{"start_line":108,"start_character":7,"end_line":111,"end_character":9},"in_reply_to":"da36d5c6_ab61c813","updated":"2017-02-28 14:52:37.000000000","message":"it\u0027s because of the composition of a scheduler client that has a reporting client embedded in it...","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"c6ac56c7096dd8f44f94d05a90e0ee8a4cada16d","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        rc_name \u003d info.get(\u0027resource_class\u0027)"},{"line_number":653,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":654,"context_line":"            # NOTE(jaypipes): resource class name is normalized in the"},{"line_number":655,"context_line":"            # scheduler reporting client"},{"line_number":656,"context_line":"            result[rc_name] \u003d {"},{"line_number":657,"context_line":"                \u0027total\u0027: 1,"},{"line_number":658,"context_line":"                \u0027reserved\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_49a98f8f","line":655,"updated":"2017-02-23 20:37:44.000000000","message":"+1","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"eb07806174d557176ef9b3c3d20d5f32bf217669","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        rc_name \u003d info.get(\u0027resource_class\u0027)"},{"line_number":653,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":654,"context_line":"            # NOTE(jaypipes): resource class name is normalized in the"},{"line_number":655,"context_line":"            # scheduler reporting client"},{"line_number":656,"context_line":"            result[rc_name] \u003d {"},{"line_number":657,"context_line":"                \u0027total\u0027: 1,"},{"line_number":658,"context_line":"                \u0027reserved\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_0da78caa","line":655,"in_reply_to":"da36d5c6_0dfdccb3","updated":"2017-02-24 14:06:39.000000000","message":"My +1 on this was on the concept of making sure the sanity check and the normalization was kept to one layer and since presumably something other than a virt driver might want to produce a resource class, \"not this layer\" is good.","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"806f7a92610d9c917aca7c935d2639edc105e59e","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        rc_name \u003d info.get(\u0027resource_class\u0027)"},{"line_number":653,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":654,"context_line":"            # NOTE(jaypipes): resource class name is normalized in the"},{"line_number":655,"context_line":"            # scheduler reporting client"},{"line_number":656,"context_line":"            result[rc_name] \u003d {"},{"line_number":657,"context_line":"                \u0027total\u0027: 1,"},{"line_number":658,"context_line":"                \u0027reserved\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_2dcd3085","line":655,"in_reply_to":"da36d5c6_0dfdccb3","updated":"2017-02-24 14:03:43.000000000","message":"See my note on the previous comment...","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"103fa6cb9504cafd42597a497b9c27b8473b6c25","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        rc_name \u003d info.get(\u0027resource_class\u0027)"},{"line_number":653,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":654,"context_line":"            # NOTE(jaypipes): resource class name is normalized in the"},{"line_number":655,"context_line":"            # scheduler reporting client"},{"line_number":656,"context_line":"            result[rc_name] \u003d {"},{"line_number":657,"context_line":"                \u0027total\u0027: 1,"},{"line_number":658,"context_line":"                \u0027reserved\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_0dfdccb3","line":655,"in_reply_to":"da36d5c6_49a98f8f","updated":"2017-02-24 13:59:20.000000000","message":"Why? This seems wrong to me. Why would we not have the driver return a sane set of classes (either standard ones or properly-prefixed CUSTOM_FOO ones?","commit_id":"ea6b76fd4110a31d2fa863c3d9a88d7b007b297d"},{"author":{"_account_id":7900,"name":"Tomasz Paszkowski","email":"ss7pro@gmail.com","username":"tpaszkowski"},"change_message_id":"9a144444bf10058924c61564e7e63ade9e8d5eb7","unresolved":false,"context_lines":[{"line_number":626,"context_line":"            obj_fields.ResourceClass.VCPU: {"},{"line_number":627,"context_line":"                \u0027total\u0027: info[\u0027vcpus\u0027],"},{"line_number":628,"context_line":"                \u0027reserved\u0027: 0,"},{"line_number":629,"context_line":"                \u0027min_unit\u0027: 1,"},{"line_number":630,"context_line":"                \u0027max_unit\u0027: info[\u0027vcpus\u0027],"},{"line_number":631,"context_line":"                \u0027step_size\u0027: 1,"},{"line_number":632,"context_line":"                \u0027allocation_ratio\u0027: 1.0,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_8c236355","line":629,"updated":"2017-02-25 16:55:20.000000000","message":"min_unit should be the same as max_unit, or the comment above is not accurate here.","commit_id":"512c11fb539c33969a7c12a5eecb6f04ca025b76"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"af80671e4f11194ac28eca39f67fd1f57695d03f","unresolved":false,"context_lines":[{"line_number":626,"context_line":"            obj_fields.ResourceClass.VCPU: {"},{"line_number":627,"context_line":"                \u0027total\u0027: info[\u0027vcpus\u0027],"},{"line_number":628,"context_line":"                \u0027reserved\u0027: 0,"},{"line_number":629,"context_line":"                \u0027min_unit\u0027: 1,"},{"line_number":630,"context_line":"                \u0027max_unit\u0027: info[\u0027vcpus\u0027],"},{"line_number":631,"context_line":"                \u0027step_size\u0027: 1,"},{"line_number":632,"context_line":"                \u0027allocation_ratio\u0027: 1.0,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba2be162_8548755d","line":629,"in_reply_to":"da36d5c6_8c236355","updated":"2017-02-28 16:23:18.000000000","message":"Hi Tomasz,\n\nYeah, originally that\u0027s how I had it, but ran into issues because Ironic returns a total value of vcpus/memory_mb/disk_gb when a node is \"disabled\" and this messed up the testing fixtures.\n\nLong term, we\u0027ll be getting rid of the VCPU, MEMORY_MB and DISK_GB entries entirely and we will only have the custom resource class.","commit_id":"512c11fb539c33969a7c12a5eecb6f04ca025b76"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2e58d1bc34ae429cede01580989388236e28feac","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":654,"context_line":"            # NOTE(jaypipes): resource class name is normalized in the"},{"line_number":655,"context_line":"            # scheduler reporting client"},{"line_number":656,"context_line":"            result[rc_name] \u003d {"},{"line_number":657,"context_line":"                \u0027total\u0027: 1,"},{"line_number":658,"context_line":"                \u0027reserved\u0027: 0,"},{"line_number":659,"context_line":"                \u0027min_unit\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_7b319459","line":656,"updated":"2017-02-24 21:05:03.000000000","message":"Just to make my comment clearer, I think we should normalize here, either with specific code or calling a normalize utility function. We should definitely check and enforce that it\u0027s legit in the top.","commit_id":"512c11fb539c33969a7c12a5eecb6f04ca025b76"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f09a941aea47102fed902ec6eaefeb209529e76","unresolved":false,"context_lines":[{"line_number":123,"context_line":"              instance\u003dinstance)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"def _normalize_resource_class_name(rc_name):"},{"line_number":127,"context_line":"    if rc_name is None:"},{"line_number":128,"context_line":"        return None"},{"line_number":129,"context_line":"    norm_name \u003d rc_name.upper()"}],"source_content_type":"text/x-python","patch_set":5,"id":"da36d5c6_ab162856","line":126,"range":{"start_line":126,"start_character":4,"end_line":126,"end_character":34},"updated":"2017-02-27 17:48:00.000000000","message":"Why is this here? This should be common for all custom resource classes, not just ironic.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d23be9fe2274d9a9d0180cf6d12012ebd7c1115c","unresolved":false,"context_lines":[{"line_number":123,"context_line":"              instance\u003dinstance)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"def _normalize_resource_class_name(rc_name):"},{"line_number":127,"context_line":"    if rc_name is None:"},{"line_number":128,"context_line":"        return None"},{"line_number":129,"context_line":"    norm_name \u003d rc_name.upper()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_77efbab0","line":126,"range":{"start_line":126,"start_character":4,"end_line":126,"end_character":34},"in_reply_to":"ba2be162_fc8711d5","updated":"2017-02-28 15:09:54.000000000","message":"It totally shouldn\u0027t be here.\n\nMy argument is that the drivers should *return* normalized class names, not that the normalization routines should be inside them.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"facaae458d77c904ff329eb455b412e65958a03e","unresolved":false,"context_lines":[{"line_number":123,"context_line":"              instance\u003dinstance)"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"def _normalize_resource_class_name(rc_name):"},{"line_number":127,"context_line":"    if rc_name is None:"},{"line_number":128,"context_line":"        return None"},{"line_number":129,"context_line":"    norm_name \u003d rc_name.upper()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_fc8711d5","line":126,"range":{"start_line":126,"start_character":4,"end_line":126,"end_character":34},"in_reply_to":"da36d5c6_ab162856","updated":"2017-02-28 14:52:37.000000000","message":"See @dansmith\u0027s comment on previous patch.","commit_id":"c4b0697d7d2d225b52716ac1da34e0c2585317ac"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c78c4f5562e190139e7b3a673bf7babf2c4eb350","unresolved":false,"context_lines":[{"line_number":128,"context_line":"        return None"},{"line_number":129,"context_line":"    norm_name \u003d rc_name.upper()"},{"line_number":130,"context_line":"    cust_prefix \u003d objects.ResourceClass.CUSTOM_NAMESPACE"},{"line_number":131,"context_line":"    if not norm_name.startswith(cust_prefix):"},{"line_number":132,"context_line":"        norm_name \u003d cust_prefix + norm_name"},{"line_number":133,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":134,"context_line":"        norm_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, norm_name)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_37e4620c","line":131,"updated":"2017-02-28 15:09:01.000000000","message":"As I said on the previous patch, this introduces ambiguity in that we will or won\u0027t add custom_ before the thing, depending on if it already exists. Let me try to explain another way.\n\nIf we have two ironic nodes, one with class \"custom_foo\" and the other with \"bar\", this code will create CUSTOM_FOO and CUSTOM_BAR in placement. That means anything else that looks at those values will have to deal with the ambiguity around the resource class either being identical to the node class, or prefixed with CUSTOM_. I think that we should *always* prefix and otherwise normalize the class name, even if it means we get a CUSTOM_CUSTOM_FOO.","commit_id":"07805789c9a13c94c5394ae9d56ba5a9749ec9b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c78c4f5562e190139e7b3a673bf7babf2c4eb350","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        norm_name \u003d cust_prefix + norm_name"},{"line_number":133,"context_line":"        # Replace some punctuation characters with underscores"},{"line_number":134,"context_line":"        norm_name \u003d re.sub(\u0027[^0-9A-Z]+\u0027, \u0027_\u0027, norm_name)"},{"line_number":135,"context_line":"        return norm_name"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"class IronicDriver(virt_driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_b79f72b3","line":135,"updated":"2017-02-28 15:09:01.000000000","message":"Obviously this is going to be needed by other drivers. When I said I wanted the drivers to return normalized and valid names, I didn\u0027t mean they all had to contain this logic. Can\u0027t we just put this in a utility method somewhere common? Could just be in driver.py since we import that here.","commit_id":"07805789c9a13c94c5394ae9d56ba5a9749ec9b7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c78c4f5562e190139e7b3a673bf7babf2c4eb350","unresolved":false,"context_lines":[{"line_number":665,"context_line":"        rc_name \u003d info.get(\u0027resource_class\u0027)"},{"line_number":666,"context_line":"        if rc_name is not None and not disabled:"},{"line_number":667,"context_line":"            norm_name \u003d _normalize_resource_class_name(rc_name)"},{"line_number":668,"context_line":"            if norm_name is not None:"},{"line_number":669,"context_line":"                result[norm_name] \u003d {"},{"line_number":670,"context_line":"                    \u0027total\u0027: 1,"},{"line_number":671,"context_line":"                    \u0027reserved\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba2be162_17a3de9f","line":668,"updated":"2017-02-28 15:09:01.000000000","message":"AFAICT, we can\u0027t get None back from _normalize_resource_class_name() since we check that it\u0027s not None before we call it. Can we just remove that conditional there and here?","commit_id":"07805789c9a13c94c5394ae9d56ba5a9749ec9b7"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a6ff2a8da55048ffa90c5d481955260c703fff5f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"from nova import objects"},{"line_number":48,"context_line":"from nova.objects import fields as obj_fields"},{"line_number":49,"context_line":"# For the normalize_resource_class_name() function..."},{"line_number":50,"context_line":"from nova.scheduler.client import report"},{"line_number":51,"context_line":"from nova import servicegroup"},{"line_number":52,"context_line":"from nova.virt import configdrive"},{"line_number":53,"context_line":"from nova.virt import driver as virt_driver"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_27b834c8","line":50,"updated":"2017-03-02 17:49:47.000000000","message":"Could it make more sense to put the normalize function in with the ResourceClass field package? Having to import the report client seems weird somehow.","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f00e37c9b95bccdbd76ecc0918b30ebe2d45d845","unresolved":false,"context_lines":[{"line_number":47,"context_line":"from nova import objects"},{"line_number":48,"context_line":"from nova.objects import fields as obj_fields"},{"line_number":49,"context_line":"# For the normalize_resource_class_name() function..."},{"line_number":50,"context_line":"from nova.scheduler.client import report"},{"line_number":51,"context_line":"from nova import servicegroup"},{"line_number":52,"context_line":"from nova.virt import configdrive"},{"line_number":53,"context_line":"from nova.virt import driver as virt_driver"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_12e36c34","line":50,"in_reply_to":"ba2be162_27b834c8","updated":"2017-03-02 17:59:52.000000000","message":"Done","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a6ff2a8da55048ffa90c5d481955260c703fff5f","unresolved":false,"context_lines":[{"line_number":619,"context_line":"        # NOTE(jaypipes): Ironic resource nodes never have any reserved"},{"line_number":620,"context_line":"        # resources nor do they ever allow oversubscription. The min and max"},{"line_number":621,"context_line":"        # unit values for VCPU, MEMORY_MB and DISK_GB should be the same, since"},{"line_number":622,"context_line":"        # these resources will be consumed in an all-or-none fashion."},{"line_number":623,"context_line":"        # TODO(jaypipes): Completely remove the reporting of VCPU, MEMORY_MB,"},{"line_number":624,"context_line":"        # and DISK_GB resource classes in early Queens when Ironic nodes will"},{"line_number":625,"context_line":"        # *always* return the custom resource class that represents the"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_0716d0f0","line":622,"updated":"2017-03-02 17:49:47.000000000","message":"You say this here, but then below the min_unit is 1?","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f00e37c9b95bccdbd76ecc0918b30ebe2d45d845","unresolved":false,"context_lines":[{"line_number":619,"context_line":"        # NOTE(jaypipes): Ironic resource nodes never have any reserved"},{"line_number":620,"context_line":"        # resources nor do they ever allow oversubscription. The min and max"},{"line_number":621,"context_line":"        # unit values for VCPU, MEMORY_MB and DISK_GB should be the same, since"},{"line_number":622,"context_line":"        # these resources will be consumed in an all-or-none fashion."},{"line_number":623,"context_line":"        # TODO(jaypipes): Completely remove the reporting of VCPU, MEMORY_MB,"},{"line_number":624,"context_line":"        # and DISK_GB resource classes in early Queens when Ironic nodes will"},{"line_number":625,"context_line":"        # *always* return the custom resource class that represents the"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba2be162_0702500b","line":622,"in_reply_to":"ba2be162_0716d0f0","updated":"2017-03-02 17:59:52.000000000","message":"Yeah, I\u0027ll remove the above. Was added before I realized the whole \"Ironic returns 0-valued inventories when a node is disabled\".","commit_id":"4a9ab32e3a300ba245b77e1ebbe9dbc1458710f4"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8677d42365338aa827e113b6e959f724fecb0681","unresolved":false,"context_lines":[{"line_number":623,"context_line":"                \u0027total\u0027: info[\u0027vcpus\u0027],"},{"line_number":624,"context_line":"                \u0027reserved\u0027: 0,"},{"line_number":625,"context_line":"                \u0027min_unit\u0027: 1,"},{"line_number":626,"context_line":"                \u0027max_unit\u0027: info[\u0027vcpus\u0027],"},{"line_number":627,"context_line":"                \u0027step_size\u0027: 1,"},{"line_number":628,"context_line":"                \u0027allocation_ratio\u0027: 1.0,"},{"line_number":629,"context_line":"            },"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_4d3e298d","line":626,"updated":"2017-03-02 18:20:10.000000000","message":"Given that if total is set to 0, then this will be set to 0 _and_ the inventory won\u0027t be written if it has a total of 0, then it might make sense to go ahead and set min_unit to info[\u0027vcpus\u0027] as well since that\u0027s the only legit allocation granularity (as you said in the now removed comment)?\n\nThis change https://review.openstack.org/#/c/428800/ is going to start validating inventories to disallow 0 in any field (other than reserved) and set defaults of at least 1 in any field (other than reserved).\n\nIt probably doesn\u0027t matter, but we may as well write the most accurate info we can?","commit_id":"26adb841520f0de30bad2c24a765b8d8ca72af66"}]}
