)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"096b000f1cd9cbd8a8bddab305d3021fa01d8b95","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Instead of calling _init_compute_node() in ResourceTracker.__init__()"},{"line_number":17,"context_line":"it is called near the beginning of update_available_resource()."},{"line_number":18,"context_line":"This has two purposes: it satisfies bug #1357453 by simplifying the"},{"line_number":19,"context_line":"update code and making the service lookup only happen once; it also"},{"line_number":20,"context_line":"creates a compute_node data structure up front."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"At the moment the update methods use a method parameter called"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9a80dd14_bd1209b2","line":19,"updated":"2015-03-12 13:59:58.000000000","message":"I am not sure this is true. We were doing the service lookup only in case there as no compute_node set on the RT object anyway.\n\nMaybe just drop the reference to the bug, since it specifically asks for creating the node only once and trusting that it\u0027s there.","commit_id":"b8a6f90afa51bf7c9d2a02631692735898e0fa2a"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"dde9fdb9c9ee6905ff0acf3b8d784d33406f7e42","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Instead of calling _init_compute_node() in ResourceTracker.__init__()"},{"line_number":17,"context_line":"it is called near the beginning of update_available_resource()."},{"line_number":18,"context_line":"This has two purposes: it satisfies bug #1357453 by simplifying the"},{"line_number":19,"context_line":"update code and making the service lookup only happen once; it also"},{"line_number":20,"context_line":"creates a compute_node data structure up front."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"At the moment the update methods use a method parameter called"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9a80dd14_c98b7a1c","line":19,"in_reply_to":"9a80dd14_bd1209b2","updated":"2015-03-12 16:15:20.000000000","message":"Yes, fair comment. I will leave it as related-to instead of fixes because there is relevant discussion on the bug report.","commit_id":"b8a6f90afa51bf7c9d2a02631692735898e0fa2a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2a1fa8aad9f01d9ba2bdafd11825a850df843c5a","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@redhat.com\u003e"},{"line_number":31,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":32,"context_line":"Co-Authored-By: Paul Murray \u003cpmurray@hp.com\u003e"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Related to bug #1357453"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"9a80dd14_cb697c26","line":32,"updated":"2015-03-19 16:49:43.000000000","message":"This line can be remove since your are the author.","commit_id":"648fa440d5bbf0909f83eb90214c543ef2a12263"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f264d7d8b7e18b88a1145b3d5f82009967b75a14","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@redhat.com\u003e"},{"line_number":31,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":32,"context_line":"Co-Authored-By: Paul Murray \u003cpmurray@hp.com\u003e"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Related to bug #1357453"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"1a6ced46_08482386","line":32,"updated":"2015-03-25 14:07:17.000000000","message":"As said in a previous patch this is redundant since you take the authorship of that patch.","commit_id":"e8d682ba44989d6570a1b21654d57ebbd3f1f992"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"3e785693d52ebac95441dff2ab52a5221355a727","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@redhat.com\u003e"},{"line_number":31,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":32,"context_line":"Co-Authored-By: Paul Murray \u003cpmurray@hp.com\u003e"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Related to bug #1357453"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"1a6ced46_ae60edcf","line":32,"in_reply_to":"1a6ced46_08482386","updated":"2015-03-25 17:41:01.000000000","message":"Yes it is redundant, but doesn\u0027t really matter - I\u0027ll change as I am changing the message anyway.","commit_id":"e8d682ba44989d6570a1b21654d57ebbd3f1f992"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"00be5eed8b95da44f6f75b72e6d976c0db9162be","unresolved":false,"context_lines":[{"line_number":31,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":32,"context_line":"Co-Authored-By: Paul Murray \u003cpmurray@hp.com\u003e"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Related to bug #1357453"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Change-Id: Ic04af76c3835a5bf63a42163d0335d6c7e26d68a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"1a6ced46_6b250c08","line":34,"updated":"2015-03-25 16:00:55.000000000","message":"Actually this fixes 1415768 and (_does not_ fix the linked bug IMHO) \n\nSee my comments on the proposed patch https://review.openstack.org/#/c/151528/\n\nLet\u0027s make sure we add it here and I\u0027m +2 on this landing for Kilo\n\nalso please remove the 135768","commit_id":"e8d682ba44989d6570a1b21654d57ebbd3f1f992"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"3e785693d52ebac95441dff2ab52a5221355a727","unresolved":false,"context_lines":[{"line_number":31,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":32,"context_line":"Co-Authored-By: Paul Murray \u003cpmurray@hp.com\u003e"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Related to bug #1357453"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Change-Id: Ic04af76c3835a5bf63a42163d0335d6c7e26d68a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"1a6ced46_ee84a55f","line":34,"in_reply_to":"1a6ced46_6b250c08","updated":"2015-03-25 17:41:01.000000000","message":"will do - fixing a bug without know eh! whatever next.\n\n(I think you mean remove 1357453 in the last line of your comment)","commit_id":"e8d682ba44989d6570a1b21654d57ebbd3f1f992"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a7c3349c26366b1bb9ba6544e817f676e2f0e63a","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":440,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":441,"context_line":"        # just update the record:"},{"line_number":442,"context_line":"        self._update(context, resources)"},{"line_number":443,"context_line":"        LOG.info(_LI(\u0027Compute_service record updated for %(host)s:%(node)s\u0027),"},{"line_number":444,"context_line":"                     {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_cd577d25","line":441,"updated":"2015-01-27 19:06:06.000000000","message":"This comment is superfluous. Actaully, you could just get rid of this method and change the calls to this to just call self._update instead.","commit_id":"63929914d070c61ac7cfb45e1ae3f23d85b972d2"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"7dd32f803e8b3a8f8490bbf20ef034b3a34629ea","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":440,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":441,"context_line":"        # just update the record:"},{"line_number":442,"context_line":"        self._update(context, resources)"},{"line_number":443,"context_line":"        LOG.info(_LI(\u0027Compute_service record updated for %(host)s:%(node)s\u0027),"},{"line_number":444,"context_line":"                     {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa81d914_d3c39e36","line":441,"in_reply_to":"fa81d914_cd577d25","updated":"2015-01-27 19:48:11.000000000","message":"I have had to add in an update to the host property - this came in from Sylvain\u0027s detatch service patches. I will address this comment in a subsequent patch if I can.","commit_id":"63929914d070c61ac7cfb45e1ae3f23d85b972d2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b1a051b723614964f2296203cdbe6391682f4efb","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.old_resources \u003d {}"},{"line_number":90,"context_line":"        self.scheduler_client \u003d scheduler_client.SchedulerClient()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_f902430a","line":92,"updated":"2015-01-27 20:55:00.000000000","message":"nit : Thinking about it, IMHO the method name is confusing : as this is called every period, it basically checks if it exists or not. \nAt least a docstring would be interesting to explain why you\u0027re returning None if self.compute_node is set","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"eec21c7ab2eedc51eacc93f3cd92d131afcb3ffc","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.old_resources \u003d {}"},{"line_number":90,"context_line":"        self.scheduler_client \u003d scheduler_client.SchedulerClient()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_8e1722a3","line":92,"in_reply_to":"fa81d914_67093a95","updated":"2015-01-28 08:57:53.000000000","message":"emm... Just create compute_node in the __init__ looks like possible also:\n\nresources \u003d self._get_driver_available_resource()\n\nself._verify_resources(resources)\n\nself._init_compute_node(context, resources)","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.old_resources \u003d {}"},{"line_number":90,"context_line":"        self.scheduler_client \u003d scheduler_client.SchedulerClient()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_3e4acc80","line":92,"in_reply_to":"fa81d914_8e1722a3","updated":"2015-01-28 22:18:56.000000000","message":"To Ed\u0027s comment: if update_usage() is called (L#295 below) it expects that compute_node already exists and contains the correct resource amounts. If it does not exist at that point it should bail (the lookup to compute_node is hidden in the disabled property). If compute_node is a property that creates itself in the getter it would create itself when it is looked up, which is not what we want. So although using a property is neat, I think it doesn\u0027t match this case.\n\nTo Alex\u0027s comment: it can go in the __init__, but if we don\u0027t have access to the database then it will not be created again. We will revisit how it gets created when the RT is reorganised for the ongoing scheduler work. At the moment update_available_resources() is called immediately after __init__() in the compute manager - it is also called periodically (although this is the part that may change in the future). For now I want to keep the possibility of creating it later when the database is inaccessible but later comes back up.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"824672c920dd8989dcab87a3db8c21dfccf764dd","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.old_resources \u003d {}"},{"line_number":90,"context_line":"        self.scheduler_client \u003d scheduler_client.SchedulerClient()"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_67093a95","line":92,"in_reply_to":"fa81d914_f902430a","updated":"2015-01-28 04:44:44.000000000","message":"I still think that making \u0027compute_node\u0027 a property that is loaded on first access makes the most sense.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b1a051b723614964f2296203cdbe6391682f4efb","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        # we need a copy of the ComputeNode record:"},{"line_number":98,"context_line":"        service \u003d self._get_service(context)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_c43672e7","line":95,"updated":"2015-01-27 20:55:00.000000000","message":"Now we have two cases when this method can return None (either if the compute_node field is already set, or if the service is not existing). I\u0027m wondering how we can make it more explicit by returning self.compute_node either way unless the service is not existing\n\n(and consequently maybe renaming the method like _get_compute_node() nope ?)","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        if self.compute_node is not None:"},{"line_number":95,"context_line":"            return"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        # we need a copy of the ComputeNode record:"},{"line_number":98,"context_line":"        service \u003d self._get_service(context)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_d12601ba","line":95,"in_reply_to":"fa81d914_c43672e7","updated":"2015-01-28 22:18:56.000000000","message":"I have changed this method around, but it does essentially the same thing - just removed unnecessary lines. I tried out returning a compute node but left it not returning a value. The point of the method is it has a side effect on compute_node so I didn\u0027t want to make it look like a pure function.\n\nI am willing to change the name but couldn\u0027t think of anything better - see comments revised version of the method.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"d15b0f3294fe106ede9dbecf693672615e6a09fa","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":390,"context_line":"            if not self.pci_tracker:"},{"line_number":391,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":392,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            devs \u003d []"},{"line_number":395,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_36377db2","line":392,"updated":"2015-01-28 01:25:33.000000000","message":"See comments at https://review.openstack.org/#/c/117042/9/nova/compute/resource_tracker.py.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":390,"context_line":"            if not self.pci_tracker:"},{"line_number":391,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":392,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            devs \u003d []"},{"line_number":395,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_5bb787d8","line":392,"in_reply_to":"fa81d914_36377db2","updated":"2015-01-28 22:18:56.000000000","message":"Done","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"824672c920dd8989dcab87a3db8c21dfccf764dd","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":390,"context_line":"            if not self.pci_tracker:"},{"line_number":391,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":392,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            devs \u003d []"},{"line_number":395,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_c76ca639","line":392,"in_reply_to":"fa81d914_36377db2","updated":"2015-01-28 04:44:44.000000000","message":"There is a race between this patch and https://review.openstack.org/145619, so no matter which gets merged first, this will be fixed.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b4089e161eccd89568230a5bce5cbda8712e8b2b","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":390,"context_line":"            if not self.pci_tracker:"},{"line_number":391,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":392,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            devs \u003d []"},{"line_number":395,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_61ef3888","line":392,"in_reply_to":"fa81d914_5bb787d8","updated":"2015-01-28 22:55:18.000000000","message":"And.... undone again - PciDevTracker does not take context as a parameter, only the node_id. So it failed tests that when I tried it. I will leave it as is and this should be dealt with in a separate patch - I think that is the one Ed is dealing with.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"824672c920dd8989dcab87a3db8c21dfccf764dd","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        resources[\u0027metrics\u0027] \u003d jsonutils.dumps(metrics)"},{"line_number":440,"context_line":"        self._sync_compute_node(context, resources)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":443,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":444,"context_line":"        # just update the record:"},{"line_number":445,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_279022ec","line":442,"updated":"2015-01-28 04:44:44.000000000","message":"I still think that this should be removed, and _update() be used instead. You can add the setting of the host there, too.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        resources[\u0027metrics\u0027] \u003d jsonutils.dumps(metrics)"},{"line_number":440,"context_line":"        self._sync_compute_node(context, resources)"},{"line_number":441,"context_line":""},{"line_number":442,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":443,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":444,"context_line":"        # just update the record:"},{"line_number":445,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_9b365f51","line":442,"in_reply_to":"fa81d914_279022ec","updated":"2015-01-28 22:18:56.000000000","message":"Agreed, will address in next follow-on - this will affect several tests and I will have to change them then - so minimising changes in this patch.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ac1c37dd40a00983d3e9eff76f09edf5c7357cb2","unresolved":false,"context_lines":[{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resoruces if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_32cb1620","line":308,"range":{"start_line":308,"start_character":39,"end_line":308,"end_character":48},"updated":"2015-02-06 18:33:09.000000000","message":"Resources is misspelled","commit_id":"db23005146f1d94403fa1196c3d45ecf86f989bd"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"0aec652a99d3c966d7c5e4c2a5e521eb9bf180bd","unresolved":false,"context_lines":[{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resoruces if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_58229652","line":308,"in_reply_to":"da86d52c_32cb1620","updated":"2015-02-12 14:55:22.000000000","message":"Done","commit_id":"db23005146f1d94403fa1196c3d45ecf86f989bd"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e5d5da26137a207d0dd23eaabfb1bd2ce99acded","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        cn.update(resources)"},{"line_number":328,"context_line":"        cn[\u0027service_id\u0027] \u003d service[\u0027id\u0027]"},{"line_number":329,"context_line":"        cn[\u0027host\u0027] \u003d self.host"},{"line_number":330,"context_line":"        self._create(context, cn)"},{"line_number":331,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":332,"context_line":"                     \u0027%(host)s:%(node)s\u0027),"},{"line_number":333,"context_line":"                 {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_73dc2774","line":330,"updated":"2015-02-24 16:12:03.000000000","message":"This is a little confusing, as you assign to self.compute_node elsewhere in the method, but here it looks as though it doesn\u0027t get assigned. I see that the _create() method does that, but it makes this code look inconsistent. I think this method and _create() should be a single point where self.compute_node is assigned, and the DB record created if necessary.","commit_id":"91aaaa1e6ce0955101211e90ebbdae9d49a97b8b"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"5c87682cc40f3dd42dae46758754c271f4dce975","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        cn.update(resources)"},{"line_number":328,"context_line":"        cn[\u0027service_id\u0027] \u003d service[\u0027id\u0027]"},{"line_number":329,"context_line":"        cn[\u0027host\u0027] \u003d self.host"},{"line_number":330,"context_line":"        self._create(context, cn)"},{"line_number":331,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":332,"context_line":"                     \u0027%(host)s:%(node)s\u0027),"},{"line_number":333,"context_line":"                 {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_644d383e","line":330,"in_reply_to":"ba7be1f8_73dc2774","updated":"2015-02-24 19:21:29.000000000","message":"Fair enough point and discussed on IRC too.\n\nInlining the create method here to make it clear that the compute_node is actually assigned. This part will be neatened up in a subsequent patch that swaps in the ComputeNode object and so removes the need for much of the json and dicts and conductor call etc.","commit_id":"91aaaa1e6ce0955101211e90ebbdae9d49a97b8b"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"72653967bbd9087de09594883c58ec9ec654249d","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    def _get_driver_available_resource(self):"},{"line_number":363,"context_line":"        resources \u003d self.driver.get_available_resource(self.nodename)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        if not resources:"},{"line_number":366,"context_line":"            # The virt driver does not support this function"},{"line_number":367,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":368,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_95ceb82c","line":365,"updated":"2015-02-25 12:19:08.000000000","message":"Not sure I understand why we move this logic into a private method. I don\u0027t think readability improves and also the check if resources was given now has to be evaluated twice.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"0e6889009b668737f7dfb641f2fc7f9d23fe52d3","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    def _get_driver_available_resource(self):"},{"line_number":363,"context_line":"        resources \u003d self.driver.get_available_resource(self.nodename)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        if not resources:"},{"line_number":366,"context_line":"            # The virt driver does not support this function"},{"line_number":367,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":368,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_9756b5cc","line":365,"in_reply_to":"ba7be1f8_03dc4f72","updated":"2015-02-25 15:07:34.000000000","message":"Re-merge sounds like it was already part of the code. As i see it this change is about to move it into a private method and I really don\u0027t see the point of it or why it\u0027s part of this change.\n\nHandling of the response is obviously still needed in update_available_resources().","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a927fa3963fc07533ff4852ac1c0797e5e4c4ce0","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    def _get_driver_available_resource(self):"},{"line_number":363,"context_line":"        resources \u003d self.driver.get_available_resource(self.nodename)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        if not resources:"},{"line_number":366,"context_line":"            # The virt driver does not support this function"},{"line_number":367,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":368,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_03dc4f72","line":365,"in_reply_to":"ba7be1f8_95ceb82c","updated":"2015-02-25 13:47:12.000000000","message":"At one point there were more than one place where this call to the driver\u0027s get_available_resource() was made, and they both had the check code for no resources. They were consolidated into a single method of the RT, and it\u0027s private because it isn\u0027t meant to be called from anywhere else.\n\nWith the latest refactoring, it no longer appears to be needed anywhere but in the update_available_resources() method. So while it could be re-merged back in without duplication, I prefer having the logic of querying the driver and handling its response in a separate method.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d7777c5d033f71f18c5baafbfdf98a63f32dd154","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    def _get_driver_available_resource(self):"},{"line_number":363,"context_line":"        resources \u003d self.driver.get_available_resource(self.nodename)"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"        if not resources:"},{"line_number":366,"context_line":"            # The virt driver does not support this function"},{"line_number":367,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":368,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_d96af8b1","line":365,"in_reply_to":"ba7be1f8_9756b5cc","updated":"2015-02-26 11:25:56.000000000","message":"Hi Hans, thanks for the comment. this was introduced in an earlier version of this patch because it was in fact used in multiple places - here and in the __init__() method. You are right, it is unnecessary now, so I will change it back (I expect that is what Ed meant). Well spotted.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"72653967bbd9087de09594883c58ec9ec654249d","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":418,"context_line":"            if not self.pci_tracker:"},{"line_number":419,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":420,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            devs \u003d []"},{"line_number":423,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_9532385d","line":420,"updated":"2015-02-25 12:19:08.000000000","message":"Why not provide this id when creating PciDevTracker and skip this call altogether?\n\n  self.pci_tracker \u003d pci_manager.PciDevTracker(node_id\u003dself.compute_node[\u0027id\u0027])\n\nI guess the only reason this call exists is because it couldn\u0027t be set at create time before.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"0e6889009b668737f7dfb641f2fc7f9d23fe52d3","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":418,"context_line":"            if not self.pci_tracker:"},{"line_number":419,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":420,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            devs \u003d []"},{"line_number":423,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_f70a9124","line":420,"in_reply_to":"ba7be1f8_03a28f98","updated":"2015-02-25 15:07:34.000000000","message":"...and we have it, so it should be passed at init no matter what we decide for the context.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"a927fa3963fc07533ff4852ac1c0797e5e4c4ce0","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":418,"context_line":"            if not self.pci_tracker:"},{"line_number":419,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":420,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            devs \u003d []"},{"line_number":423,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_03a28f98","line":420,"in_reply_to":"ba7be1f8_9532385d","updated":"2015-02-25 13:47:12.000000000","message":"There is also a bug that I have a patch for. You need to pass a context when creating the PciDevTracker. The compute node ID is optional, but it is cleaner to pass it at init if you have it.\n\nhttps://review.openstack.org/#/c/145619/10/nova/compute/resource_tracker.py,cm  Line 357","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"0da53796bd429dbdcc07175acb88238d2c0583b0","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":418,"context_line":"            if not self.pci_tracker:"},{"line_number":419,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":420,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            devs \u003d []"},{"line_number":423,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_f41dfa0f","line":420,"in_reply_to":"ba7be1f8_ed3c56ca","updated":"2015-02-26 15:01:58.000000000","message":"Ok, I get why it can\u0027t be done now.\nThanks","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"78210e40de154613411c70465dfc542cd1549315","unresolved":false,"context_lines":[{"line_number":417,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":418,"context_line":"            if not self.pci_tracker:"},{"line_number":419,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":420,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"            devs \u003d []"},{"line_number":423,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_ed3c56ca","line":420,"in_reply_to":"ba7be1f8_f70a9124","updated":"2015-02-26 13:01:07.000000000","message":"The PciDevTracker init method does not take a context - if you provide the node id it will blow up - that\u0027s the bug that Ed is referring to. See: https://bugs.launchpad.net/nova/+bug/1408480","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"021b7b2163d310324afcead674835b8bafd937e2","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        # so this can be removed when using ComputeNode."},{"line_number":333,"context_line":"        cn[\u0027stats\u0027] \u003d jsonutils.dumps(cn[\u0027stats\u0027])"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        self.compute_node \u003d self.conductor_api.compute_node_create(context, cn)"},{"line_number":336,"context_line":"        # NOTE(sbauza): We don\u0027t want to miss the first creation event"},{"line_number":337,"context_line":"        self._update_resource_stats(context, cn)"},{"line_number":338,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_ee3e0db7","line":335,"updated":"2015-02-26 14:11:20.000000000","message":"There should be short window, scheduler get the compute node status after this and before RT consume the resource usage. Emm...but looks like fine, there won\u0027t be something big problem.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d94f39da8ff9b16b8ae7965930e4054277cb339c","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        # so this can be removed when using ComputeNode."},{"line_number":333,"context_line":"        cn[\u0027stats\u0027] \u003d jsonutils.dumps(cn[\u0027stats\u0027])"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        self.compute_node \u003d self.conductor_api.compute_node_create(context, cn)"},{"line_number":336,"context_line":"        # NOTE(sbauza): We don\u0027t want to miss the first creation event"},{"line_number":337,"context_line":"        self._update_resource_stats(context, cn)"},{"line_number":338,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_64aa3186","line":335,"in_reply_to":"ba7be1f8_ee3e0db7","updated":"2015-02-26 18:13:24.000000000","message":"Yes, you are right. There was some discussion about this and it was decided this was ok for now (see bug report).","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"021b7b2163d310324afcead674835b8bafd937e2","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        self.compute_node \u003d self.conductor_api.compute_node_create(context, cn)"},{"line_number":336,"context_line":"        # NOTE(sbauza): We don\u0027t want to miss the first creation event"},{"line_number":337,"context_line":"        self._update_resource_stats(context, cn)"},{"line_number":338,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":339,"context_line":"                     \u0027%(host)s:%(node)s\u0027),"},{"line_number":340,"context_line":"                 {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_8e6d91ce","line":337,"updated":"2015-02-26 14:11:20.000000000","message":"Not sure we need this. There is update after consume resource usage. But this maybe not your patch\u0027s scope.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d94f39da8ff9b16b8ae7965930e4054277cb339c","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        self.compute_node \u003d self.conductor_api.compute_node_create(context, cn)"},{"line_number":336,"context_line":"        # NOTE(sbauza): We don\u0027t want to miss the first creation event"},{"line_number":337,"context_line":"        self._update_resource_stats(context, cn)"},{"line_number":338,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":339,"context_line":"                     \u0027%(host)s:%(node)s\u0027),"},{"line_number":340,"context_line":"                 {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_448e0d2f","line":337,"in_reply_to":"ba7be1f8_8e6d91ce","updated":"2015-02-26 18:13:24.000000000","message":"Absolutely agree - I plan to change this in subsequent patches, but as you say, not in scope for this patch.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2475c20d8ecb366370eb3f27cffae831445a8998","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":377,"context_line":"            self.compute_node \u003d None"},{"line_number":378,"context_line":"            return"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # We want the \u0027cpu_info\u0027 to be None from the POV of the"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_142e0ea5","line":379,"updated":"2015-02-26 15:05:07.000000000","message":"nit: unrelated whitespace","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d94f39da8ff9b16b8ae7965930e4054277cb339c","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":377,"context_line":"            self.compute_node \u003d None"},{"line_number":378,"context_line":"            return"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"},{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        # We want the \u0027cpu_info\u0027 to be None from the POV of the"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_c4791d30","line":379,"in_reply_to":"ba7be1f8_142e0ea5","updated":"2015-02-26 18:13:24.000000000","message":"Done","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"021b7b2163d310324afcead674835b8bafd937e2","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        # exist."},{"line_number":405,"context_line":"        self._init_compute_node(context, resources)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        if self.disabled:"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_5185f857","line":407,"updated":"2015-02-26 14:11:20.000000000","message":"emm....this is needless? after execute _init_compute_node, the \"self.compute_node is None\" must be false?","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"67913c83b6a8e0645699d8e53e8c5d8e108edcf4","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        # exist."},{"line_number":405,"context_line":"        self._init_compute_node(context, resources)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        if self.disabled:"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_44850aac","line":407,"in_reply_to":"ba7be1f8_44952d31","updated":"2015-02-27 11:09:04.000000000","message":"Emm.....Can I ask when the service can be not present?","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"186834e5431b50560a2a2c64d4f0eb97879c720a","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        # exist."},{"line_number":405,"context_line":"        self._init_compute_node(context, resources)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        if self.disabled:"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_6baa0486","line":407,"in_reply_to":"ba7be1f8_5185f857","updated":"2015-02-26 17:06:45.000000000","message":"Alex: no, if the service is not present (line 311), then the compute node will not be created, and disabled will be True.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2475c20d8ecb366370eb3f27cffae831445a8998","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        # exist."},{"line_number":405,"context_line":"        self._init_compute_node(context, resources)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        if self.disabled:"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_74690af8","line":407,"in_reply_to":"ba7be1f8_5185f857","updated":"2015-02-26 15:05:07.000000000","message":"yea, looks true.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d94f39da8ff9b16b8ae7965930e4054277cb339c","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        # exist."},{"line_number":405,"context_line":"        self._init_compute_node(context, resources)"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        if self.disabled:"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_44952d31","line":407,"in_reply_to":"ba7be1f8_6baa0486","updated":"2015-02-26 18:13:24.000000000","message":"Added comment to avoid confusion","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2475c20d8ecb366370eb3f27cffae831445a8998","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":411,"context_line":"            if not self.pci_tracker:"},{"line_number":412,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":413,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"            devs \u003d []"},{"line_number":416,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_b79b8464","line":413,"updated":"2015-02-26 15:05:07.000000000","message":"Since you remove the comment in the method about its purpose, maybe put a todo here saying node id can be set in PciDevTracker init after https://bugs.launchpad.net/nova/+bug/1408480 is solved.","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d94f39da8ff9b16b8ae7965930e4054277cb339c","unresolved":false,"context_lines":[{"line_number":410,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"},{"line_number":411,"context_line":"            if not self.pci_tracker:"},{"line_number":412,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker()"},{"line_number":413,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"            devs \u003d []"},{"line_number":416,"context_line":"            for dev in jsonutils.loads(resources.pop("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_24b919cc","line":413,"in_reply_to":"ba7be1f8_b79b8464","updated":"2015-02-26 18:13:24.000000000","message":"Done","commit_id":"2091acce1ef40f843a3430d937a5516d1ebd7c1f"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"17278ae2ad7625456062ee2bc4d20a0ed0bb302c","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        # so we need to create a new compute node. This needs"},{"line_number":323,"context_line":"        # to be initialised with resource values."},{"line_number":324,"context_line":"        cn \u003d {}"},{"line_number":325,"context_line":"        cn.update(resources)"},{"line_number":326,"context_line":"        cn[\u0027service_id\u0027] \u003d service[\u0027id\u0027]"},{"line_number":327,"context_line":"        cn[\u0027host\u0027] \u003d self.host"},{"line_number":328,"context_line":"        # initialize load stats from existing instances:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_492a8fe8","line":325,"updated":"2015-02-27 06:22:28.000000000","message":"pci_passthrough_devices should not be added from resources - it is not stored in the compute node. See L #411-426","commit_id":"9519e9327ac4aa858be8a023bf061c644be33f2a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f49adf6545530216cc612b6a92a6ea58cbc89201","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        if not resources:"},{"line_number":377,"context_line":"            # The virt driver does not support this function"},{"line_number":378,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":379,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":380,"context_line":"            self.compute_node \u003d None"},{"line_number":381,"context_line":"            return"},{"line_number":382,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_bc196b24","line":379,"updated":"2015-02-27 17:08:13.000000000","message":"nit: not necessary","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"9ae11f5a92b3201f1d4ad46e2452f488fdbf668c","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        self._update_available_resource(context, resources)"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE)"},{"line_number":403,"context_line":"    def _update_available_resource(self, context, resources):"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"        # initialise the compute node object, creating it"},{"line_number":406,"context_line":"        # if it does not already exist."}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_34cc6761","line":403,"updated":"2015-03-02 18:28:47.000000000","message":"Nit: blank line is not useful here.","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"dde9fdb9c9ee6905ff0acf3b8d784d33406f7e42","unresolved":false,"context_lines":[{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        self.compute_node \u003d self.conductor_api.compute_node_create(context, cn)"},{"line_number":339,"context_line":"        # NOTE(sbauza): We don\u0027t want to miss the first creation event"},{"line_number":340,"context_line":"        self._update_resource_stats(context, cn)"},{"line_number":341,"context_line":"        LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":342,"context_line":"                     \u0027%(host)s:%(node)s\u0027),"},{"line_number":343,"context_line":"                 {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a80dd14_32956040","line":340,"updated":"2015-03-12 16:15:20.000000000","message":"This call to self._update_resource_stats() is not needed and not really wanted either. There will be a subsequent call when the calculations have been done. Previously this call was necessary when this code section came at the end because there was no subsequent update.","commit_id":"b8a6f90afa51bf7c9d2a02631692735898e0fa2a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"096b000f1cd9cbd8a8bddab305d3021fa01d8b95","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        resources[\u0027metrics\u0027] \u003d jsonutils.dumps(metrics)"},{"line_number":470,"context_line":"        self._sync_compute_node(context, resources)"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":473,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":474,"context_line":"        # just update the record:"},{"line_number":475,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a80dd14_b953fe37","line":472,"updated":"2015-03-12 13:59:58.000000000","message":"Why do we need this method now? we call it in one place - just inline it","commit_id":"b8a6f90afa51bf7c9d2a02631692735898e0fa2a"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"dde9fdb9c9ee6905ff0acf3b8d784d33406f7e42","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        resources[\u0027metrics\u0027] \u003d jsonutils.dumps(metrics)"},{"line_number":470,"context_line":"        self._sync_compute_node(context, resources)"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":473,"context_line":"        \"\"\"Update the compute node DB record.\"\"\""},{"line_number":474,"context_line":"        # just update the record:"},{"line_number":475,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a80dd14_f25828fc","line":472,"in_reply_to":"9a80dd14_b953fe37","updated":"2015-03-12 16:15:20.000000000","message":"Yes, I do that in a later patch, but may was well do it here.","commit_id":"b8a6f90afa51bf7c9d2a02631692735898e0fa2a"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2f5cfefde9db4cc4609f90cf59c7834579768f8f","unresolved":false,"context_lines":[{"line_number":400,"context_line":"    def _sync_compute_node(self, context, resources):"},{"line_number":401,"context_line":"        \"\"\"Create or update the compute node DB record.\"\"\""},{"line_number":402,"context_line":"        if not self.compute_node:"},{"line_number":403,"context_line":"            # we need a copy of the ComputeNode record:"},{"line_number":404,"context_line":"            service \u003d self._get_service(context)"},{"line_number":405,"context_line":"            if not service:"},{"line_number":406,"context_line":"                # no service record, disable resource"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_84b2b704","side":"PARENT","line":403,"updated":"2015-03-16 21:36:27.000000000","message":"Here it rather says it\u0027s purpose is to get the compute node.","commit_id":"3ae32a84b12bbbef4b29947e8e51c943c313fc80"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2f5cfefde9db4cc4609f90cf59c7834579768f8f","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resources if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"},{"line_number":312,"context_line":"            # no service record, disable resource"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_84247761","line":309,"updated":"2015-03-16 21:36:27.000000000","message":"Is the purpose of this call really meant to be a sanity check? The original comment says something else (see comment further down).\n\nFrom commit history it can be seen that the compute node record initially was fetched together with the service, using the relationship between the two. Since we now look up the compute node directly using host/node I would argue we don\u0027t need this call anymore.\n\nDo we need it as a sanity check? When a nova-compute service starts up, it will look for an existing service record and if not found will create one before moving on. This code is executed by the service, so having no db record at this point shouldn\u0027t really be possible, right?","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"c5b166709fd1b327c22eab7e16e992db6c32a94a","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resources if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"},{"line_number":312,"context_line":"            # no service record, disable resource"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_4394038c","line":309,"in_reply_to":"9a80dd14_44397640","updated":"2015-03-17 18:33:33.000000000","message":"I totally agree, we still need to look it up to fill in the service_id. I just didn\u0027t want the comment to state new reasons for its existens.\n\nWhat I don\u0027t agree about is the sanity thing. The service must be up and registered in the database or else this code won\u0027t be running. Consistency is of course important and since self.host \u003d\u003d service.host, compute.host must be correct too even without having to look up the service.","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e6c82ef09b1c8eb0675b7a38342596e41589a2c2","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resources if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"},{"line_number":312,"context_line":"            # no service record, disable resource"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_44397640","line":309,"in_reply_to":"9a80dd14_84247761","updated":"2015-03-17 15:38:06.000000000","message":"Hans, we have an existing behaviour where we need to make sure that a compute node still is related to an existing service. There can be N compute nodes per service (eg. for Ironic, there is a nova-compute service having N nodes (each having a nodename equals to an Ironic UUID).\n\nAs the virt driver is reporting the list of compute nodes, we need to have a check (call it sanity) to make sure that the corresponding RPC service is up (because when it\u0027s up, it creates a DB entry).\n\nThat said, now that bp detach-service-from-computenode is now fully implemented, that\u0027s the only logic that we need to care about. There is no longer any need to add the service information, we just need to make sure that compute.host and service.host fields are consistent.\n\nPlease note that the old service_id field set at L326 is now deprecated (because we first check by looking at the host field) and will be removed in a next patch series related to that. In the meantime, Paul can just add a comment on top of L326 saying that service_id is now deprecated and will be removed later on.\n\nCaution, we can\u0027t easily remove that line because it first needs to bump a new Service object version as service_id is still considered as a mandatory field.","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"ac403d9a1e1bf03c11c3af760291452311b30c5f","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        # we need to go to the database, so check that we"},{"line_number":307,"context_line":"        # have a service defined. This is really a sanity"},{"line_number":308,"context_line":"        # check, don\u0027t start reporting resources if the"},{"line_number":309,"context_line":"        # service is not registered in the database."},{"line_number":310,"context_line":"        service \u003d self._get_service(context)"},{"line_number":311,"context_line":"        if not service:"},{"line_number":312,"context_line":"            # no service record, disable resource"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_c4a8067a","line":309,"in_reply_to":"9a80dd14_84247761","updated":"2015-03-17 15:37:56.000000000","message":"You\u0027re right, this should be going away - however, for now we need the service to assign service_id below. I will keep the service lookup while the service_id field gets deprecated, but I will add a comment to say it should be going away.","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e6c82ef09b1c8eb0675b7a38342596e41589a2c2","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        # to be initialised with resource values."},{"line_number":324,"context_line":"        cn \u003d {}"},{"line_number":325,"context_line":"        cn.update(resources)"},{"line_number":326,"context_line":"        cn[\u0027service_id\u0027] \u003d service[\u0027id\u0027]"},{"line_number":327,"context_line":"        cn[\u0027host\u0027] \u003d self.host"},{"line_number":328,"context_line":"        # initialize load stats from existing instances:"},{"line_number":329,"context_line":"        self._write_ext_resources(cn)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_a41eb2b9","line":326,"updated":"2015-03-17 15:38:06.000000000","message":"erm, please change to dot notation, here you get a Service object","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"8c86fbaad8ddbadbc85dd0d7aaccfa76928c3bbd","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        # to be initialised with resource values."},{"line_number":324,"context_line":"        cn \u003d {}"},{"line_number":325,"context_line":"        cn.update(resources)"},{"line_number":326,"context_line":"        cn[\u0027service_id\u0027] \u003d service[\u0027id\u0027]"},{"line_number":327,"context_line":"        cn[\u0027host\u0027] \u003d self.host"},{"line_number":328,"context_line":"        # initialize load stats from existing instances:"},{"line_number":329,"context_line":"        self._write_ext_resources(cn)"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_2411e25d","line":326,"in_reply_to":"9a80dd14_a41eb2b9","updated":"2015-03-17 15:42:49.000000000","message":"will do","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e6c82ef09b1c8eb0675b7a38342596e41589a2c2","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        if not resources:"},{"line_number":377,"context_line":"            # The virt driver does not support this function"},{"line_number":378,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":379,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":380,"context_line":"            self.compute_node \u003d None"},{"line_number":381,"context_line":"            return"},{"line_number":382,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_697573cb","line":379,"updated":"2015-03-17 15:38:06.000000000","message":"nit : not necessary but that\u0027s fine","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"87bcb0ea7c61811eda5283e9b0ca2800568c240d","unresolved":false,"context_lines":[{"line_number":377,"context_line":"        if not resources:"},{"line_number":378,"context_line":"            # The virt driver does not support this function"},{"line_number":379,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":380,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":381,"context_line":"            self.compute_node \u003d None"},{"line_number":382,"context_line":"            return"},{"line_number":383,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_e0b3044b","line":380,"updated":"2015-03-18 13:19:49.000000000","message":"would it be possible to align the \" if you need to repsin","commit_id":"16be9a5bbd7e4bfdd51212ba1a8a16619b935e3b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2a1fa8aad9f01d9ba2bdafd11825a850df843c5a","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        return self.compute_node is None"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _init_compute_node(self, context, resources):"},{"line_number":287,"context_line":"        \"\"\"Initialise the compute node if it does not already exist."},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"        The resource tracker will be inoperable if compute_node"},{"line_number":290,"context_line":"        is not defined. The compute_node will remain undefined if"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_8b6ea4fe","line":287,"updated":"2015-03-19 16:49:43.000000000","message":"s/Initialise/Initialize","commit_id":"648fa440d5bbf0909f83eb90214c543ef2a12263"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2a1fa8aad9f01d9ba2bdafd11825a850df843c5a","unresolved":false,"context_lines":[{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        # if we could not init the compute node the tracker will be"},{"line_number":411,"context_line":"        # disabled and we should quit now"},{"line_number":412,"context_line":"        if self.disabled:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_c6e33303","line":412,"updated":"2015-03-19 16:49:43.000000000","message":"actually this disabled property looks really weird for me, it does not make sense since we do not know when used whether an attempt to initialize compute_node has been done. (in this context we know, but in other methods...) we should keep using\n\n  if [not] self.compute_node:\n     ...","commit_id":"648fa440d5bbf0909f83eb90214c543ef2a12263"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b5fcf555cef92416719249a68eb0f10f2d603b35","unresolved":false,"context_lines":[{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        # if there is already a compute node we don\u0027t"},{"line_number":302,"context_line":"        # need to do anything"},{"line_number":303,"context_line":"        if self.compute_node:"},{"line_number":304,"context_line":"            return"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # TODO(pmurray): this lookup should be removed when the service_id"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_955ae281","line":303,"updated":"2015-03-23 07:34:05.000000000","message":"consider adding in a log debug here. it may be helpful with debugging","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"718409c4ad3ff878f002ccb6531f4c19038dc8c2","unresolved":false,"context_lines":[{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        # if there is already a compute node we don\u0027t"},{"line_number":302,"context_line":"        # need to do anything"},{"line_number":303,"context_line":"        if self.compute_node:"},{"line_number":304,"context_line":"            return"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # TODO(pmurray): this lookup should be removed when the service_id"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_bed176cf","line":303,"in_reply_to":"1a6ced46_955ae281","updated":"2015-03-23 10:46:59.000000000","message":"Mmm, the previous behaviour was not including those logs, I think it\u0027s a good option but can be done later in another patch","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b5fcf555cef92416719249a68eb0f10f2d603b35","unresolved":false,"context_lines":[{"line_number":308,"context_line":"        # but still a required field, so it has to be assigned below."},{"line_number":309,"context_line":"        service \u003d self._get_service(context)"},{"line_number":310,"context_line":"        if not service:"},{"line_number":311,"context_line":"            # no service record, disable resource"},{"line_number":312,"context_line":"            return"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"        # now try to get the compute node record from the"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_d5537a6a","line":311,"updated":"2015-03-23 07:34:05.000000000","message":"ditto","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b5fcf555cef92416719249a68eb0f10f2d603b35","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        # now try to get the compute node record from the"},{"line_number":315,"context_line":"        # database. If we get one we are done."},{"line_number":316,"context_line":"        self.compute_node \u003d self._get_compute_node(context)"},{"line_number":317,"context_line":"        if self.compute_node:"},{"line_number":318,"context_line":"            return"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        # there was no local copy and none in the database"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_354beeb1","line":317,"updated":"2015-03-23 07:34:05.000000000","message":"ditto","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b5fcf555cef92416719249a68eb0f10f2d603b35","unresolved":false,"context_lines":[{"line_number":377,"context_line":"        if not resources:"},{"line_number":378,"context_line":"            # The virt driver does not support this function"},{"line_number":379,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":380,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":381,"context_line":"            self.compute_node \u003d None"},{"line_number":382,"context_line":"            return"},{"line_number":383,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_b5bb5e71","line":380,"updated":"2015-03-23 07:34:05.000000000","message":"this is kind of unrelated to this patch set.","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"718409c4ad3ff878f002ccb6531f4c19038dc8c2","unresolved":false,"context_lines":[{"line_number":377,"context_line":"        if not resources:"},{"line_number":378,"context_line":"            # The virt driver does not support this function"},{"line_number":379,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":380,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":381,"context_line":"            self.compute_node \u003d None"},{"line_number":382,"context_line":"            return"},{"line_number":383,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_9ef3da72","line":380,"in_reply_to":"1a6ced46_b5bb5e71","updated":"2015-03-23 10:46:59.000000000","message":"Agreed, but that\u0027s not need to see a -1 for it","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b5fcf555cef92416719249a68eb0f10f2d603b35","unresolved":false,"context_lines":[{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        # if we could not init the compute node the tracker will be"},{"line_number":411,"context_line":"        # disabled and we should quit now"},{"line_number":412,"context_line":"        if self.disabled:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_35904eec","line":412,"updated":"2015-03-23 07:34:05.000000000","message":"should this not be:\nif (self.compute_node is None or\n    self.disabled):\n    return","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"346c1ff1923a99b8384a3d7096f874c522da7420","unresolved":false,"context_lines":[{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        # if we could not init the compute node the tracker will be"},{"line_number":411,"context_line":"        # disabled and we should quit now"},{"line_number":412,"context_line":"        if self.disabled:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_58609f35","line":412,"in_reply_to":"1a6ced46_35904eec","updated":"2015-03-23 08:03:41.000000000","message":"self.disable means self.compute_node is None - one of my previous comment was about to remove this property.","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"718409c4ad3ff878f002ccb6531f4c19038dc8c2","unresolved":false,"context_lines":[{"line_number":409,"context_line":""},{"line_number":410,"context_line":"        # if we could not init the compute node the tracker will be"},{"line_number":411,"context_line":"        # disabled and we should quit now"},{"line_number":412,"context_line":"        if self.disabled:"},{"line_number":413,"context_line":"            return"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        if \u0027pci_passthrough_devices\u0027 in resources:"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a6ced46_1effea35","line":412,"in_reply_to":"1a6ced46_58609f35","updated":"2015-03-23 10:46:59.000000000","message":"Agreed, the disabled property is actually checking this.","commit_id":"2f21c14e815ae79d32abf615d5a4adcfe641a818"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a5c16af1cfe14df521377f85cca9adf5441564c7","unresolved":false,"context_lines":[{"line_number":418,"context_line":"            resources[\u0027host\u0027] \u003d self.host"},{"line_number":419,"context_line":"            self._create(context, resources)"},{"line_number":420,"context_line":"            if self.pci_tracker:"},{"line_number":421,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":422,"context_line":"            LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":423,"context_line":"                         \u0027%(host)s:%(node)s\u0027),"},{"line_number":424,"context_line":"                     {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a6ced46_22340992","side":"PARENT","line":421,"updated":"2015-03-26 16:08:24.000000000","message":"Why was this removed from the refactored code?","commit_id":"d67cee9eea6a9921b5ee5dfacb4507785f5ddafe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b7969247229f67cbedb55364f9a9f60b6d4d111d","unresolved":false,"context_lines":[{"line_number":418,"context_line":"            resources[\u0027host\u0027] \u003d self.host"},{"line_number":419,"context_line":"            self._create(context, resources)"},{"line_number":420,"context_line":"            if self.pci_tracker:"},{"line_number":421,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":422,"context_line":"            LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":423,"context_line":"                         \u0027%(host)s:%(node)s\u0027),"},{"line_number":424,"context_line":"                     {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_6cbe2401","side":"PARENT","line":421,"in_reply_to":"1a6ced46_22340992","updated":"2015-03-26 16:15:43.000000000","message":"We never need to set it since it can\u0027t be none ever again, and it will always be passed in when creating the tracker.\n\nActually it should probably be made a positional instead of a kwarg there.\n\nActually it was the source for the bug linked in the commit - so it\u0027s _really_ good that it\u0027s gone as it\u0027s one of the design issues with the pci tracker.","commit_id":"d67cee9eea6a9921b5ee5dfacb4507785f5ddafe"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"8787a3e74a1fb67e5153dd723fa8b47cdbb2e3f7","unresolved":false,"context_lines":[{"line_number":418,"context_line":"            resources[\u0027host\u0027] \u003d self.host"},{"line_number":419,"context_line":"            self._create(context, resources)"},{"line_number":420,"context_line":"            if self.pci_tracker:"},{"line_number":421,"context_line":"                self.pci_tracker.set_compute_node_id(self.compute_node[\u0027id\u0027])"},{"line_number":422,"context_line":"            LOG.info(_LI(\u0027Compute_service record created for \u0027"},{"line_number":423,"context_line":"                         \u0027%(host)s:%(node)s\u0027),"},{"line_number":424,"context_line":"                     {\u0027host\u0027: self.host, \u0027node\u0027: self.nodename})"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_13c5493b","side":"PARENT","line":421,"in_reply_to":"fa963961_6cbe2401","updated":"2015-03-27 18:16:33.000000000","message":"Before the change, self.pci_tracker is created at the start of _update_available_resource() before compute_node exists. This section of code was executed at the end of _update_available_resource() and creates the compute_node, so it is the first time the id is available, so it sets the id in the pci_tracker.\n\nAfter this change this section of code comes at the start of _update_available_resource() before the pci_tracker has been created. So it does not set the id in the pci_tracker. Creation of the pci_tracker comes next in _update_available_resource() and the id is set there.","commit_id":"d67cee9eea6a9921b5ee5dfacb4507785f5ddafe"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"30d93e8177e3c037a76445d7d3d29dc9e09f36d3","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        if not resources:"},{"line_number":379,"context_line":"            # The virt driver does not support this function"},{"line_number":380,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":381,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":382,"context_line":"            self.compute_node \u003d None"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_6e55f4c4","line":381,"updated":"2015-03-26 18:27:44.000000000","message":"Is this change related?","commit_id":"c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"8787a3e74a1fb67e5153dd723fa8b47cdbb2e3f7","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        if not resources:"},{"line_number":379,"context_line":"            # The virt driver does not support this function"},{"line_number":380,"context_line":"            LOG.info(_LI(\"Virt driver does not support \""},{"line_number":381,"context_line":"                 \"\u0027get_available_resource\u0027. Compute tracking is disabled.\"))"},{"line_number":382,"context_line":"            self.compute_node \u003d None"},{"line_number":383,"context_line":"            return"},{"line_number":384,"context_line":"        resources[\u0027host_ip\u0027] \u003d CONF.my_ip"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_e1ee8e8a","line":381,"in_reply_to":"fa963961_6e55f4c4","updated":"2015-03-27 18:16:33.000000000","message":"No, it was just there when I took the patch over and I never dropped it. Someone clearly objects to incorrect grammar. I\u0027ll drop it if another change has to be made.","commit_id":"c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"c5d428097319bd52cd145b96b1cc13037e05d369","unresolved":false,"context_lines":[{"line_number":423,"context_line":"                if self.pci_filter.device_assignable(dev):"},{"line_number":424,"context_line":"                    devs.append(dev)"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"            if not self.pci_tracker:"},{"line_number":427,"context_line":"                n_id \u003d self.compute_node[\u0027id\u0027] if self.compute_node else None"},{"line_number":428,"context_line":"                self.pci_tracker \u003d pci_manager.PciDevTracker(context,"},{"line_number":429,"context_line":"                                                             node_id\u003dn_id)"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_281a4f6f","line":426,"updated":"2015-03-26 14:00:32.000000000","message":"nit: seems that the check is not necessary any more. See line 413.","commit_id":"c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13"}],"nova/pci/manager.py":[{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"d15b0f3294fe106ede9dbecf693672615e6a09fa","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                    self._free_device(dev)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def set_compute_node_id(self, node_id):"},{"line_number":255,"context_line":"        \"\"\"Set the compute node id that this object is tracking for.\"\"\""},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        if self.node_id and self.node_id !\u003d node_id:"},{"line_number":258,"context_line":"            raise exception.PciTrackerInvalidNodeId(node_id\u003dself.node_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_96f829e7","line":255,"updated":"2015-01-28 01:25:33.000000000","message":"see comments here:https://review.openstack.org/#/c/117042/9/nova/pci/manager.py\n\nAny feedback on that?","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b4089e161eccd89568230a5bce5cbda8712e8b2b","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                    self._free_device(dev)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def set_compute_node_id(self, node_id):"},{"line_number":255,"context_line":"        \"\"\"Set the compute node id that this object is tracking for.\"\"\""},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        if self.node_id and self.node_id !\u003d node_id:"},{"line_number":258,"context_line":"            raise exception.PciTrackerInvalidNodeId(node_id\u003dself.node_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_81c0ace1","line":255,"in_reply_to":"fa81d914_1b30af00","updated":"2015-01-28 22:55:18.000000000","message":"Actually - leaving alone - the constructor does not take a context so using that with node_id failed tests.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                    self._free_device(dev)"},{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def set_compute_node_id(self, node_id):"},{"line_number":255,"context_line":"        \"\"\"Set the compute node id that this object is tracking for.\"\"\""},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        if self.node_id and self.node_id !\u003d node_id:"},{"line_number":258,"context_line":"            raise exception.PciTrackerInvalidNodeId(node_id\u003dself.node_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_1b30af00","line":255,"in_reply_to":"fa81d914_96f829e7","updated":"2015-01-28 22:18:56.000000000","message":"Done - removed associated test as well.","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"2f5cfefde9db4cc4609f90cf59c7834579768f8f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                for dev in devs:"},{"line_number":267,"context_line":"                    self._free_device(dev)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def set_compute_node_id(self, node_id):"},{"line_number":270,"context_line":"        \"\"\"Set the compute node id that this object is tracking for.\"\"\""},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if self.node_id and self.node_id !\u003d node_id:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_893e50d9","line":269,"updated":"2015-03-16 21:36:27.000000000","message":"Nothing is calling this method anymore so just remove it.","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"ac403d9a1e1bf03c11c3af760291452311b30c5f","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                for dev in devs:"},{"line_number":267,"context_line":"                    self._free_device(dev)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def set_compute_node_id(self, node_id):"},{"line_number":270,"context_line":"        \"\"\"Set the compute node id that this object is tracking for.\"\"\""},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"        if self.node_id and self.node_id !\u003d node_id:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_44025670","line":269,"in_reply_to":"9a80dd14_893e50d9","updated":"2015-03-17 15:37:56.000000000","message":"will do","commit_id":"147048ae11c7e10afbb3c673b761985cb32a5b39"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a5c16af1cfe14df521377f85cca9adf5441564c7","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                                                    new_node_id\u003dnode_id)"},{"line_number":281,"context_line":"        self.node_id \u003d node_id"},{"line_number":282,"context_line":"        for dev in self.pci_devs:"},{"line_number":283,"context_line":"            dev.compute_node_id \u003d node_id"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"def get_instance_pci_devs(inst, request_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a6ced46_82221d50","side":"PARENT","line":283,"updated":"2015-03-26 16:08:24.000000000","message":"Why was this removed?","commit_id":"d67cee9eea6a9921b5ee5dfacb4507785f5ddafe"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"b7969247229f67cbedb55364f9a9f60b6d4d111d","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                                                    new_node_id\u003dnode_id)"},{"line_number":281,"context_line":"        self.node_id \u003d node_id"},{"line_number":282,"context_line":"        for dev in self.pci_devs:"},{"line_number":283,"context_line":"            dev.compute_node_id \u003d node_id"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"def get_instance_pci_devs(inst, request_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa963961_0c6318b3","side":"PARENT","line":283,"in_reply_to":"1a6ced46_82221d50","updated":"2015-03-26 16:15:43.000000000","message":"See my comment on resource_tracker.py","commit_id":"d67cee9eea6a9921b5ee5dfacb4507785f5ddafe"}],"nova/tests/unit/compute/test_resource_tracker.py":[{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"9539c651641e936b65163b0430bdc2fde8f95fbe","unresolved":false,"context_lines":[{"line_number":723,"context_line":"        super(SchedulerClientTrackerTestCase, self).setUp()"},{"line_number":724,"context_line":"        self.tracker.scheduler_client.update_resource_stats \u003d mock.Mock()"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    def test_create_resource(self):"},{"line_number":727,"context_line":"        self.tracker._write_ext_resources \u003d mock.Mock()"},{"line_number":728,"context_line":"        self.tracker.conductor_api.compute_node_create \u003d mock.Mock("},{"line_number":729,"context_line":"            return_value\u003ddict(id\u003d1))"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_e4b848ca","side":"PARENT","line":726,"updated":"2015-02-24 19:27:56.000000000","message":"Note: the RT _create() method has been removed and the code inlined in _init_compute_node(). There are already tests to check update_resource_stats() is called when a compute node is created in tests for _init_compute_node(). So this can just be dropped.","commit_id":"7f7ecb2f634f9155436bc01a3b279d62aa59a693"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"6cd7cc8172eaa3a73c1e2e833d2d0eafcaa348f9","unresolved":false,"context_lines":[{"line_number":721,"context_line":""},{"line_number":722,"context_line":"    def setUp(self):"},{"line_number":723,"context_line":"        super(SchedulerClientTrackerTestCase, self).setUp()"},{"line_number":724,"context_line":"        self.tracker.scheduler_client.update_resource_stats \u003d mock.Mock()"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    def test_update_resource(self):"},{"line_number":727,"context_line":"        self.tracker._write_ext_resources \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_c4718445","line":724,"updated":"2015-02-24 19:30:06.000000000","message":"Shouldn\u0027t the TestInitComputeNode class be here instead of test_tracker.py? (fwiw, I don\u0027t really understand the split of these two scripts, but...)","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"2ea3ec1ef05220ade1e72755d318d280b66eac4e","unresolved":false,"context_lines":[{"line_number":721,"context_line":""},{"line_number":722,"context_line":"    def setUp(self):"},{"line_number":723,"context_line":"        super(SchedulerClientTrackerTestCase, self).setUp()"},{"line_number":724,"context_line":"        self.tracker.scheduler_client.update_resource_stats \u003d mock.Mock()"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    def test_update_resource(self):"},{"line_number":727,"context_line":"        self.tracker._write_ext_resources \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba7be1f8_ffc371ea","line":724,"in_reply_to":"ba7be1f8_c4718445","updated":"2015-02-24 20:03:59.000000000","message":"test_tracker.py is going to supersede test_resource_tracker.py. So new stuff is going into test_tracker.py. test_resource_tracker.py will be dropped when we know everything in it is covered by the other file.","commit_id":"a89a42081ee5fb10d79b6886e6904ed518f65397"}],"nova/tests/unit/compute/test_tracker.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b1a051b723614964f2296203cdbe6391682f4efb","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        expected_resources[\u0027host\u0027] \u003d \u0027fake-host\u0027"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        self.rt._init_compute_node(mock.sentinel.ctx, resources)"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        self.assertFalse(self.rt.disabled)"},{"line_number":775,"context_line":"        service_mock.assert_called_once_with(mock.sentinel.ctx, \u0027fake-host\u0027)"},{"line_number":776,"context_line":"        get_mock.assert_called_once_with(mock.sentinel.ctx, \u0027fake-host\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_646ea6cb","line":773,"updated":"2015-01-27 20:55:00.000000000","message":"IMHO, we should explicitely add coverage for the method you\u0027re adding so we could isolate any problem later on","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"b646d77a4fa4a32e1b522a38a9254a202f973b93","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        expected_resources[\u0027host\u0027] \u003d \u0027fake-host\u0027"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        self.rt._init_compute_node(mock.sentinel.ctx, resources)"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"        self.assertFalse(self.rt.disabled)"},{"line_number":775,"context_line":"        service_mock.assert_called_once_with(mock.sentinel.ctx, \u0027fake-host\u0027)"},{"line_number":776,"context_line":"        get_mock.assert_called_once_with(mock.sentinel.ctx, \u0027fake-host\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa81d914_8651a210","line":773,"in_reply_to":"fa81d914_646ea6cb","updated":"2015-01-28 22:18:56.000000000","message":"Done - added TestInitComputeNode class with methods","commit_id":"1c758dba524492cc2360d626769632ba95a62861"},{"author":{"_account_id":1981,"name":"Don Dugger","email":"n0ano@n0ano.com","username":"n0ano"},"change_message_id":"8eacf39647787b63d4edd199065df20dca5321cc","unresolved":false,"context_lines":[{"line_number":835,"context_line":"        expected_resources[\u0027service_id\u0027] \u003d 1"},{"line_number":836,"context_line":"        # NOTE(sbauza): ResourceTracker adds host field"},{"line_number":837,"context_line":"        expected_resources[\u0027host\u0027] \u003d \u0027fake-host\u0027"},{"line_number":838,"context_line":"        # pci_passthrough_devices should is not held in compute nodes"},{"line_number":839,"context_line":"        del expected_resources[\u0027pci_passthrough_devices\u0027]"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        self.rt._init_compute_node(mock.sentinel.ctx, resources)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_4471640b","line":838,"updated":"2015-03-17 21:00:49.000000000","message":"I think this should read `...should not be held...\u0027","commit_id":"16be9a5bbd7e4bfdd51212ba1a8a16619b935e3b"}],"nova/virt/fake.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f49adf6545530216cc612b6a92a6ea58cbc89201","unresolved":false,"context_lines":[{"line_number":110,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":111,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":112,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":114,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":115,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":116,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_1cbb57c5","line":113,"updated":"2015-02-27 17:08:13.000000000","message":"Why is it related to the patch ?","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"9ef787efc6fa334eb8ce93cb536ee167d341a3c9","unresolved":false,"context_lines":[{"line_number":110,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":111,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":112,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":114,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":115,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":116,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_7e4ecbc2","line":113,"in_reply_to":"ba7be1f8_1cbb57c5","updated":"2015-03-02 14:59:46.000000000","message":"See patch set 4. It is related because the value was not previously used in tests, but because of this patch it is.\n\nThe value of local_gb_used should be in GB - it was not. That was a latent bug that surfaced in this change when we started putting the values returned by the virt driver in the database as initial values for the compute_node record. Now that we are doing that it turns out this value is too large to fit in the local_gb_used field in the database.\n\nIt didn\u0027t matter before because the RT replaced this value with its own calculations.\n\nI don\u0027t think it makes sense to put a comment in the code here because it would say something like \"this GB value is in GB\"","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":7461,"name":"Paul Murray","email":"pmurray@hpe.com","username":"ptm"},"change_message_id":"d96e527587b2f84e7fd10b580013a540cb29e3bc","unresolved":false,"context_lines":[{"line_number":110,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":111,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":112,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":114,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":115,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":116,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_a47cb4b6","line":113,"in_reply_to":"ba7be1f8_41f84a7c","updated":"2015-03-02 15:32:57.000000000","message":"The value wasn\u0027t actually used previously, so the test for it would be to check the fake has a correct value, which doesn\u0027t really test anything. Now that the value is used, the existing checks fail because it is incorrect. So really the coverage is already there. So if you don\u0027t mind I would like to keep it as part of this patch just to keep things simple.","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bedda5da0ba77bee7d3902907f35402f56578214","unresolved":false,"context_lines":[{"line_number":110,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":111,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":112,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":114,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":115,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":116,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_41f84a7c","line":113,"in_reply_to":"ba7be1f8_7e4ecbc2","updated":"2015-03-02 15:09:32.000000000","message":"I see, thanks for your explanation. The main problem is that it wasn\u0027t covered before so it appeared now and you had to fix it.\n\nIf that doesn\u0027t bother you too much, what do you think about first fixing the missing coverage and add this in a first patch so that this one could be dependent ? \nThat way, it would make things clearer.","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"bb891fd5e324cb405639b446d3bcb18eab6305e0","unresolved":false,"context_lines":[{"line_number":110,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":111,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":112,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":114,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":115,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":116,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_25951b40","line":113,"in_reply_to":"ba7be1f8_a47cb4b6","updated":"2015-03-02 17:04:01.000000000","message":"Fair enough","commit_id":"d91444364d11481bd364843b250c808fa3ba8b10"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"87bcb0ea7c61811eda5283e9b0ca2800568c240d","unresolved":false,"context_lines":[{"line_number":111,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":112,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":113,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":114,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":115,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":116,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":117,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a80dd14_00bad8e0","line":114,"updated":"2015-03-18 13:19:49.000000000","message":"not 100% sure why we need this. can you please explain.","commit_id":"16be9a5bbd7e4bfdd51212ba1a8a16619b935e3b"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"64561dcf16138c3c52945657431833adec127b0f","unresolved":false,"context_lines":[{"line_number":149,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":150,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":151,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":152,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":153,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":154,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":155,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_a821b39f","line":152,"updated":"2015-03-19 14:50:53.000000000","message":"Do we even need these anymore now that resources are tracked in the Resources class?","commit_id":"648fa440d5bbf0909f83eb90214c543ef2a12263"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"486bf14bcb4412fdec1e2697960d04f023dc093c","unresolved":false,"context_lines":[{"line_number":149,"context_line":"          \u0027local_gb\u0027: self.local_gb,"},{"line_number":150,"context_line":"          \u0027vcpus_used\u0027: 0,"},{"line_number":151,"context_line":"          \u0027memory_mb_used\u0027: 0,"},{"line_number":152,"context_line":"          \u0027local_gb_used\u0027: 100000000000 / units.Gi,"},{"line_number":153,"context_line":"          \u0027hypervisor_type\u0027: \u0027fake\u0027,"},{"line_number":154,"context_line":"          \u0027hypervisor_version\u0027: utils.convert_version_to_int(\u00271.0\u0027),"},{"line_number":155,"context_line":"          \u0027hypervisor_hostname\u0027: CONF.host,"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_286a191a","line":152,"in_reply_to":"9a80dd14_a821b39f","updated":"2015-03-19 17:51:14.000000000","message":"right, this no longer does anything. posting a new version now.","commit_id":"648fa440d5bbf0909f83eb90214c543ef2a12263"}]}