)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":29,"context_line":"In order to keep the L3 agent CPU usage down, it will spawn a process"},{"line_number":30,"context_line":"per HA router. That process will start \u0027ip monitor address\u0027. Whenever"},{"line_number":31,"context_line":"it gets an IP address change event, it will notify the L3 agent via"},{"line_number":32,"context_line":"a unix domain socket."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: I2022bced330d5f108fbedd40548a901225d7ea1c"},{"line_number":35,"context_line":"Closes-Bug: #1402010"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"ba7be1f8_89fc3240","line":32,"updated":"2015-03-02 13:13:25.000000000","message":"DocImpact tag is missing as the change adds options","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":29,"context_line":"In order to keep the L3 agent CPU usage down, it will spawn a process"},{"line_number":30,"context_line":"per HA router. That process will start \u0027ip monitor address\u0027. Whenever"},{"line_number":31,"context_line":"it gets an IP address change event, it will notify the L3 agent via"},{"line_number":32,"context_line":"a unix domain socket."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"Change-Id: I2022bced330d5f108fbedd40548a901225d7ea1c"},{"line_number":35,"context_line":"Closes-Bug: #1402010"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"9a80dd14_062e3dfb","line":32,"in_reply_to":"ba7be1f8_89fc3240","updated":"2015-03-06 21:01:39.000000000","message":"Done","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"}],"etc/l3_agent.ini":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a4314a7f01f1441f284d4eba67406b2551f07661","unresolved":false,"context_lines":[{"line_number":112,"context_line":"# The advertisement interval in seconds"},{"line_number":113,"context_line":"# ha_vrrp_advert_int \u003d 2"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"[KEEPALIVED_STATE_CHANGE]"},{"line_number":116,"context_line":"# socket \u003d $state_path/keepalived_state_change"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"# Maximum number of queued requests for keepalived state change server"}],"source_content_type":"text/x-properties","patch_set":23,"id":"da86d52c_38c90c7a","line":115,"updated":"2015-02-14 23:09:18.000000000","message":"Other than \"DEFAULT\" section, we use a lower case name as a section name. Please consider following the convention in OpenStack.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":112,"context_line":"# The advertisement interval in seconds"},{"line_number":113,"context_line":"# ha_vrrp_advert_int \u003d 2"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"[KEEPALIVED_STATE_CHANGE]"},{"line_number":116,"context_line":"# socket \u003d $state_path/keepalived_state_change"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"# Maximum number of queued requests for keepalived state change server"}],"source_content_type":"text/x-properties","patch_set":23,"id":"ba7be1f8_6e3c6ee3","line":115,"in_reply_to":"da86d52c_38c90c7a","updated":"2015-02-24 02:06:35.000000000","message":"Done","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":116,"context_line":"# socket \u003d $state_path/keepalived_state_change"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"# Maximum number of queued requests for keepalived state change server"},{"line_number":119,"context_line":"# backlog \u003d 4096"}],"source_content_type":"text/x-properties","patch_set":38,"id":"9a80dd14_e6ae4e80","line":119,"updated":"2015-03-15 13:01:03.000000000","message":"Why is it necessary to expose these two as configurable options?\n\nWhy would it be interesting for the admin to configure?","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"783fe6165e0b716e5dd58ac1b40384ce8df2c15a","unresolved":false,"context_lines":[{"line_number":116,"context_line":"# socket \u003d $state_path/keepalived_state_change"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"# Maximum number of queued requests for keepalived state change server"},{"line_number":119,"context_line":"# backlog \u003d 4096"}],"source_content_type":"text/x-properties","patch_set":38,"id":"9a80dd14_e11097ad","line":119,"in_reply_to":"9a80dd14_e6ae4e80","updated":"2015-03-17 17:38:42.000000000","message":"I coped it from the metadata proxy options. I personally don\u0027t think it\u0027s useful. I\u0027ll happily remove it and if anyone can explain to me why users would want to change these values, or if a user request comes up in the future, I\u0027ll put it back in.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"}],"neutron/agent/l3/ha.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a4314a7f01f1441f284d4eba67406b2551f07661","unresolved":false,"context_lines":[{"line_number":84,"context_line":"            self.conf.KEEPALIVED_STATE_CHANGE.socket)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def run(self):"},{"line_number":87,"context_line":"        server \u003d metadata_agent.UnixDomainWSGIServer("},{"line_number":88,"context_line":"            \u0027neutron-keepalived-state-change\u0027)"},{"line_number":89,"context_line":"        server.start(KeepalivedStateChangeHandler(self.agent),"},{"line_number":90,"context_line":"                     self.conf.KEEPALIVED_STATE_CHANGE.socket,"}],"source_content_type":"text/x-python","patch_set":23,"id":"da86d52c_b8a41cba","line":87,"updated":"2015-02-14 23:09:18.000000000","message":"UnixDomainWSGIServer is not used only for metadata_agent now.\nIt seems better to move UnixDomainWSGIServer to some common place.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"3d062d3c6143c0b91daae2eef462a1abe7ca1a01","unresolved":false,"context_lines":[{"line_number":84,"context_line":"            self.conf.KEEPALIVED_STATE_CHANGE.socket)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def run(self):"},{"line_number":87,"context_line":"        server \u003d metadata_agent.UnixDomainWSGIServer("},{"line_number":88,"context_line":"            \u0027neutron-keepalived-state-change\u0027)"},{"line_number":89,"context_line":"        server.start(KeepalivedStateChangeHandler(self.agent),"},{"line_number":90,"context_line":"                     self.conf.KEEPALIVED_STATE_CHANGE.socket,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_b7e3d9a2","line":87,"in_reply_to":"ba7be1f8_78226202","updated":"2015-03-06 21:40:40.000000000","message":"Done","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":84,"context_line":"            self.conf.KEEPALIVED_STATE_CHANGE.socket)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def run(self):"},{"line_number":87,"context_line":"        server \u003d metadata_agent.UnixDomainWSGIServer("},{"line_number":88,"context_line":"            \u0027neutron-keepalived-state-change\u0027)"},{"line_number":89,"context_line":"        server.start(KeepalivedStateChangeHandler(self.agent),"},{"line_number":90,"context_line":"                     self.conf.KEEPALIVED_STATE_CHANGE.socket,"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_78226202","line":87,"in_reply_to":"da86d52c_b8a41cba","updated":"2015-02-24 02:06:35.000000000","message":"Still need to address this.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        self.enqueue(router_id, state)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def enqueue(self, router_id, state):"},{"line_number":61,"context_line":"        LOG.debug(\u0027Keepalived notification for router: %s, state: %s\u0027 %"},{"line_number":62,"context_line":"                  (router_id, state))"},{"line_number":63,"context_line":"        self.agent.enqueue_state_change(router_id, state)"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_0c585031","line":61,"updated":"2015-03-02 13:13:25.000000000","message":"Use lazy formatting: LOG.debug(..., ...)","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        self.enqueue(router_id, state)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def enqueue(self, router_id, state):"},{"line_number":61,"context_line":"        LOG.debug(\u0027Keepalived notification for router: %s, state: %s\u0027 %"},{"line_number":62,"context_line":"                  (router_id, state))"},{"line_number":63,"context_line":"        self.agent.enqueue_state_change(router_id, state)"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_86414dab","line":61,"in_reply_to":"ba7be1f8_0c585031","updated":"2015-03-06 21:01:39.000000000","message":"Whups. Done.","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":7183,"name":"Xu Han Peng","email":"xuhanp@linux.vnet.ibm.com","username":"xuhanp"},"change_message_id":"b75b42e2f0cc630dd22f6c954dc0e4e0de3f5486","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        if state \u003d\u003d \u0027master\u0027:"},{"line_number":123,"context_line":"            LOG.debug(\u0027Spawning metadata proxy for router %s\u0027, router_id)"},{"line_number":124,"context_line":"            self.metadata_driver.spawn_monitored_metadata_proxy("},{"line_number":125,"context_line":"                ri.router_id, ri.ns_name)"},{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            LOG.debug(\u0027Closing metadata proxy for router %s\u0027, router_id)"},{"line_number":128,"context_line":"            self.metadata_driver.destroy_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_49cdca5c","line":125,"updated":"2015-03-06 06:56:29.000000000","message":"these two arguments don\u0027t seem to match with spawn_monitored_metadata_proxy in neutron/agent/metadata/driver.py","commit_id":"d2e6917495b729c290ec809e1d83fc1965fa381d"},{"author":{"_account_id":7183,"name":"Xu Han Peng","email":"xuhanp@linux.vnet.ibm.com","username":"xuhanp"},"change_message_id":"b75b42e2f0cc630dd22f6c954dc0e4e0de3f5486","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            LOG.debug(\u0027Closing metadata proxy for router %s\u0027, router_id)"},{"line_number":128,"context_line":"            self.metadata_driver.destroy_monitored_metadata_proxy("},{"line_number":129,"context_line":"                ri.router_id, ri.ns_name)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _init_ha_conf_path(self):"},{"line_number":132,"context_line":"        ha_full_path \u003d os.path.dirname(\"/%s/\" % self.conf.ha_confs_path)"}],"source_content_type":"text/x-python","patch_set":29,"id":"9a80dd14_c908da93","line":129,"updated":"2015-03-06 06:56:29.000000000","message":"Same as above.","commit_id":"d2e6917495b729c290ec809e1d83fc1965fa381d"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                  \u0027state\u0027: state})"},{"line_number":111,"context_line":"        self._update_metadata_proxy(router_id, state)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def _update_metadata_proxy(self, router_id, state):"},{"line_number":114,"context_line":"        try:"},{"line_number":115,"context_line":"            ri \u003d self.router_info[router_id]"},{"line_number":116,"context_line":"        except AttributeError:"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_66f09e91","line":113,"updated":"2015-03-15 13:01:03.000000000","message":"What\u0027s to prevent a race in case same router flips quick enough?","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"df6b3fd4b3298f7dd5c03b8b7f330089c21d6646","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                  \u0027state\u0027: state})"},{"line_number":111,"context_line":"        self._update_metadata_proxy(router_id, state)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def _update_metadata_proxy(self, router_id, state):"},{"line_number":114,"context_line":"        try:"},{"line_number":115,"context_line":"            ri \u003d self.router_info[router_id]"},{"line_number":116,"context_line":"        except AttributeError:"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_bd2d4091","line":113,"in_reply_to":"9a80dd14_66f09e91","updated":"2015-03-15 15:09:57.000000000","message":"The keepalived-state-change process for that router is guaranteed to send the state changes in the correct order. Those messages are then implicitly put in a queue in the L3 agent (This is done by the Unix domain socket WSGI server moved around in the preceding patch) and are processed one by one by a single worker, so we should be good.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"}],"neutron/agent/l3/ha_router.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7783bdf533960381a0e06c187d95bc747db0ec12","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            with open(ha_state_path, \u0027w\u0027) as f:"},{"line_number":74,"context_line":"                f.write(new_state)"},{"line_number":75,"context_line":"        except (OSError, IOError):"},{"line_number":76,"context_line":"            LOG.debug(\u0027Error while writing HA state for %s\u0027, self.router_id)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_keepalived_manager(self):"},{"line_number":79,"context_line":"        return keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":23,"id":"da86d52c_5414089f","line":76,"updated":"2015-02-13 00:48:35.000000000","message":"are you sure that the debug level is appropriate for this type of error?","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            with open(ha_state_path, \u0027w\u0027) as f:"},{"line_number":74,"context_line":"                f.write(new_state)"},{"line_number":75,"context_line":"        except (OSError, IOError):"},{"line_number":76,"context_line":"            LOG.debug(\u0027Error while writing HA state for %s\u0027, self.router_id)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_keepalived_manager(self):"},{"line_number":79,"context_line":"        return keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_d87a6ef0","line":76,"in_reply_to":"da86d52c_5414089f","updated":"2015-02-24 02:06:35.000000000","message":"Still need to address this. I\u0027ll raise it to error. Actually I need to think about error handling here, I\u0027m not sure what I\u0027d want to do if we fail over. It\u0027s kind of critical...","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            with open(ha_state_path, \u0027w\u0027) as f:"},{"line_number":75,"context_line":"                f.write(new_state)"},{"line_number":76,"context_line":"        except (OSError, IOError):"},{"line_number":77,"context_line":"            LOG.debug(\u0027Error while writing HA state for %s\u0027, self.router_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_keepalived_manager(self):"},{"line_number":80,"context_line":"        return keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_6caaacd3","line":77,"updated":"2015-03-02 13:13:25.000000000","message":"it seems a LOG.warning/error would be better?","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            with open(ha_state_path, \u0027w\u0027) as f:"},{"line_number":75,"context_line":"                f.write(new_state)"},{"line_number":76,"context_line":"        except (OSError, IOError):"},{"line_number":77,"context_line":"            LOG.debug(\u0027Error while writing HA state for %s\u0027, self.router_id)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_keepalived_manager(self):"},{"line_number":80,"context_line":"        return keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_e6720910","line":77,"in_reply_to":"ba7be1f8_6caaacd3","updated":"2015-03-06 21:01:39.000000000","message":"Done","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"ff061bad39612eb10729745b279316b65e41c0d2","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            conf,"},{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_cac23c33","line":275,"updated":"2015-03-04 01:53:40.000000000","message":"This needs to use the process monitor and not ProcessManager.","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            conf,"},{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_3cb50e7e","line":275,"in_reply_to":"ba7be1f8_cac23c33","updated":"2015-03-06 21:01:39.000000000","message":"Done","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"},{"line_number":279,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_0ccbf0a5","line":276,"updated":"2015-03-02 13:13:25.000000000","message":"_get_state_..., spawn/destroy_state_change... look likes metadata proxy driver ones?","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"},{"line_number":279,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_9c76fad5","line":276,"in_reply_to":"9a80dd14_7044a174","updated":"2015-03-06 21:01:39.000000000","message":"You\u0027re right that it\u0027s pretty much the same, however we\u0027re talking about 3 lines of shared code (In the PS I\u0027m about to push). It\u0027ll take more lines of code to make sure they\u0027re shared, I don\u0027t think it\u0027s worth it.","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"ff061bad39612eb10729745b279316b65e41c0d2","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"},{"line_number":279,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_eabd40b2","line":276,"in_reply_to":"ba7be1f8_0ccbf0a5","updated":"2015-03-04 01:53:40.000000000","message":"Sorry I don\u0027t think I understand what you mean exactly.","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cac300ec372a2057c05379ecbfa414745cfdb2a1","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027%s.ip_monitor\u0027 % self.router_id,"},{"line_number":274,"context_line":"            self.ns_name)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def spawn_state_change_monitor(self, conf):"},{"line_number":277,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager(conf)"},{"line_number":279,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_7044a174","line":276,"in_reply_to":"ba7be1f8_eabd40b2","updated":"2015-03-04 10:06:44.000000000","message":"when i look a the change some code seems shared with metadata driver/namespace_proxy ... could they share it?","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def spawn_state_change_monitor(self, process_monitor):"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"},{"line_number":279,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":280,"context_line":"        pm.enable(callback)"},{"line_number":281,"context_line":"        process_monitor.register(self.router_id, \u0027ip_monitor\u0027, pm)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def destroy_state_change_monitor(self, process_monitor):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_a6c9b627","line":280,"updated":"2015-03-15 13:01:03.000000000","message":"The callback should be set as \u0027default_callback\u0027 in the ProcessManager that you create, otherwise ProcessMonitor wouldn\u0027t know how to respawn it..","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"783fe6165e0b716e5dd58ac1b40384ce8df2c15a","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def spawn_state_change_monitor(self, process_monitor):"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"},{"line_number":279,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":280,"context_line":"        pm.enable(callback)"},{"line_number":281,"context_line":"        process_monitor.register(self.router_id, \u0027ip_monitor\u0027, pm)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def destroy_state_change_monitor(self, process_monitor):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_0d0ebae5","line":280,"in_reply_to":"9a80dd14_3d595035","updated":"2015-03-17 17:38:42.000000000","message":"Done","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"df6b3fd4b3298f7dd5c03b8b7f330089c21d6646","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def spawn_state_change_monitor(self, process_monitor):"},{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"},{"line_number":279,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":280,"context_line":"        pm.enable(callback)"},{"line_number":281,"context_line":"        process_monitor.register(self.router_id, \u0027ip_monitor\u0027, pm)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def destroy_state_change_monitor(self, process_monitor):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_3d595035","line":280,"in_reply_to":"9a80dd14_a6c9b627","updated":"2015-03-15 15:09:57.000000000","message":"Talk about a surprising API! I\u0027ll look in to this tomorrow along with the other comments.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"},{"line_number":279,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":280,"context_line":"        pm.enable(callback)"},{"line_number":281,"context_line":"        process_monitor.register(self.router_id, \u0027ip_monitor\u0027, pm)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def destroy_state_change_monitor(self, process_monitor):"},{"line_number":284,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_e6cf2e27","line":281,"updated":"2015-03-15 13:01:03.000000000","message":"It would be better if \u0027ip_monitor\u0027 was a constant","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"783fe6165e0b716e5dd58ac1b40384ce8df2c15a","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"},{"line_number":279,"context_line":"        callback \u003d self._get_state_change_monitor_callback()"},{"line_number":280,"context_line":"        pm.enable(callback)"},{"line_number":281,"context_line":"        process_monitor.register(self.router_id, \u0027ip_monitor\u0027, pm)"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def destroy_state_change_monitor(self, process_monitor):"},{"line_number":284,"context_line":"        pm \u003d self._get_state_change_monitor_process_manager()"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_cd9f4264","line":281,"in_reply_to":"9a80dd14_e6cf2e27","updated":"2015-03-17 17:38:42.000000000","message":"Done","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        ha_cidr \u003d self._get_primary_vip()"},{"line_number":295,"context_line":"        state \u003d \u0027master\u0027 if ha_cidr in cidrs else \u0027backup\u0027"},{"line_number":296,"context_line":"        self.ha_state \u003d state"},{"line_number":297,"context_line":"        callback(self.router_id, state)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_66a89e20","line":297,"updated":"2015-03-15 13:01:03.000000000","message":"Would there be a race if right before this line is executed the router transitions state?\n\nIs it possible that this method would run during agent restart while a state change monitor is already running?","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"df6b3fd4b3298f7dd5c03b8b7f330089c21d6646","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        ha_cidr \u003d self._get_primary_vip()"},{"line_number":295,"context_line":"        state \u003d \u0027master\u0027 if ha_cidr in cidrs else \u0027backup\u0027"},{"line_number":296,"context_line":"        self.ha_state \u003d state"},{"line_number":297,"context_line":"        callback(self.router_id, state)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_dd7674a3","line":297,"in_reply_to":"9a80dd14_66a89e20","updated":"2015-03-15 15:09:57.000000000","message":"I think you\u0027re right. I\u0027ll look in to putting a per-router lock in the keepalived state change handler and here.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"783fe6165e0b716e5dd58ac1b40384ce8df2c15a","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        ha_cidr \u003d self._get_primary_vip()"},{"line_number":295,"context_line":"        state \u003d \u0027master\u0027 if ha_cidr in cidrs else \u0027backup\u0027"},{"line_number":296,"context_line":"        self.ha_state \u003d state"},{"line_number":297,"context_line":"        callback(self.router_id, state)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_92564729","line":297,"in_reply_to":"9a80dd14_dd7674a3","updated":"2015-03-17 17:38:42.000000000","message":"So I\u0027ve been playing around with this, trying to reproduce it, and I couldn\u0027t. I thought about it and I think I want to treat this as one of those \u0027If we ever get an actual bug report\u0027 issues, and not introduce additional complexity right now.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"0e7b5a1b4b5a2dfa16faec493102c592608ab3a5","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        except (OSError, IOError):"},{"line_number":83,"context_line":"            LOG.error(_LE(\u0027Error while writing HA state for %s\u0027),"},{"line_number":84,"context_line":"                      self.router_id)"},{"line_number":85,"context_line":"            return None"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _init_keepalived_manager(self, process_monitor):"},{"line_number":88,"context_line":"        self.keepalived_manager \u003d keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_35972eed","line":85,"updated":"2015-03-18 04:24:20.000000000","message":"(No action required) Returning None is redundant.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a948099a290129e61060787de10966ef3763a197","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        except (OSError, IOError):"},{"line_number":83,"context_line":"            LOG.error(_LE(\u0027Error while writing HA state for %s\u0027),"},{"line_number":84,"context_line":"                      self.router_id)"},{"line_number":85,"context_line":"            return None"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _init_keepalived_manager(self, process_monitor):"},{"line_number":88,"context_line":"        self.keepalived_manager \u003d keepalived.KeepalivedManager("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_4d940444","line":85,"in_reply_to":"9a80dd14_35972eed","updated":"2015-03-18 20:19:50.000000000","message":"Oy, silly copy/paste from the method above... This method never returns anything, the return None really is redundant.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"}],"neutron/agent/l3/keepalived_state_change.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7783bdf533960381a0e06c187d95bc747db0ec12","unresolved":false,"context_lines":[{"line_number":15,"context_line":"import os"},{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import eventlet"},{"line_number":19,"context_line":"eventlet.monkey_patch()"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import httplib2"}],"source_content_type":"text/x-python","patch_set":23,"id":"da86d52c_f4225cff","line":18,"updated":"2015-02-13 00:48:35.000000000","message":"please consider following the same convention as Ihar\u0027s chain of changes:\n\nhttps://review.openstack.org/#/c/155373\n\nI\u0027d rather have all the mains in one place, rather than scattered around, and split main logic from the rest.\n\nJust something to wish for...","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"3d062d3c6143c0b91daae2eef462a1abe7ca1a01","unresolved":false,"context_lines":[{"line_number":15,"context_line":"import os"},{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import eventlet"},{"line_number":19,"context_line":"eventlet.monkey_patch()"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import httplib2"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_b34ccc43","line":18,"in_reply_to":"ba7be1f8_b873fa13","updated":"2015-03-06 21:40:40.000000000","message":"Done","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"import os"},{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import eventlet"},{"line_number":19,"context_line":"eventlet.monkey_patch()"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import httplib2"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_b873fa13","line":18,"in_reply_to":"da86d52c_f4225cff","updated":"2015-02-24 02:06:35.000000000","message":"Still need to address this.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":60,"context_line":"        # Old style super initialization is required!"},{"line_number":61,"context_line":"        namespace_proxy.UnixDomainHTTPConnection.__init__("},{"line_number":62,"context_line":"            self, *args, **kwargs)"},{"line_number":63,"context_line":"        self.socket_path \u003d cfg.CONF.KEEPALIVED_STATE_CHANGE.socket"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_b8e13a9c","line":62,"updated":"2015-02-24 02:06:35.000000000","message":"Looks like I missed changing KEEPALIVED_STATE_CHANGE to lower case here. Akihiro, this is why the change to UnixDomainHTTPConnection is needed.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        super(MonitorDaemon, self).__init__(pidfile, uuid\u003drouter_id)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def run(self):"},{"line_number":83,"context_line":"        super(MonitorDaemon, self).run()"},{"line_number":84,"context_line":"        # run_as_root\u003dFalse as this process is already running as root"},{"line_number":85,"context_line":"        monitor \u003d ip_monitor.IPMonitor(namespace\u003dcfg.CONF.namespace,"},{"line_number":86,"context_line":"                                       run_as_root\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_8c0360b7","line":83,"updated":"2015-03-02 13:13:25.000000000","message":"iirc, super.run() drop privileges (when user/group will be provided) ... it seems the drop should be done after opening the socket (monitor.start())","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        super(MonitorDaemon, self).__init__(pidfile, uuid\u003drouter_id)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def run(self):"},{"line_number":83,"context_line":"        super(MonitorDaemon, self).run()"},{"line_number":84,"context_line":"        # run_as_root\u003dFalse as this process is already running as root"},{"line_number":85,"context_line":"        monitor \u003d ip_monitor.IPMonitor(namespace\u003dcfg.CONF.namespace,"},{"line_number":86,"context_line":"                                       run_as_root\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_3cb34e0b","line":83,"in_reply_to":"ba7be1f8_8c0360b7","updated":"2015-03-06 21:01:39.000000000","message":"Dropped privileges right after monitor.start (Which performs a netns command).","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"0e7b5a1b4b5a2dfa16faec493102c592608ab3a5","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def write_state_change(router_id, state):"},{"line_number":34,"context_line":"    with open(os.path.join("},{"line_number":35,"context_line":"            cfg.CONF.conf_dir, \u0027state\u0027), \u0027w\u0027) as state_file:"},{"line_number":36,"context_line":"        state_file.write(state)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_f57af6b8","line":33,"updated":"2015-03-18 04:24:20.000000000","message":"(No action required) Testing would be cleaner if these files were part of the MonitorDaemon and it could be configured locally rather than globally.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a948099a290129e61060787de10966ef3763a197","unresolved":false,"context_lines":[{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def write_state_change(router_id, state):"},{"line_number":34,"context_line":"    with open(os.path.join("},{"line_number":35,"context_line":"            cfg.CONF.conf_dir, \u0027state\u0027), \u0027w\u0027) as state_file:"},{"line_number":36,"context_line":"        state_file.write(state)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_637b9929","line":33,"in_reply_to":"9a80dd14_f57af6b8","updated":"2015-03-18 20:19:50.000000000","message":"Done","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"0e7b5a1b4b5a2dfa16faec493102c592608ab3a5","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def write_state_change(router_id, state):"},{"line_number":34,"context_line":"    with open(os.path.join("},{"line_number":35,"context_line":"            cfg.CONF.conf_dir, \u0027state\u0027), \u0027w\u0027) as state_file:"},{"line_number":36,"context_line":"        state_file.write(state)"},{"line_number":37,"context_line":"    LOG.debug(\u0027Wrote router %s state %s\u0027, router_id, state)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_30765ca4","line":34,"updated":"2015-03-18 04:24:20.000000000","message":"If an exception occurs during this file operation, the monitor process would die.  Though the process monitor can be relied upon to restart, would the error be logged to aid in diagnosis?","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a948099a290129e61060787de10966ef3763a197","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def write_state_change(router_id, state):"},{"line_number":34,"context_line":"    with open(os.path.join("},{"line_number":35,"context_line":"            cfg.CONF.conf_dir, \u0027state\u0027), \u0027w\u0027) as state_file:"},{"line_number":36,"context_line":"        state_file.write(state)"},{"line_number":37,"context_line":"    LOG.debug(\u0027Wrote router %s state %s\u0027, router_id, state)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_b533f511","line":34,"in_reply_to":"9a80dd14_30765ca4","updated":"2015-03-18 20:19:50.000000000","message":"Done","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b5035d4e3e8f08737e7fd043b7a5eb20827a9c68","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):"},{"line_number":56,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":57,"context_line":"        # Old style super initialization is required!"},{"line_number":58,"context_line":"        agent_utils.UnixDomainHTTPConnection.__init__("},{"line_number":59,"context_line":"            self, *args, **kwargs)"},{"line_number":60,"context_line":"        self.socket_path \u003d ("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_c7a8ecdb","line":57,"updated":"2015-03-18 02:17:27.000000000","message":"how so? Isn\u0027t this technical baggage we\u0027d want to clean at some point?","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7b43fb8a2e9069c09ea963d411d9c4f2f7d52d5f","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):"},{"line_number":56,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":57,"context_line":"        # Old style super initialization is required!"},{"line_number":58,"context_line":"        agent_utils.UnixDomainHTTPConnection.__init__("},{"line_number":59,"context_line":"            self, *args, **kwargs)"},{"line_number":60,"context_line":"        self.socket_path \u003d ("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_7be995b6","line":57,"in_reply_to":"9a80dd14_67af407e","updated":"2015-03-18 05:36:45.000000000","message":"where\u0027s will there\u0027s a way...","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"5876d4cddae3c02f637d4113eab202b0f5469441","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):"},{"line_number":56,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":57,"context_line":"        # Old style super initialization is required!"},{"line_number":58,"context_line":"        agent_utils.UnixDomainHTTPConnection.__init__("},{"line_number":59,"context_line":"            self, *args, **kwargs)"},{"line_number":60,"context_line":"        self.socket_path \u003d ("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_ec11315e","line":57,"in_reply_to":"9a80dd14_7be995b6","updated":"2015-03-18 17:35:40.000000000","message":"iirc:\n\n  class NewStyle(object, OldStyle):\n    ...\n\ntransforms an old style class into a new one.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"1ed8e983e136dbfedc69de4a483891a2084ac998","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):"},{"line_number":56,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":57,"context_line":"        # Old style super initialization is required!"},{"line_number":58,"context_line":"        agent_utils.UnixDomainHTTPConnection.__init__("},{"line_number":59,"context_line":"            self, *args, **kwargs)"},{"line_number":60,"context_line":"        self.socket_path \u003d ("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_67af407e","line":57,"in_reply_to":"9a80dd14_c7a8ecdb","updated":"2015-03-18 02:25:03.000000000","message":"I don\u0027t know if we can. If you follow up the inheritance tree, the http2 base class is old-style.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"aef600749c3234f88c7092b34e519586c3346378","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"class KeepalivedUnixDomainConnection(agent_utils.UnixDomainHTTPConnection):"},{"line_number":56,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":57,"context_line":"        # Old style super initialization is required!"},{"line_number":58,"context_line":"        agent_utils.UnixDomainHTTPConnection.__init__("},{"line_number":59,"context_line":"            self, *args, **kwargs)"},{"line_number":60,"context_line":"        self.socket_path \u003d ("}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_b0014306","line":57,"in_reply_to":"9a80dd14_ec11315e","updated":"2015-03-18 20:21:16.000000000","message":"Thanks Cedric. I think this should go in a follow up though, it\u0027s the same case in the metadata proxy.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b5035d4e3e8f08737e7fd043b7a5eb20827a9c68","unresolved":false,"context_lines":[{"line_number":65,"context_line":"def handle_event(event):"},{"line_number":66,"context_line":"    if (event.interface \u003d\u003d cfg.CONF.monitor_interface and"},{"line_number":67,"context_line":"            event.cidr \u003d\u003d cfg.CONF.monitor_cidr):"},{"line_number":68,"context_line":"        new_state \u003d \u0027master\u0027 if event.added else \u0027backup\u0027"},{"line_number":69,"context_line":"        write_state_change(cfg.CONF.router_id, new_state)"},{"line_number":70,"context_line":"        notify_agent(cfg.CONF.router_id, new_state)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_a7dbf87b","line":68,"updated":"2015-03-18 02:17:27.000000000","message":"weren\u0027t we using master/standby in l3_hamode_db? This is a bit confusing.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7b43fb8a2e9069c09ea963d411d9c4f2f7d52d5f","unresolved":false,"context_lines":[{"line_number":65,"context_line":"def handle_event(event):"},{"line_number":66,"context_line":"    if (event.interface \u003d\u003d cfg.CONF.monitor_interface and"},{"line_number":67,"context_line":"            event.cidr \u003d\u003d cfg.CONF.monitor_cidr):"},{"line_number":68,"context_line":"        new_state \u003d \u0027master\u0027 if event.added else \u0027backup\u0027"},{"line_number":69,"context_line":"        write_state_change(cfg.CONF.router_id, new_state)"},{"line_number":70,"context_line":"        notify_agent(cfg.CONF.router_id, new_state)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_dbd9a123","line":68,"in_reply_to":"9a80dd14_0770443f","updated":"2015-03-18 05:36:45.000000000","message":"that\u0027s still a source of confusion, a note would help","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"1ed8e983e136dbfedc69de4a483891a2084ac998","unresolved":false,"context_lines":[{"line_number":65,"context_line":"def handle_event(event):"},{"line_number":66,"context_line":"    if (event.interface \u003d\u003d cfg.CONF.monitor_interface and"},{"line_number":67,"context_line":"            event.cidr \u003d\u003d cfg.CONF.monitor_cidr):"},{"line_number":68,"context_line":"        new_state \u003d \u0027master\u0027 if event.added else \u0027backup\u0027"},{"line_number":69,"context_line":"        write_state_change(cfg.CONF.router_id, new_state)"},{"line_number":70,"context_line":"        notify_agent(cfg.CONF.router_id, new_state)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_0770443f","line":68,"in_reply_to":"9a80dd14_a7dbf87b","updated":"2015-03-18 02:25:03.000000000","message":"The agent uses the keepalived / VRRP terminology (And has been before this change). These terms are then translated (Later in the patch series) when updates are sent to the controller.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"aef600749c3234f88c7092b34e519586c3346378","unresolved":false,"context_lines":[{"line_number":65,"context_line":"def handle_event(event):"},{"line_number":66,"context_line":"    if (event.interface \u003d\u003d cfg.CONF.monitor_interface and"},{"line_number":67,"context_line":"            event.cidr \u003d\u003d cfg.CONF.monitor_cidr):"},{"line_number":68,"context_line":"        new_state \u003d \u0027master\u0027 if event.added else \u0027backup\u0027"},{"line_number":69,"context_line":"        write_state_change(cfg.CONF.router_id, new_state)"},{"line_number":70,"context_line":"        notify_agent(cfg.CONF.router_id, new_state)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_300d5336","line":68,"in_reply_to":"9a80dd14_dbd9a123","updated":"2015-03-18 20:21:16.000000000","message":"I don\u0027t know what to do about this. I mean these terms are used all over the L3 agent (And now here).","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"667d0a1fbc22b01906025507edd7643f6331114d","unresolved":false,"context_lines":[{"line_number":11,"context_line":"#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import os"},{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_d025d75c","line":14,"updated":"2015-03-18 20:29:28.000000000","message":"Notable changes from the revision last reviewed: Classified everything up + Error handling","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28b983185deb17a65d23a8c2f7a0f29ad62d5a4c","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import httplib2"},{"line_number":19,"context_line":"from neutron.i18n import _LE"},{"line_number":20,"context_line":"from oslo_config import cfg"},{"line_number":21,"context_line":"from oslo_log import log as logging"},{"line_number":22,"context_line":"import requests"}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_00b2bce4","line":19,"updated":"2015-03-18 22:29:24.000000000","message":"nit: wrong section","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"e0164375fcfb03779a2e4d37184424231c93537f","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import sys"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import httplib2"},{"line_number":19,"context_line":"from neutron.i18n import _LE"},{"line_number":20,"context_line":"from oslo_config import cfg"},{"line_number":21,"context_line":"from oslo_log import log as logging"},{"line_number":22,"context_line":"import requests"}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_2b26c116","line":19,"in_reply_to":"9a80dd14_00b2bce4","updated":"2015-03-18 23:12:49.000000000","message":"NoooOooOoooOooo","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"667d0a1fbc22b01906025507edd7643f6331114d","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            if event.interface \u003d\u003d self.interface and event.cidr \u003d\u003d self.cidr:"},{"line_number":72,"context_line":"                new_state \u003d \u0027master\u0027 if event.added else \u0027backup\u0027"},{"line_number":73,"context_line":"                self.write_state_change(new_state)"},{"line_number":74,"context_line":"                self.notify_agent(new_state)"},{"line_number":75,"context_line":"        except Exception:"},{"line_number":76,"context_line":"            LOG.exception(_LE("},{"line_number":77,"context_line":"                \u0027Failed to process or handle event for line %s\u0027), iterable)"}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_d0b31708","line":74,"updated":"2015-03-18 20:29:28.000000000","message":"This process shouldn\u0027t crash even a single event failed (For whatever weird reason), it should continue on to the next event. That\u0027s the reason for the broad \u0027except\u0027 clause. Think of web servers returning error 500 instead of crashing to hell.","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"}],"neutron/agent/l3/router_info.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85c20cc0c8253c6e5f34cec74eb53e5348054a50","unresolved":false,"context_lines":[{"line_number":18,"context_line":"class RouterInfo(object):"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    def __init__(self, router_id, root_helper, router,"},{"line_number":21,"context_line":"                 use_ipv6\u003dFalse, ns_name\u003dNone, conf\u003dNone):"},{"line_number":22,"context_line":"        self.router_id \u003d router_id"},{"line_number":23,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":24,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa81d914_a8b92530","line":21,"updated":"2015-01-28 17:25:33.000000000","message":"I can see why you did this to avoid changing test_l3_agent.py.  However, it isn\u0027t really right to make passing the agent\u0027s conf optional.","commit_id":"ba27fc6a31b4a03973a69ce605d259add4d3135a"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"71d9aa897c7f3e0a2d338755282daed05e8a0be5","unresolved":false,"context_lines":[{"line_number":18,"context_line":"class RouterInfo(object):"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    def __init__(self, router_id, root_helper, router,"},{"line_number":21,"context_line":"                 use_ipv6\u003dFalse, ns_name\u003dNone, conf\u003dNone):"},{"line_number":22,"context_line":"        self.router_id \u003d router_id"},{"line_number":23,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":24,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa81d914_08011f05","line":21,"in_reply_to":"fa81d914_a8b92530","updated":"2015-01-29 13:49:46.000000000","message":"We should be good now with your patch merged.","commit_id":"ba27fc6a31b4a03973a69ce605d259add4d3135a"}],"neutron/agent/l3_agent.py":[{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"change_message_id":"658d3c768ca9de978605b6e44b2636ea301fcd58","unresolved":false,"context_lines":[{"line_number":772,"context_line":"            self._destroy_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":773,"context_line":"        del self.router_info[router_id]"},{"line_number":774,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"    @classmethod"},{"line_number":777,"context_line":"    def _get_metadata_proxy_callback(cls, router_id, conf):"},{"line_number":778,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_4be30b05","line":775,"updated":"2014-10-03 04:09:01.000000000","message":"is this related change?","commit_id":"c0f12d89ce41df86b72b5bf1c32be0d76762b1c9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"552d932731b682ec8373ed8eff0c0ee0e616cbef","unresolved":false,"context_lines":[{"line_number":772,"context_line":"            self._destroy_metadata_proxy(ri.router_id, ri.ns_name)"},{"line_number":773,"context_line":"        del self.router_info[router_id]"},{"line_number":774,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":775,"context_line":""},{"line_number":776,"context_line":"    @classmethod"},{"line_number":777,"context_line":"    def _get_metadata_proxy_callback(cls, router_id, conf):"},{"line_number":778,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_d6b5dbee","line":775,"in_reply_to":"baa201ad_4be30b05","updated":"2014-10-03 08:25:54.000000000","message":"I need to spawn and disable the metadata proxy in the new script defined in this patch:\n\nneutron/cmd/keepalived_state_change.py","commit_id":"c0f12d89ce41df86b72b5bf1c32be0d76762b1c9"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":800,"context_line":"            ns_name)"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    @classmethod"},{"line_number":803,"context_line":"    def _spawn_metadata_proxy(cls, router_id, ns_name, conf\u003dcfg.CONF):"},{"line_number":804,"context_line":"        callback \u003d cls._get_metadata_proxy_callback(router_id, conf)"},{"line_number":805,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(router_id, ns_name, conf)"},{"line_number":806,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a701eab0","line":803,"updated":"2014-10-05 14:16:51.000000000","message":"Post-rebase note: need to make sure this plays well with https://review.openstack.org/#/c/124752/\n\nMy instinct will be to make \u0027conf\u0027 a non-positional argument (and make sure that it is called with self.conf only).","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":800,"context_line":"            ns_name)"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    @classmethod"},{"line_number":803,"context_line":"    def _spawn_metadata_proxy(cls, router_id, ns_name, conf\u003dcfg.CONF):"},{"line_number":804,"context_line":"        callback \u003d cls._get_metadata_proxy_callback(router_id, conf)"},{"line_number":805,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(router_id, ns_name, conf)"},{"line_number":806,"context_line":"        pm.enable(callback)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_e762123d","line":803,"in_reply_to":"baa201ad_a701eab0","updated":"2014-10-05 16:04:32.000000000","message":"Good catch.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"425bcf78e95d407907374157207a697167601940","unresolved":false,"context_lines":[{"line_number":774,"context_line":"        del self.router_info[router_id]"},{"line_number":775,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @classmethod"},{"line_number":778,"context_line":"    def _get_metadata_proxy_callback(cls, router_id, conf):"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_8d159d9d","line":777,"updated":"2014-11-18 01:06:19.000000000","message":"why did you changes these? Where are we using these?","commit_id":"5232aefea9cb4c3475a14b119319ea8503753442"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"bcc515227a56cbddaaadffd768c31cb9e2105698","unresolved":false,"context_lines":[{"line_number":774,"context_line":"        del self.router_info[router_id]"},{"line_number":775,"context_line":"        self._destroy_router_namespace(ri.ns_name)"},{"line_number":776,"context_line":""},{"line_number":777,"context_line":"    @classmethod"},{"line_number":778,"context_line":"    def _get_metadata_proxy_callback(cls, router_id, conf):"},{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        def callback(pid_file):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_e693604d","line":777,"in_reply_to":"5a890539_8d159d9d","updated":"2014-11-19 15:19:17.000000000","message":"neutron/cmd/keepalived_state_change.py","commit_id":"5232aefea9cb4c3475a14b119319ea8503753442"}],"neutron/agent/l3_ha_agent.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        router_directory \u003d ri.keepalived_manager.get_conf_dir()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        # Create this router\u0027s external PID directory with the agent\u0027s"},{"line_number":187,"context_line":"        # user \u0026 permissions"},{"line_number":188,"context_line":"        pm \u003d self._get_metadata_proxy_process_manager(ri.router_id,"},{"line_number":189,"context_line":"                                                      ri.ns_name, self.conf)"},{"line_number":190,"context_line":"        pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_4713de0c","line":187,"updated":"2014-10-05 14:16:51.000000000","message":"I don\u0027t like the way this is done now (accessing metadata PM to make sure the permissions are valid, etc).\n\nWhat about having KeepalivedManager inherit from ProcessMonitor so ri.keepalived_manager could provide this functionality?","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        router_directory \u003d ri.keepalived_manager.get_conf_dir()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        # Create this router\u0027s external PID directory with the agent\u0027s"},{"line_number":187,"context_line":"        # user \u0026 permissions"},{"line_number":188,"context_line":"        pm \u003d self._get_metadata_proxy_process_manager(ri.router_id,"},{"line_number":189,"context_line":"                                                      ri.ns_name, self.conf)"},{"line_number":190,"context_line":"        pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_475e7e77","line":187,"in_reply_to":"baa201ad_4713de0c","updated":"2014-10-05 16:04:32.000000000","message":"What is it that you don\u0027t like exactly? Any specific reason?\n\nThere\u0027s no reason for KeepalivedManager to inherit from ProccessManager. It uses it internally and provides similar methods (Spawn vs enable, for example). I don\u0027t want KeepalivedManager to expose \u0027enable\u0027.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"11515ee224c0e05b73d1708d648d4fcf44ea3d49","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        router_directory \u003d ri.keepalived_manager.get_conf_dir()"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        # Create this router\u0027s external PID directory with the agent\u0027s"},{"line_number":187,"context_line":"        # user \u0026 permissions"},{"line_number":188,"context_line":"        pm \u003d self._get_metadata_proxy_process_manager(ri.router_id,"},{"line_number":189,"context_line":"                                                      ri.ns_name, self.conf)"},{"line_number":190,"context_line":"        pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_0bc38089","line":187,"in_reply_to":"baa201ad_475e7e77","updated":"2014-10-07 08:10:20.000000000","message":"The way it is now, in order to ensure that keepalived_manager can operate correctly, you use metadata proxy\u0027s PM functions to ensure filesystem status.\n\nIt\u0027s like using A to do X, but using B to setup the groundworks, even though A and B have nothing in common (excpet they both handle processes).","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                                                      ri.ns_name, self.conf)"},{"line_number":190,"context_line":"        pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        full_path \u003d distutils.spawn.find_executable("},{"line_number":193,"context_line":"            \u0027neutron-keepalived-state-change\u0027)"},{"line_number":194,"context_line":"        for state in keepalived.VALID_NOTIFY_STATES:"},{"line_number":195,"context_line":"            command \u003d (\u0027%s --router_id\u003d%s --router_namespace\u003d%s --state\u003d%s \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_67fb82c1","line":192,"updated":"2014-10-05 14:16:51.000000000","message":"nit: \u0027path\u0027 is enough as it is usually full enough :)","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                                                      ri.ns_name, self.conf)"},{"line_number":190,"context_line":"        pm.get_pid_file_name(ensure_pids_dir\u003dTrue)"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        full_path \u003d distutils.spawn.find_executable("},{"line_number":193,"context_line":"            \u0027neutron-keepalived-state-change\u0027)"},{"line_number":194,"context_line":"        for state in keepalived.VALID_NOTIFY_STATES:"},{"line_number":195,"context_line":"            command \u003d (\u0027%s --router_id\u003d%s --router_namespace\u003d%s --state\u003d%s \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_6756225e","line":192,"in_reply_to":"baa201ad_67fb82c1","updated":"2014-10-05 16:04:32.000000000","message":"full_path acts as a comment as to why we even need to use find_executable.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"}],"neutron/agent/linux/ip_monitor.py":[{"author":{"_account_id":7921,"name":"Mike Kolesnik","email":"mkolesni@redhat.com","username":"mkolesni"},"change_message_id":"1c8fd4c4335807892a35ff11c6358798abbeb916","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def __init__(self,"},{"line_number":71,"context_line":"                 namespace\u003dNone,"},{"line_number":72,"context_line":"                 run_as_root\u003dTrue,"},{"line_number":73,"context_line":"                 respawn_interval\u003dNone):"},{"line_number":74,"context_line":"        super(IPMonitor, self).__init__([\u0027ip\u0027, \u0027-o\u0027, \u0027monitor\u0027, \u0027address\u0027],"},{"line_number":75,"context_line":"                                        run_as_root\u003drun_as_root,"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_fa041e16","line":72,"updated":"2015-03-15 13:01:03.000000000","message":"Why is this necessary to expose now?\n\nIsn\u0027t your code the only one using it, sending always False?","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"783fe6165e0b716e5dd58ac1b40384ce8df2c15a","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def __init__(self,"},{"line_number":71,"context_line":"                 namespace\u003dNone,"},{"line_number":72,"context_line":"                 run_as_root\u003dTrue,"},{"line_number":73,"context_line":"                 respawn_interval\u003dNone):"},{"line_number":74,"context_line":"        super(IPMonitor, self).__init__([\u0027ip\u0027, \u0027-o\u0027, \u0027monitor\u0027, \u0027address\u0027],"},{"line_number":75,"context_line":"                                        run_as_root\u003drun_as_root,"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_126c1708","line":72,"in_reply_to":"9a80dd14_fa041e16","updated":"2015-03-17 17:38:42.000000000","message":"The functional tests don\u0027t run as root so they construct an monitor with run_as_root\u003dTrue.","commit_id":"6c26232e8e2768bf6fc7d244b5dfb5701c76b00f"}],"neutron/agent/linux/keepalived.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                               (\u0027        %s\u0027 % i for i in self.instance_names),"},{"line_number":99,"context_line":"                               [\u0027    }\u0027],"},{"line_number":100,"context_line":"                               (\u0027    notify_%s \"%s\"\u0027 % (state, path)"},{"line_number":101,"context_line":"                                for state, path in self.notifiers),"},{"line_number":102,"context_line":"                               [\u0027}\u0027])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_07d8f679","line":101,"updated":"2014-10-05 14:16:51.000000000","message":"self.notifiers now contain (state, path) tuples, so this should be changed accordingly.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                               (\u0027        %s\u0027 % i for i in self.instance_names),"},{"line_number":99,"context_line":"                               [\u0027    }\u0027],"},{"line_number":100,"context_line":"                               (\u0027    notify_%s \"%s\"\u0027 % (state, path)"},{"line_number":101,"context_line":"                                for state, path in self.notifiers),"},{"line_number":102,"context_line":"                               [\u0027}\u0027])"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a74c8ac9","line":101,"in_reply_to":"baa201ad_07d8f679","updated":"2014-10-05 16:04:32.000000000","message":"I assume you mean (state, command)?\n\nDone.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"}],"neutron/agent/metadata/driver.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7783bdf533960381a0e06c187d95bc747db0ec12","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        router.iptables_manager.apply()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        if not router.is_ha:"},{"line_number":65,"context_line":"            self.spawn_monitored_metadata_proxy(router.router_id,"},{"line_number":66,"context_line":"                                                router.ns_name)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def before_router_removed(self, router):"}],"source_content_type":"text/x-python","patch_set":23,"id":"da86d52c_54f08866","line":65,"updated":"2015-02-13 00:48:35.000000000","message":"I think these changes deserve an explanation; don\u0027t you?","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        router.iptables_manager.apply()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        if not router.is_ha:"},{"line_number":65,"context_line":"            self.spawn_monitored_metadata_proxy(router.router_id,"},{"line_number":66,"context_line":"                                                router.ns_name)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def before_router_removed(self, router):"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_7850a25e","line":65,"in_reply_to":"da86d52c_54f08866","updated":"2015-02-24 02:06:35.000000000","message":"These driver methods are now used by the agent, so I removed the underscore.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"}],"neutron/agent/metadata/namespace_proxy.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"a4314a7f01f1441f284d4eba67406b2551f07661","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.sock \u003d socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)"},{"line_number":46,"context_line":"        if self.timeout:"},{"line_number":47,"context_line":"            self.sock.settimeout(self.timeout)"},{"line_number":48,"context_line":"        self.sock.connect(self.socket_path)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class NetworkMetadataProxyHandler(object):"}],"source_content_type":"text/x-python","patch_set":23,"id":"da86d52c_98867815","line":48,"updated":"2015-02-14 23:09:18.000000000","message":"Why did you make this change?","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"ab785dba356cdac1c03f60ac62c8d779025680a8","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.sock \u003d socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)"},{"line_number":46,"context_line":"        if self.timeout:"},{"line_number":47,"context_line":"            self.sock.settimeout(self.timeout)"},{"line_number":48,"context_line":"        self.sock.connect(self.socket_path)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class NetworkMetadataProxyHandler(object):"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_8b1634a6","line":48,"in_reply_to":"ba7be1f8_f8cbb21b","updated":"2015-02-27 10:22:22.000000000","message":"Thanks for the clarification.\nSorry I forgot to remove this comment after reading the whole patch.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"58446ac59fc61ed2b747fb23a6ef62daed7d994a","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self.sock \u003d socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)"},{"line_number":46,"context_line":"        if self.timeout:"},{"line_number":47,"context_line":"            self.sock.settimeout(self.timeout)"},{"line_number":48,"context_line":"        self.sock.connect(self.socket_path)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"class NetworkMetadataProxyHandler(object):"}],"source_content_type":"text/x-python","patch_set":23,"id":"ba7be1f8_f8cbb21b","line":48,"in_reply_to":"da86d52c_98867815","updated":"2015-02-24 02:06:35.000000000","message":"So that I could build UnixDomainHTTPConnection with a different socket_path in the keepalived_state_change script.","commit_id":"0d25b59bdd2cd9401abd1120739c891abb000f47"}],"neutron/cmd/keepalived_state_change.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                self.router_id, self.ns_name)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"def _configure():"},{"line_number":59,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_id\u0027,"},{"line_number":60,"context_line":"                                         help\u003d\u0027ID of the router\u0027))"},{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_076636c1","line":58,"updated":"2014-10-05 14:16:51.000000000","message":"I wonder why you didn\u0027t use the OPTS variable like other modules did (for consistency)?","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                self.router_id, self.ns_name)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"def _configure():"},{"line_number":59,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_id\u0027,"},{"line_number":60,"context_line":"                                         help\u003d\u0027ID of the router\u0027))"},{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_87da26e6","line":58,"in_reply_to":"baa201ad_076636c1","updated":"2014-10-05 16:04:32.000000000","message":"It\u0027s a CLI opt. I don\u0027t care much either way.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"def _configure():"},{"line_number":59,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_id\u0027,"},{"line_number":60,"context_line":"                                         help\u003d\u0027ID of the router\u0027))"},{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"},{"line_number":62,"context_line":"                                         help\u003d\u0027Namespace of the router\u0027))"},{"line_number":63,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027state\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_e76052a7","line":60,"updated":"2014-10-05 14:16:51.000000000","message":"All these help strings should have _() wrappings.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"def _configure():"},{"line_number":59,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_id\u0027,"},{"line_number":60,"context_line":"                                         help\u003d\u0027ID of the router\u0027))"},{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"},{"line_number":62,"context_line":"                                         help\u003d\u0027Namespace of the router\u0027))"},{"line_number":63,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027state\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_07f63663","line":60,"in_reply_to":"baa201ad_e76052a7","updated":"2014-10-05 16:04:32.000000000","message":"Done","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"},{"line_number":62,"context_line":"                                         help\u003d\u0027Namespace of the router\u0027))"},{"line_number":63,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027state\u0027,"},{"line_number":64,"context_line":"                                         help\u003d\u0027The new HA state\u0027))"},{"line_number":65,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027conf_dir\u0027,"},{"line_number":66,"context_line":"                                         help\u003d\u0027Path to the router directory\u0027))"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_c750aed6","line":64,"updated":"2014-10-05 14:16:51.000000000","message":"Specify which states are acceptable for \u0027state\u0027 with the choices\u003d[] argument, since \"state\u003dcrap\" doesn\u0027t seem to be a valid one.\n\nAlso, I think you need to explicitly make sure that all of these variables were given by the user (but I didn\u0027t check if oslo config checks this automatically anyway).","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027router_namespace\u0027,"},{"line_number":62,"context_line":"                                         help\u003d\u0027Namespace of the router\u0027))"},{"line_number":63,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027state\u0027,"},{"line_number":64,"context_line":"                                         help\u003d\u0027The new HA state\u0027))"},{"line_number":65,"context_line":"    cfg.CONF.register_cli_opt(cfg.StrOpt(\u0027conf_dir\u0027,"},{"line_number":66,"context_line":"                                         help\u003d\u0027Path to the router directory\u0027))"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_e7f05269","line":64,"in_reply_to":"baa201ad_c750aed6","updated":"2014-10-05 16:04:32.000000000","message":"This isn\u0027t meant to be used by a \u0027user\u0027, only by keepalived. The functional test verifies that all parameters were received, there\u0027s no need to do this in here.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    agent_config.register_root_helper(cfg.CONF)"},{"line_number":80,"context_line":"    config.init(sys.argv[1:])"},{"line_number":81,"context_line":"    cfg.CONF.set_override(\u0027log_dir\u0027, cfg.CONF.conf_dir)"},{"line_number":82,"context_line":"    cfg.CONF.set_override(\u0027debug\u0027, True)"},{"line_number":83,"context_line":"    cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":84,"context_line":"    config.setup_logging()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a753aae0","line":81,"updated":"2014-10-05 14:16:51.000000000","message":"I think these 3 variables should be up to the user to config as well (since I imagine a case where the debug logs will spam a lot and someone will want to turn the logs off manually/in a patch).\n\nAlso, you never use verbose here?","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    agent_config.register_root_helper(cfg.CONF)"},{"line_number":80,"context_line":"    config.init(sys.argv[1:])"},{"line_number":81,"context_line":"    cfg.CONF.set_override(\u0027log_dir\u0027, cfg.CONF.conf_dir)"},{"line_number":82,"context_line":"    cfg.CONF.set_override(\u0027debug\u0027, True)"},{"line_number":83,"context_line":"    cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":84,"context_line":"    config.setup_logging()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_67dd42e0","line":81,"in_reply_to":"baa201ad_a753aae0","updated":"2014-10-05 16:04:32.000000000","message":"I don\u0027t see a good reason for this. If you can think of a valid use case of disabling logging (To a separate, per HA router file) please share.","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def main():"},{"line_number":93,"context_line":"    _configure()"},{"line_number":94,"context_line":"    eventlet.monkey_patch()"},{"line_number":95,"context_line":"    StateChangeEvent(cfg.CONF.router_id,"},{"line_number":96,"context_line":"                     cfg.CONF.router_namespace,"},{"line_number":97,"context_line":"                     cfg.CONF.state,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_67746268","line":94,"updated":"2014-10-05 14:16:51.000000000","message":"I think the monkey_patch() call should be done immediately after the import is done (this is done in all the other calls to monkey_patch throughout the code).","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"def main():"},{"line_number":93,"context_line":"    _configure()"},{"line_number":94,"context_line":"    eventlet.monkey_patch()"},{"line_number":95,"context_line":"    StateChangeEvent(cfg.CONF.router_id,"},{"line_number":96,"context_line":"                     cfg.CONF.router_namespace,"},{"line_number":97,"context_line":"                     cfg.CONF.state,"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_e7c93227","line":94,"in_reply_to":"baa201ad_67746268","updated":"2014-10-05 16:04:32.000000000","message":"Done","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a67e2d73dbe411db795d6e470118195d4daa805c","unresolved":false,"context_lines":[{"line_number":92,"context_line":"def main():"},{"line_number":93,"context_line":"    _configure()"},{"line_number":94,"context_line":"    eventlet.monkey_patch()"},{"line_number":95,"context_line":"    StateChangeEvent(cfg.CONF.router_id,"},{"line_number":96,"context_line":"                     cfg.CONF.router_namespace,"},{"line_number":97,"context_line":"                     cfg.CONF.state,"},{"line_number":98,"context_line":"                     cfg.CONF.conf_dir)()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_a76aca7e","line":95,"updated":"2014-10-05 14:16:51.000000000","message":"I\u0027m not sure why a class is needed as it stands now. Basically you have 4 parameters which you pass around to 2 functions (which you might as well pass implicitly through cfg.CONF), so this doesn\u0027t really require a class of its own.\n\nIf you do keep the class, I think _configure should be a part of the class (probably as a staticmethod?)","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ae704b5a30fe424b35d28d9761b28ac3e984079","unresolved":false,"context_lines":[{"line_number":92,"context_line":"def main():"},{"line_number":93,"context_line":"    _configure()"},{"line_number":94,"context_line":"    eventlet.monkey_patch()"},{"line_number":95,"context_line":"    StateChangeEvent(cfg.CONF.router_id,"},{"line_number":96,"context_line":"                     cfg.CONF.router_namespace,"},{"line_number":97,"context_line":"                     cfg.CONF.state,"},{"line_number":98,"context_line":"                     cfg.CONF.conf_dir)()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa201ad_47d59eb7","line":95,"in_reply_to":"baa201ad_a76aca7e","updated":"2014-10-05 16:04:32.000000000","message":"_configure is part of the script itself, while StateChangeEvent represents a handler for a state change. A state change event definitely shouldn\u0027t know how to configure a root helper (For example).","commit_id":"c2f0ff4224faa2ea6090c8b1d56e8a9079636ade"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd50e335da3d9a1787f77f87ef2721963191ec00","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    agent_config.register_root_helper(cfg.CONF)"},{"line_number":83,"context_line":"    config.init(sys.argv[1:])"},{"line_number":84,"context_line":"    cfg.CONF.set_override(\u0027log_dir\u0027, cfg.CONF.conf_dir)"},{"line_number":85,"context_line":"    cfg.CONF.set_override(\u0027debug\u0027, True)"},{"line_number":86,"context_line":"    cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":87,"context_line":"    config.setup_logging()"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa201ad_caa459bb","line":84,"updated":"2014-10-14 13:50:18.000000000","message":"Why do you want all the logs sent to the configuration directory?","commit_id":"cdd349e56c6b4f78d4e7d497e95806b3f0a8a841"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"542cb77de844467264a0ccedf246f5cb6da42ffa","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    agent_config.register_root_helper(cfg.CONF)"},{"line_number":83,"context_line":"    config.init(sys.argv[1:])"},{"line_number":84,"context_line":"    cfg.CONF.set_override(\u0027log_dir\u0027, cfg.CONF.conf_dir)"},{"line_number":85,"context_line":"    cfg.CONF.set_override(\u0027debug\u0027, True)"},{"line_number":86,"context_line":"    cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":87,"context_line":"    config.setup_logging()"}],"source_content_type":"text/x-python","patch_set":6,"id":"baa201ad_c507ea7f","line":84,"in_reply_to":"baa201ad_caa459bb","updated":"2014-10-14 14:16:12.000000000","message":"cfg.CONF.conf_dir is per-router here.","commit_id":"cdd349e56c6b4f78d4e7d497e95806b3f0a8a841"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b5035d4e3e8f08737e7fd043b7a5eb20827a9c68","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"def main():"},{"line_number":19,"context_line":"    keepalived_state_change.main()"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_472ebc43","line":19,"updated":"2015-03-18 02:17:27.000000000","message":"so this doesn\u0027t need to be monkeypatched?","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"1ed8e983e136dbfedc69de4a483891a2084ac998","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"def main():"},{"line_number":19,"context_line":"    keepalived_state_change.main()"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_a7cf7856","line":19,"in_reply_to":"9a80dd14_472ebc43","updated":"2015-03-18 02:25:03.000000000","message":"Looking at what the script does, no, I don\u0027t think so.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7b43fb8a2e9069c09ea963d411d9c4f2f7d52d5f","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"def main():"},{"line_number":19,"context_line":"    keepalived_state_change.main()"}],"source_content_type":"text/x-python","patch_set":43,"id":"9a80dd14_3b063d86","line":19,"in_reply_to":"9a80dd14_a7cf7856","updated":"2015-03-18 05:36:45.000000000","message":"I am gonna quote you when you change your mind.","commit_id":"10b8df621aa119d07323f690ab695074f1c8d5a9"}],"neutron/tests/functional/agent/l3/test_keepalived_state_change.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4f9570b44d8076c789a8b9f0c1d885ed5122ae4d","unresolved":false,"context_lines":[{"line_number":43,"context_line":"            \u0027namespace\u0027,"},{"line_number":44,"context_line":"            self.conf_dir,"},{"line_number":45,"context_line":"            self.interface_name,"},{"line_number":46,"context_line":"            self.cidr)"},{"line_number":47,"context_line":"        mock.patch.object(self.monitor, \u0027notify_agent\u0027).start()"},{"line_number":48,"context_line":"        self.line \u003d \u00271: %s    inet %s\u0027 % (self.interface_name, self.cidr)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_506ba784","line":46,"updated":"2015-03-18 20:27:11.000000000","message":"I was trying to strike a balance here between testing depth and simplicity. Not mocking the notify method (Or its internal httplib2 usage, alternatively) means adding a fake (Or real) listener. Adding the real one proved a challenge because it performs a wait, so that\u0027d have to be started in a separate greenthread, and I don\u0027t think that\u0027s worth it. Adding a fake one would be similarly costly and I just don\u0027t see the pay off, since we have an explicit test for events in the L3 agent functional tests.","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"28b983185deb17a65d23a8c2f7a0f29ad62d5a4c","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        mock.patch.object(self.monitor, \u0027notify_agent\u0027).start()"},{"line_number":48,"context_line":"        self.line \u003d \u00271: %s    inet %s\u0027 % (self.interface_name, self.cidr)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_handle_event_wrong_device_or_cidr(self):"},{"line_number":51,"context_line":"        self.monitor.parse_and_handle_event("},{"line_number":52,"context_line":"            \u00271: wrong_device    inet wrong_cidr\u0027)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_a02bb0e9","line":50,"updated":"2015-03-18 22:29:24.000000000","message":"(No action required) Consider adding a suffix of \u0027completes without error\u0027 to indicate your intent.  Also, naming the test after the method under test (i.e. test_[method name]_[intent] makes finding coverage easier.","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"e0164375fcfb03779a2e4d37184424231c93537f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        mock.patch.object(self.monitor, \u0027notify_agent\u0027).start()"},{"line_number":48,"context_line":"        self.line \u003d \u00271: %s    inet %s\u0027 % (self.interface_name, self.cidr)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_handle_event_wrong_device_or_cidr(self):"},{"line_number":51,"context_line":"        self.monitor.parse_and_handle_event("},{"line_number":52,"context_line":"            \u00271: wrong_device    inet wrong_cidr\u0027)"},{"line_number":53,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"9a80dd14_ab79d1fd","line":50,"in_reply_to":"9a80dd14_a02bb0e9","updated":"2015-03-18 23:12:49.000000000","message":"You can see I renamed all of the other test methods and forgot about this one","commit_id":"7f3c84e1c103498856d0ab7f7d02f3918c4569f1"}],"neutron/tests/sub_base.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"987979b7fcbca6fbdf2bef8652243d25c22e1dc1","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            ))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        #test_timeout \u003d int(os.environ.get(\u0027OS_TEST_TIMEOUT\u0027, 0))"},{"line_number":81,"context_line":"        test_timeout \u003d 0"},{"line_number":82,"context_line":"        if test_timeout \u003d\u003d -1:"},{"line_number":83,"context_line":"            test_timeout \u003d 0"},{"line_number":84,"context_line":"        if test_timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba7be1f8_4c7c0821","line":81,"updated":"2015-03-02 13:13:25.000000000","message":"looks like a WIP :)","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5e3c7c2e0a626f218c7e1c899df0bbcf9f62114b","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            ))"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        #test_timeout \u003d int(os.environ.get(\u0027OS_TEST_TIMEOUT\u0027, 0))"},{"line_number":81,"context_line":"        test_timeout \u003d 0"},{"line_number":82,"context_line":"        if test_timeout \u003d\u003d -1:"},{"line_number":83,"context_line":"            test_timeout \u003d 0"},{"line_number":84,"context_line":"        if test_timeout \u003e 0:"}],"source_content_type":"text/x-python","patch_set":27,"id":"9a80dd14_217c0b83","line":81,"in_reply_to":"ba7be1f8_4c7c0821","updated":"2015-03-06 21:01:39.000000000","message":"I meant to do that...","commit_id":"3dc785089caf95cea979c4d4f51afb45d19c90a6"}]}