)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4d0a8374d473b35ea94288cb475d5351186360d6","unresolved":true,"context_lines":[{"line_number":11,"context_line":"PCI stats will need to know the compute node it\u0027s running on. Prepare"},{"line_number":12,"context_line":"for this by always passing node_id to the PCI manager. It was"},{"line_number":13,"context_line":"previously optional, but that looks to have been only to facilitate"},{"line_number":14,"context_line":"testing, as that\u0027s the only place where it was not passed it."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":17,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fe47e6e5_5c81b452","line":14,"updated":"2021-03-08 11:34:19.000000000","message":"yepp seem like it","commit_id":"d7ae8f5e79e317525adb2f5478635fe0cdbe4006"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cce1b683296dbe35bfcb168cb71211934c41d315","unresolved":true,"context_lines":[{"line_number":11,"context_line":"PCI stats will need to know the compute node it\u0027s running on. Prepare"},{"line_number":12,"context_line":"for this by replacing the node_id parameter with compute_node. Node_id"},{"line_number":13,"context_line":"was previously optional, but that looks to have been only to"},{"line_number":14,"context_line":"facilitate testing, as that\u0027s the only place where it was not passed"},{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"252f1b8b_c8b0ea69","line":14,"updated":"2021-03-09 21:18:25.000000000","message":"this is wrogn the compute node_id was a kwarg but it was required.\nin testin it could be ommited yes but in the resouce tracker it was always passed\nwe did not need to pass the compute node object here as we already had the id.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e5f6aa89ca7f550766dafa09fcb0c19c1992488f","unresolved":true,"context_lines":[{"line_number":11,"context_line":"PCI stats will need to know the compute node it\u0027s running on. Prepare"},{"line_number":12,"context_line":"for this by replacing the node_id parameter with compute_node. Node_id"},{"line_number":13,"context_line":"was previously optional, but that looks to have been only to"},{"line_number":14,"context_line":"facilitate testing, as that\u0027s the only place where it was not passed"},{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d7a44b64_d758155e","line":14,"in_reply_to":"252f1b8b_c8b0ea69","updated":"2021-03-10 08:34:10.000000000","message":"It was use in production code as mandatory but such mandatoryness was not enforced by the signature of the function.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"14ffa6012f816c8644c9439a2dd87cc303f92f4d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"PCI stats will need to know the compute node it\u0027s running on. Prepare"},{"line_number":12,"context_line":"for this by replacing the node_id parameter with compute_node. Node_id"},{"line_number":13,"context_line":"was previously optional, but that looks to have been only to"},{"line_number":14,"context_line":"facilitate testing, as that\u0027s the only place where it was not passed"},{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"dec06b52_5c31acfc","line":14,"in_reply_to":"d7a44b64_d758155e","updated":"2021-03-10 10:17:57.000000000","message":"Admittedly the wording here isn\u0027t great, but gibi and I both knew what artom meant from reading previous reviews (which were actually clearer iirc)","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fa79055e0c16f84d477365bcda7d9212e1d2dab4","unresolved":true,"context_lines":[{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"},{"line_number":18,"context_line":"needlessly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":21,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5bd4da5f_04d2cd37","line":18,"updated":"2021-03-09 13:19:12.000000000","message":"Thank you","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e5f6aa89ca7f550766dafa09fcb0c19c1992488f","unresolved":true,"context_lines":[{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"},{"line_number":18,"context_line":"needlessly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":21,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c78fb1a9_006ad2a5","line":18,"in_reply_to":"2de91eb7_10b2a378","updated":"2021-03-10 08:34:10.000000000","message":"I think smaller commits are easier to review.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cce1b683296dbe35bfcb168cb71211934c41d315","unresolved":true,"context_lines":[{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"},{"line_number":18,"context_line":"needlessly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":21,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2de91eb7_10b2a378","line":18,"in_reply_to":"5bd4da5f_04d2cd37","updated":"2021-03-09 21:18:25.000000000","message":"i would have preferfed that you did not do this optimisation in this change and only did it if needed\nin the latere change.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"11b0841c7d5e72bdeacf8202e9d71cc3194b41ba","unresolved":false,"context_lines":[{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"},{"line_number":18,"context_line":"needlessly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":21,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2da4ff60_72c277cb","line":18,"in_reply_to":"68553770_e15d5f4c","updated":"2021-03-10 11:11:02.000000000","message":"normally i would agree but not in this case\n\nif this had just made node_id a postional argumet i would have been ok with that.\nchanging it to a data type that is not used in this patch is what i dont like about this.\ni woudl have preferd that change to be done with the optimisation that used it.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"14ffa6012f816c8644c9439a2dd87cc303f92f4d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"it. We use compute_node (instead of just making node_id mandatory)"},{"line_number":16,"context_line":"because it allows for an optimization later on wherein the PCI manager"},{"line_number":17,"context_line":"does not need to pull the ComputeNode object from the database"},{"line_number":18,"context_line":"needlessly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements: blueprint pci-socket-affinity"},{"line_number":21,"context_line":"Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"68553770_e15d5f4c","line":18,"in_reply_to":"c78fb1a9_006ad2a5","updated":"2021-03-10 10:17:57.000000000","message":"As do I. I waited to +W this until I\u0027d looked at the following patches to see how it was used.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"}],"nova/pci/manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd5681d57b680d143d70e8d4fa2add5d62a266e3","unresolved":true,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        :param context: The request context."},{"line_number":58,"context_line":"        :param compute_node: The object.ComputeNode whose PCI devices we\u0027re"},{"line_number":59,"context_line":"                             tracking."},{"line_number":60,"context_line":"        \"\"\""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(PciDevTracker, self).__init__()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f32b89a_2b939691","line":59,"range":{"start_line":59,"start_character":13,"end_line":59,"end_character":29},"updated":"2021-03-09 17:20:35.000000000","message":"nit: you don\u0027t really need this","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"d5fef91823134186b322a432ee9248c041280f72","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        :param context: The request context."},{"line_number":58,"context_line":"        :param compute_node: The object.ComputeNode whose PCI devices we\u0027re"},{"line_number":59,"context_line":"                             tracking."},{"line_number":60,"context_line":"        \"\"\""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(PciDevTracker, self).__init__()"}],"source_content_type":"text/x-python","patch_set":2,"id":"0e5dd52a_173778b9","line":59,"range":{"start_line":59,"start_character":13,"end_line":59,"end_character":29},"in_reply_to":"5f32b89a_2b939691","updated":"2021-03-09 18:18:21.000000000","message":"Done","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd5681d57b680d143d70e8d4fa2add5d62a266e3","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(PciDevTracker, self).__init__()"},{"line_number":63,"context_line":"        self.stale \u003d {}"},{"line_number":64,"context_line":"        self.node_id \u003d compute_node.id"},{"line_number":65,"context_line":"        self.dev_filter \u003d whitelist.Whitelist(CONF.pci.passthrough_whitelist)"},{"line_number":66,"context_line":"        self.stats \u003d stats.PciDeviceStats(dev_filter\u003dself.dev_filter)"},{"line_number":67,"context_line":"        self._context \u003d context"}],"source_content_type":"text/x-python","patch_set":2,"id":"524cb9a0_0038f783","line":64,"updated":"2021-03-09 17:20:35.000000000","message":"was going to note that you\u0027re not actually using this, but that\u0027s coming in a follow-up as I now see noted in the commit message","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cce1b683296dbe35bfcb168cb71211934c41d315","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(PciDevTracker, self).__init__()"},{"line_number":63,"context_line":"        self.stale \u003d {}"},{"line_number":64,"context_line":"        self.node_id \u003d compute_node.id"},{"line_number":65,"context_line":"        self.dev_filter \u003d whitelist.Whitelist(CONF.pci.passthrough_whitelist)"},{"line_number":66,"context_line":"        self.stats \u003d stats.PciDeviceStats(dev_filter\u003dself.dev_filter)"},{"line_number":67,"context_line":"        self._context \u003d context"}],"source_content_type":"text/x-python","patch_set":2,"id":"a5be1384_a2c57674","line":64,"in_reply_to":"524cb9a0_0038f783","updated":"2021-03-09 21:18:25.000000000","message":"i dont think this should have been seprated out from the chagne that uses it.","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"}],"nova/tests/unit/pci/test_manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"dd5681d57b680d143d70e8d4fa2add5d62a266e3","unresolved":true,"context_lines":[{"line_number":251,"context_line":"        fake_pci_devs \u003d [fake_pci]"},{"line_number":252,"context_line":"        fake_pci_devs_json \u003d jsonutils.dumps(fake_pci_devs)"},{"line_number":253,"context_line":"        tracker \u003d manager.PciDevTracker("},{"line_number":254,"context_line":"            self.fake_context, objects.ComputeNode(id\u003d1))"},{"line_number":255,"context_line":"        # We expect that the device with 32bit PCI domain is ignored, so we\u0027ll"},{"line_number":256,"context_line":"        # have only the 3 original fake devs"},{"line_number":257,"context_line":"        tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)"}],"source_content_type":"text/x-python","patch_set":2,"id":"926b4658_56d925b3","line":254,"updated":"2021-03-09 17:20:35.000000000","message":"Can we get a check under this that there are 3 devices already here before the call to \u0027update_devices_from_hypervisor_resources\u0027. That would mean the comment below would make more sense","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"d5fef91823134186b322a432ee9248c041280f72","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        fake_pci_devs \u003d [fake_pci]"},{"line_number":252,"context_line":"        fake_pci_devs_json \u003d jsonutils.dumps(fake_pci_devs)"},{"line_number":253,"context_line":"        tracker \u003d manager.PciDevTracker("},{"line_number":254,"context_line":"            self.fake_context, objects.ComputeNode(id\u003d1))"},{"line_number":255,"context_line":"        # We expect that the device with 32bit PCI domain is ignored, so we\u0027ll"},{"line_number":256,"context_line":"        # have only the 3 original fake devs"},{"line_number":257,"context_line":"        tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fdabdcf0_d841662f","line":254,"in_reply_to":"926b4658_56d925b3","updated":"2021-03-09 18:18:21.000000000","message":"Done","commit_id":"6c3175d3ee8ffe323630f57655318ba3da9fe8b5"}]}