)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2badffbbf1882cde497a0ba8ec031fa458c304f1","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A scheduler host state instance contains caches of a compute node, its"},{"line_number":10,"context_line":"service, aggregates and instances. Their updating logics are collected"},{"line_number":11,"context_line":"together and then added an inter-process lock to make them atomic in"},{"line_number":12,"context_line":"this patch. The lock is added per host state, in order to reduce the"},{"line_number":13,"context_line":"unecessary blocks to the minimum. The according tests are added to make"},{"line_number":14,"context_line":"sure the locks are set when doing updates."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a7b0d38_82bfa3cf","line":11,"updated":"2015-12-23 13:16:48.000000000","message":"It\u0027s not an inter-process lock. That would imply the lock is between several processes. You could say \"intra-process\" if you want to convey what it is but that\u0027s still confusing for the reader.\n\nYou might say: \"Their updating logics are collected together under a synchronized to make the updating atomic in the scheduler process.\" Or something along those lines.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A scheduler host state instance contains caches of a compute node, its"},{"line_number":10,"context_line":"service, aggregates and instances. Their updating logics are collected"},{"line_number":11,"context_line":"together and then added an inter-process lock to make them atomic in"},{"line_number":12,"context_line":"this patch. The lock is added per host state, in order to reduce the"},{"line_number":13,"context_line":"unecessary blocks to the minimum. The according tests are added to make"},{"line_number":14,"context_line":"sure the locks are set when doing updates."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a7b0d38_de231aa6","line":11,"in_reply_to":"1a7b0d38_82bfa3cf","updated":"2015-12-24 16:51:42.000000000","message":"Sorry, I meant \"inner\" process when wrote this, but had fat fingers...","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    def __init__(self, host, node):"},{"line_number":110,"context_line":"        self.host \u003d host"},{"line_number":111,"context_line":"        self.nodename \u003d node"},{"line_number":112,"context_line":"        self.lock_name \u003d (host, node)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Mutable available resources."},{"line_number":115,"context_line":"        # These will change as resources are virtually \"consumed\"."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_7ecfa2b0","line":112,"range":{"start_line":112,"start_character":13,"end_line":112,"end_character":22},"updated":"2015-12-23 18:47:48.000000000","message":"nit: I\u0027d make this private personally","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    def __init__(self, host, node):"},{"line_number":110,"context_line":"        self.host \u003d host"},{"line_number":111,"context_line":"        self.nodename \u003d node"},{"line_number":112,"context_line":"        self.lock_name \u003d (host, node)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Mutable available resources."},{"line_number":115,"context_line":"        # These will change as resources are virtually \"consumed\"."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_01c86721","line":112,"range":{"start_line":112,"start_character":13,"end_line":112,"end_character":22},"in_reply_to":"1a7b0d38_7ecfa2b0","updated":"2015-12-24 16:51:42.000000000","message":"Done","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2badffbbf1882cde497a0ba8ec031fa458c304f1","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        \"\"\"Update all information about a host.\"\"\""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        @utils.synchronized(self.lock_name)"},{"line_number":161,"context_line":"        def _locked_update(self, compute, service, aggregates, inst_dict):"},{"line_number":162,"context_line":"            if compute is not None:"},{"line_number":163,"context_line":"                self._update_from_compute_node(compute)"},{"line_number":164,"context_line":"            if aggregates is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_e2c46793","line":161,"updated":"2015-12-23 13:16:48.000000000","message":"Do you need to pass self to the method or can you just use the one that is covered by the closure? I\u0027m not certain.\n\nI suppose if you don\u0027t need to it doesn\u0027t really matter.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        \"\"\"Update all information about a host.\"\"\""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        @utils.synchronized(self.lock_name)"},{"line_number":161,"context_line":"        def _locked_update(self, compute, service, aggregates, inst_dict):"},{"line_number":162,"context_line":"            if compute is not None:"},{"line_number":163,"context_line":"                self._update_from_compute_node(compute)"},{"line_number":164,"context_line":"            if aggregates is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_1e4e8296","line":161,"in_reply_to":"1a7b0d38_d66804f8","updated":"2015-12-24 16:51:42.000000000","message":"@Nikola: Yes technically, there is no need to use a closure to implement an instance-grained lock. But, as @Chris said, I have to use this closure to pass the `self` reference to `utils.synchronized` in the current implementation of oslo_concurrency. \n\nHere is another example using this closure without firing threads: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L234-L239\n\nI\u0027ve tried to decorate it directly but failed with error(NameError: name \u0027self\u0027 is not defined), so I have to keep this until I figure out a work-around.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        \"\"\"Update all information about a host.\"\"\""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        @utils.synchronized(self.lock_name)"},{"line_number":161,"context_line":"        def _locked_update(self, compute, service, aggregates, inst_dict):"},{"line_number":162,"context_line":"            if compute is not None:"},{"line_number":163,"context_line":"                self._update_from_compute_node(compute)"},{"line_number":164,"context_line":"            if aggregates is not None:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_d66804f8","line":161,"in_reply_to":"1a7b0d38_e2c46793","updated":"2015-12-23 18:47:48.000000000","message":"Yep it\u0027s part of the closure so does not need to be passed as an argument. Not a huge deal though...\n\nI think that this pattern of defining a closure for a locked operation comes from the compute manager, and makes sense only if we fire of the operation that may block on a separate thread if it\u0027s a cast callback on an rpc endpoint class, so that we don\u0027t exhaust the executor connection pool needlessly. \n\nI don\u0027t think this is necessary here - you can decorate the method directly","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        self.updated \u003d None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def update(self, compute\u003dNone, service\u003dNone, aggregates\u003dNone,"},{"line_number":157,"context_line":"            inst_dict\u003dNone):"},{"line_number":158,"context_line":"        \"\"\"Update all information about a host.\"\"\""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        @utils.synchronized(self.lock_name)"},{"line_number":161,"context_line":"        def _locked_update(self, compute, service, aggregates, inst_dict):"},{"line_number":162,"context_line":"            if compute is not None:"},{"line_number":163,"context_line":"                self._update_from_compute_node(compute)"},{"line_number":164,"context_line":"            if aggregates is not None:"},{"line_number":165,"context_line":"                self.aggregates \u003d aggregates"},{"line_number":166,"context_line":"            if service is not None:"},{"line_number":167,"context_line":"                self.service \u003d ReadOnlyDict(service)"},{"line_number":168,"context_line":"            if inst_dict is not None:"},{"line_number":169,"context_line":"                self.instances \u003d inst_dict"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        return _locked_update(self, compute, service, aggregates, inst_dict)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def _update_from_compute_node(self, compute):"},{"line_number":174,"context_line":"        \"\"\"Update information about a host from a ComputeNode object.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_1ecf7e80","line":171,"range":{"start_line":156,"start_character":4,"end_line":171,"end_character":76},"updated":"2015-12-23 18:47:48.000000000","message":"It would be excellent to have some debug logging added too.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        self.updated \u003d None"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def update(self, compute\u003dNone, service\u003dNone, aggregates\u003dNone,"},{"line_number":157,"context_line":"            inst_dict\u003dNone):"},{"line_number":158,"context_line":"        \"\"\"Update all information about a host.\"\"\""},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        @utils.synchronized(self.lock_name)"},{"line_number":161,"context_line":"        def _locked_update(self, compute, service, aggregates, inst_dict):"},{"line_number":162,"context_line":"            if compute is not None:"},{"line_number":163,"context_line":"                self._update_from_compute_node(compute)"},{"line_number":164,"context_line":"            if aggregates is not None:"},{"line_number":165,"context_line":"                self.aggregates \u003d aggregates"},{"line_number":166,"context_line":"            if service is not None:"},{"line_number":167,"context_line":"                self.service \u003d ReadOnlyDict(service)"},{"line_number":168,"context_line":"            if inst_dict is not None:"},{"line_number":169,"context_line":"                self.instances \u003d inst_dict"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        return _locked_update(self, compute, service, aggregates, inst_dict)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def _update_from_compute_node(self, compute):"},{"line_number":174,"context_line":"        \"\"\"Update information about a host from a ComputeNode object.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_a150735f","line":171,"range":{"start_line":156,"start_character":4,"end_line":171,"end_character":76},"in_reply_to":"1a7b0d38_1ecf7e80","updated":"2015-12-24 16:51:42.000000000","message":"Done","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    \"\"\"Base HostManager class.\"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    # Can be overridden in a subclass"},{"line_number":300,"context_line":"    def host_state_cls(self, host, node, **kwargs):"},{"line_number":301,"context_line":"        return HostState(host, node)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_be3c2a94","line":300,"range":{"start_line":300,"start_character":41,"end_line":300,"end_character":49},"updated":"2015-12-23 18:47:48.000000000","message":"why keep kwargs here ?","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    \"\"\"Base HostManager class.\"\"\""},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    # Can be overridden in a subclass"},{"line_number":300,"context_line":"    def host_state_cls(self, host, node, **kwargs):"},{"line_number":301,"context_line":"        return HostState(host, node)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_a10f132d","line":300,"range":{"start_line":300,"start_character":41,"end_line":300,"end_character":49},"in_reply_to":"1a7b0d38_be3c2a94","updated":"2015-12-24 16:51:42.000000000","message":"Here is the reason: https://github.com/openstack/nova/blob/master/nova/scheduler/ironic_host_manager.py#L85-L91\n\nI remember there is a plan to deprecate ironic_host_manager, maybe we can remove `**kwargs` after that.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":541,"context_line":"            state_key \u003d (host, node)"},{"line_number":542,"context_line":"            host_state \u003d self.host_state_map.get(state_key)"},{"line_number":543,"context_line":"            if not host_state:"},{"line_number":544,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":545,"context_line":"                self.host_state_map[state_key] \u003d host_state"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            # We force to update the aggregates info each time a new request"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_9ed04e55","line":544,"range":{"start_line":544,"start_character":61,"end_line":544,"end_character":76},"updated":"2015-12-23 18:47:48.000000000","message":"so we pass this in but completely ignore it? Why not remove the kwargs then?","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":541,"context_line":"            state_key \u003d (host, node)"},{"line_number":542,"context_line":"            host_state \u003d self.host_state_map.get(state_key)"},{"line_number":543,"context_line":"            if not host_state:"},{"line_number":544,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":545,"context_line":"                self.host_state_map[state_key] \u003d host_state"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            # We force to update the aggregates info each time a new request"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_012707b7","line":544,"range":{"start_line":544,"start_character":61,"end_line":544,"end_character":76},"in_reply_to":"1a7b0d38_9ed04e55","updated":"2015-12-24 16:51:42.000000000","message":"explained above, there is a child ironic host state class implements another version of  host_state_cls() using the kw args.","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"c725f980d54adb6d5fb46fc630ec87e48fe30d55","unresolved":false,"context_lines":[{"line_number":543,"context_line":"            if not host_state:"},{"line_number":544,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":545,"context_line":"                self.host_state_map[state_key] \u003d host_state"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            # We force to update the aggregates info each time a new request"},{"line_number":548,"context_line":"            # comes in, because some changes on the aggregates could have been"},{"line_number":549,"context_line":"            # happening after setting this field for the first time"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_8aaa50d3","line":546,"updated":"2015-12-23 18:47:48.000000000","message":"Random whitespace :(","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"},{"author":{"_account_id":15648,"name":"Yingxin","email":"yingxin.cheng@intel.com","username":"Yingxin"},"change_message_id":"5dcdca61c578c390aff686cdd1e30e3c528168a8","unresolved":false,"context_lines":[{"line_number":543,"context_line":"            if not host_state:"},{"line_number":544,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":545,"context_line":"                self.host_state_map[state_key] \u003d host_state"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            # We force to update the aggregates info each time a new request"},{"line_number":548,"context_line":"            # comes in, because some changes on the aggregates could have been"},{"line_number":549,"context_line":"            # happening after setting this field for the first time"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a7b0d38_0175c796","line":546,"in_reply_to":"1a7b0d38_8aaa50d3","updated":"2015-12-24 16:51:42.000000000","message":"truly sorry","commit_id":"82b80378246598bb7a5e5fab23ccc5605d256d4b"}]}