)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"28aecf2a8e94b39025ce8577652e5c6b601f87f8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"All launchers implemented in common.service require each service to"},{"line_number":10,"context_line":"implement reset method because it is called in case a process"},{"line_number":11,"context_line":"recieves a SIGHUP."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This change adds the reset method to neutron.service.RpcWorker and"},{"line_number":14,"context_line":"neutron.wsgi.WorkerService which are used to wrap rpc and api"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9aa53dc9_c7bf20cf","line":11,"updated":"2015-04-07 14:56:28.000000000","message":"receives?","commit_id":"3265702b6c40bca98a73b1f15cb6568ecc6665e7"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"b494fc6ae1feb41db92159ca7e46c8464c394b92","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"All launchers implemented in common.service require each service to"},{"line_number":10,"context_line":"implement reset method because it is called in case a process"},{"line_number":11,"context_line":"recieves a SIGHUP."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This change adds the reset method to neutron.service.RpcWorker and"},{"line_number":14,"context_line":"neutron.wsgi.WorkerService which are used to wrap rpc and api"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9aa53dc9_a7d7204a","line":11,"in_reply_to":"9aa53dc9_c7bf20cf","updated":"2015-04-09 12:17:18.000000000","message":"thanks, I\u0027ll fix it in the follow-up patchset","commit_id":"3265702b6c40bca98a73b1f15cb6568ecc6665e7"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"28aecf2a8e94b39025ce8577652e5c6b601f87f8","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  resetted and started again (see openstack.common.service code)."},{"line_number":27,"context_line":"  As graceful shutdown implies calling service.stop() and then"},{"line_number":28,"context_line":"  service.wait() we don\u0027t want to clean self._servers list because"},{"line_number":29,"context_line":"  it would the be impossible to wait for them to stop processing"},{"line_number":30,"context_line":"  requests and cleaning up their resources."},{"line_number":31,"context_line":"  Otherwise, this would lead to problems with rpc after starting"},{"line_number":32,"context_line":"  the rpc server again."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9aa53dc9_87ec78c8","line":29,"updated":"2015-04-07 14:56:28.000000000","message":"be the","commit_id":"3265702b6c40bca98a73b1f15cb6568ecc6665e7"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"b494fc6ae1feb41db92159ca7e46c8464c394b92","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  resetted and started again (see openstack.common.service code)."},{"line_number":27,"context_line":"  As graceful shutdown implies calling service.stop() and then"},{"line_number":28,"context_line":"  service.wait() we don\u0027t want to clean self._servers list because"},{"line_number":29,"context_line":"  it would the be impossible to wait for them to stop processing"},{"line_number":30,"context_line":"  requests and cleaning up their resources."},{"line_number":31,"context_line":"  Otherwise, this would lead to problems with rpc after starting"},{"line_number":32,"context_line":"  the rpc server again."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9aa53dc9_27c4308f","line":29,"in_reply_to":"9aa53dc9_87ec78c8","updated":"2015-04-09 12:17:18.000000000","message":"thanks, will fix it","commit_id":"3265702b6c40bca98a73b1f15cb6568ecc6665e7"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a71d9cd3acf5a39e0e6fa3321ceecd2d820cfbac","unresolved":false,"context_lines":[{"line_number":18,"context_line":"rpc_workers \u003e 0) and metadata agent don\u0027t die on receiving SIGHUP and support"},{"line_number":19,"context_line":"reloading policy_path and logging options in config."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Note that reset is called only in case a service is running in daemon mode."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Other changes made in the scope of this patch that need to be mentioned:"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_0e7f0fe1","line":21,"updated":"2015-05-29 13:22:14.000000000","message":"Sorry for my ignorance, but what does running in daemon mode specifically mean in terms of neutron-server?","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"44f65d365b942770ac5203f02731d1f62e27ee8b","unresolved":false,"context_lines":[{"line_number":18,"context_line":"rpc_workers \u003e 0) and metadata agent don\u0027t die on receiving SIGHUP and support"},{"line_number":19,"context_line":"reloading policy_path and logging options in config."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Note that reset is called only in case a service is running in daemon mode."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Other changes made in the scope of this patch that need to be mentioned:"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_548d98d1","line":21,"in_reply_to":"7a016987_0e7f0fe1","updated":"2015-05-29 14:30:55.000000000","message":"It means that the process group of a process doesn\u0027t match the process group of the controlling terminal, like it\u0027s defined in service.py code [1].\n\nFor devstack it simply means starting neutron server like this:\n\n    python /usr/local/bin/neutron-server --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/ml2_conf.ini \u0026 \n\n[1] https://github.com/openstack/oslo-incubator/blob/master/openstack/common/service.py#L47","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"853ce2c6fc6377efda535e63ca1b63124feb506d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"rpc_workers \u003e 0) and metadata agent don\u0027t die on receiving SIGHUP and support"},{"line_number":19,"context_line":"reloading policy_path and logging options in config."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Note that reset is called only in case a service is running in daemon mode."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Other changes made in the scope of this patch that need to be mentioned:"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_64db3f7d","line":21,"in_reply_to":"7a016987_548d98d1","updated":"2015-06-01 12:28:57.000000000","message":"Interesting. Do you know why the special handling? Is it because we assume that only Unix *daemons* should reload configuration on SIGHUP?","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"364db786aca013e893655cda36338790a7099ec3","unresolved":false,"context_lines":[{"line_number":18,"context_line":"rpc_workers \u003e 0) and metadata agent don\u0027t die on receiving SIGHUP and support"},{"line_number":19,"context_line":"reloading policy_path and logging options in config."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Note that reset is called only in case a service is running in daemon mode."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Other changes made in the scope of this patch that need to be mentioned:"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_16c25385","line":21,"in_reply_to":"7a016987_64db3f7d","updated":"2015-06-01 12:42:32.000000000","message":"It seems that there was some screen-related problem in devstack:\n\nhttps://bugs.launchpad.net/oslo-incubator/+bug/1263122","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"}],"neutron/common/config.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7f8f465d4e04a3741c7df870728aa8cd685fcd6a","unresolved":false,"context_lines":[{"line_number":216,"context_line":"    # Note that this is called only in case a service is running in"},{"line_number":217,"context_line":"    # daemon mode."},{"line_number":218,"context_line":"    setup_logging()"},{"line_number":219,"context_line":"    policy.refresh()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"def load_paste_app(app_name):"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_129b0274","line":219,"updated":"2015-06-09 03:28:49.000000000","message":"I wonder why policy.refresh() is related to \"config\".\nneutron.service.Service.reset() method does the same\nand \"policy\" is not related to \"config\", so I think we don\u0027t need a new function which contains only two lines.\n\nOn the other hand, we cannot define a similar method at now, because this method is called in wsgi.py and moving it to service.py would lead to circular imports between wsgi and service.","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c1befaaaa1616bfcb3fc515b88629d8ca3a5d162","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    # Reset worker in case SIGHUP is called."},{"line_number":216,"context_line":"    # Note that this is called only in case a service is running in"},{"line_number":217,"context_line":"    # daemon mode."},{"line_number":218,"context_line":"    setup_logging()"},{"line_number":219,"context_line":"    policy.refresh()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3afb71cf_9b97c347","line":218,"updated":"2015-06-09 18:03:23.000000000","message":"One question: it resets logging using logging.setup(), but it still uses global CONF object without reparsing configuration files. So what would it mean to reload logging with the same configuration then? Isn\u0027t it NOOP?","commit_id":"6d0d72973152bb45587437c80d4ffe0fe7bba761"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"322f8fd1aed57ff144c2556dd85b98f86a84d2cb","unresolved":false,"context_lines":[{"line_number":215,"context_line":"    # Reset worker in case SIGHUP is called."},{"line_number":216,"context_line":"    # Note that this is called only in case a service is running in"},{"line_number":217,"context_line":"    # daemon mode."},{"line_number":218,"context_line":"    setup_logging()"},{"line_number":219,"context_line":"    policy.refresh()"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3afb71cf_ec55dfc1","line":218,"in_reply_to":"3afb71cf_9b97c347","updated":"2015-06-09 19:08:53.000000000","message":"Well, the configuration file is re-read in common/service on service restart, for example https://github.com/openstack/oslo-incubator/blob/master/openstack/common/service.py#L134","commit_id":"6d0d72973152bb45587437c80d4ffe0fe7bba761"}],"neutron/service.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a71d9cd3acf5a39e0e6fa3321ceecd2d820cfbac","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                server.stop()"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    @staticmethod"},{"line_number":137,"context_line":"    def reset():"},{"line_number":138,"context_line":"        # Reset worker in case SIGHUP is called."},{"line_number":139,"context_line":"        # Note that this is called only in case a service is running in"},{"line_number":140,"context_line":"        # daemon mode."}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_8e009f35","line":137,"updated":"2015-05-29 13:22:14.000000000","message":"reset() methods are identical. Can we avoid duplication? The methods are static anyway, so they could be just a utility function somewhere in neutron.common (?)","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"853ce2c6fc6377efda535e63ca1b63124feb506d","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                server.stop()"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    @staticmethod"},{"line_number":137,"context_line":"    def reset():"},{"line_number":138,"context_line":"        # Reset worker in case SIGHUP is called."},{"line_number":139,"context_line":"        # Note that this is called only in case a service is running in"},{"line_number":140,"context_line":"        # daemon mode."}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_a442570d","line":137,"in_reply_to":"7a016987_14504031","updated":"2015-06-01 12:28:57.000000000","message":"It could still be a common interface with a base extendable implementation.","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"44f65d365b942770ac5203f02731d1f62e27ee8b","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                server.stop()"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    @staticmethod"},{"line_number":137,"context_line":"    def reset():"},{"line_number":138,"context_line":"        # Reset worker in case SIGHUP is called."},{"line_number":139,"context_line":"        # Note that this is called only in case a service is running in"},{"line_number":140,"context_line":"        # daemon mode."}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_14504031","line":137,"in_reply_to":"7a016987_8e009f35","updated":"2015-05-29 14:30:55.000000000","message":"These methods may become different in the future if we add support for reloading any options other than logging (like rpc-specific options for RpcWorker and so on).","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"364db786aca013e893655cda36338790a7099ec3","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                server.stop()"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    @staticmethod"},{"line_number":137,"context_line":"    def reset():"},{"line_number":138,"context_line":"        # Reset worker in case SIGHUP is called."},{"line_number":139,"context_line":"        # Note that this is called only in case a service is running in"},{"line_number":140,"context_line":"        # daemon mode."}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_76b2a707","line":137,"in_reply_to":"7a016987_a442570d","updated":"2015-06-01 12:42:32.000000000","message":"Okay, you convinced me :)","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"7f8f465d4e04a3741c7df870728aa8cd685fcd6a","unresolved":false,"context_lines":[{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    def reset(self):"},{"line_number":294,"context_line":"        config.setup_logging()"},{"line_number":295,"context_line":"        policy.refresh()"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def periodic_tasks(self, raise_on_error\u003dFalse):"},{"line_number":298,"context_line":"        \"\"\"Tasks to be run at a periodic interval.\"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_6d693bbe","line":295,"updated":"2015-06-09 03:28:49.000000000","message":"If we keep reset_service() in neutron.common.config, this also needs to use config.reset_service().","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"0a9ed49e741dce7260f4358bfc21c7cef5c11a15","unresolved":false,"context_lines":[{"line_number":292,"context_line":""},{"line_number":293,"context_line":"    def reset(self):"},{"line_number":294,"context_line":"        config.setup_logging()"},{"line_number":295,"context_line":"        policy.refresh()"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def periodic_tasks(self, raise_on_error\u003dFalse):"},{"line_number":298,"context_line":"        \"\"\"Tasks to be run at a periodic interval.\"\"\""}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_0a3a74cc","line":295,"in_reply_to":"3afb71cf_6d693bbe","updated":"2015-06-09 12:10:05.000000000","message":"Done","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"}],"neutron/tests/functional/test_server.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"bcbdb412063d9215f9870ba129d17c6d41e008ad","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"},{"line_number":102,"context_line":"                     for l in f.readlines()]"},{"line_number":103,"context_line":"        if self.workers \u003e 0:"},{"line_number":104,"context_line":"            return [p for p, pp in processes if pp \u003d\u003d self.pid]"},{"line_number":105,"context_line":"        else:"},{"line_number":106,"context_line":"            return [p for p, pp in processes if p \u003d\u003d self.pid]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7aaa499b_2ecf772a","line":104,"updated":"2015-04-15 12:37:26.000000000","message":"nit: better not to shorten, so pid, ppid","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"8a399fe4e85612e26de3e1b29970eb3955e9a114","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"},{"line_number":102,"context_line":"                     for l in f.readlines()]"},{"line_number":103,"context_line":"        if self.workers \u003e 0:"},{"line_number":104,"context_line":"            return [p for p, pp in processes if pp \u003d\u003d self.pid]"},{"line_number":105,"context_line":"        else:"},{"line_number":106,"context_line":"            return [p for p, pp in processes if p \u003d\u003d self.pid]"},{"line_number":107,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_68eef88c","line":104,"in_reply_to":"7aaa499b_2ecf772a","updated":"2015-04-16 12:38:29.000000000","message":"Ok, I\u0027ll fix this. Thanks","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"bcbdb412063d9215f9870ba129d17c6d41e008ad","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            # Wait at most 5 seconds to restart workers"},{"line_number":123,"context_line":"            condition \u003d lambda: start_workers \u003d\u003d self._get_workers()"},{"line_number":124,"context_line":"            timeout \u003d 5"},{"line_number":125,"context_line":"            self._conditional_wait(condition, timeout)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Make sure worker pids match"},{"line_number":128,"context_line":"        end_workers \u003d self._get_workers()"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aaa499b_eeb2af93","line":125,"updated":"2015-04-15 12:37:26.000000000","message":"this does not look as \u0027waiting for restart\u0027.\n\nHow does the list of worker pids tell you if they were restarted if you expect them to not change?","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"28a2dc5475929a00170e77bd9bd77ce20e128f69","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            # Wait at most 5 seconds to restart workers"},{"line_number":123,"context_line":"            condition \u003d lambda: start_workers \u003d\u003d self._get_workers()"},{"line_number":124,"context_line":"            timeout \u003d 5"},{"line_number":125,"context_line":"            self._conditional_wait(condition, timeout)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Make sure worker pids match"},{"line_number":128,"context_line":"        end_workers \u003d self._get_workers()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_833127ef","line":125,"in_reply_to":"5aa745d1_e83fa823","updated":"2015-04-16 12:44:56.000000000","message":"My suggestion would be to await some output from child process, for example, some files could be written by child processes upon SIGHUP handling which you could then check for existence.","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"8a399fe4e85612e26de3e1b29970eb3955e9a114","unresolved":false,"context_lines":[{"line_number":122,"context_line":"            # Wait at most 5 seconds to restart workers"},{"line_number":123,"context_line":"            condition \u003d lambda: start_workers \u003d\u003d self._get_workers()"},{"line_number":124,"context_line":"            timeout \u003d 5"},{"line_number":125,"context_line":"            self._conditional_wait(condition, timeout)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Make sure worker pids match"},{"line_number":128,"context_line":"        end_workers \u003d self._get_workers()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_e83fa823","line":125,"in_reply_to":"7aaa499b_eeb2af93","updated":"2015-04-16 12:38:29.000000000","message":"In this test I take it for granted that if SIGHUP is sent to a process it is not ignored (as it is not ignored in common.service code).\n\nSo, if SIGHUP is not ignored, then all workers will either exit and we\u0027ll get an empty list or they will be restarted and their pids will not change.\n\nAs I have already said, the test assumes that processes don\u0027t ignore SIGHUP and that makes the test not really blackbox. I\u0027d welcome any suggestions on how to improve the situation.","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"bcbdb412063d9215f9870ba129d17c6d41e008ad","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Make sure worker pids match"},{"line_number":128,"context_line":"        end_workers \u003d self._get_workers()"},{"line_number":129,"context_line":"        self.assertEqual(start_workers, end_workers)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"class TestWsgiServer(TestNeutronServer):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aaa499b_8e7c4bb6","line":129,"updated":"2015-04-15 12:37:26.000000000","message":"so per previous comment this may be always True when children were never sighuped too.\n\nalso, what give a guarantee that these lists are sorted?","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"8a399fe4e85612e26de3e1b29970eb3955e9a114","unresolved":false,"context_lines":[{"line_number":126,"context_line":""},{"line_number":127,"context_line":"        # Make sure worker pids match"},{"line_number":128,"context_line":"        end_workers \u003d self._get_workers()"},{"line_number":129,"context_line":"        self.assertEqual(start_workers, end_workers)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"class TestWsgiServer(TestNeutronServer):"}],"source_content_type":"text/x-python","patch_set":15,"id":"5aa745d1_e8db28ec","line":129,"in_reply_to":"7aaa499b_8e7c4bb6","updated":"2015-04-16 12:38:29.000000000","message":"See reply above.\n\nIt is guaranteed by ps, because it by default sorts the output in increasing numerical or lexicographic order.","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"61f3ce377d01cd60067795cf3784f0337d7515b9","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_3ab46bcf","line":79,"updated":"2015-04-22 09:20:05.000000000","message":"I think you can use  \"neutron.common.utils.wait_until_true(lambda: self.workers \u003d\u003d len(self._get_workers()), timeout\u003d10)\"","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"3c85c086cbd48faf4393a0aaa67d684a9b8c5ab6","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_89a6e5bf","line":79,"in_reply_to":"bab921fb_13d8db52","updated":"2015-04-23 14:01:05.000000000","message":"Done: reused utils.wait_until_true","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"2e3672b62c2d1d21fbc0da7fb6ab1711e5412b16","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_38905eec","line":79,"in_reply_to":"bab921fb_18959ae3","updated":"2015-04-22 22:08:49.000000000","message":"*into helper functions","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"1e99f65dc56b56d415c90a1df2545a6584e9dc6b","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_13d8db52","line":79,"in_reply_to":"bab921fb_335e3fea","updated":"2015-04-22 22:28:43.000000000","message":"Well, here I also pass lambdas as parameters to self._conditional_wait :)","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"184d61feda9a243d8eb4b1df02ce0ecf254c8e67","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_335e3fea","line":79,"in_reply_to":"bab921fb_38905eec","updated":"2015-04-22 22:25:38.000000000","message":"No, I just find passing bare lambdas like suggested above more confusing when reading code, therefore i think current way is more clear.","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"288d143ca98beeec1d53921f67a055d704ec0489","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_18959ae3","line":79,"in_reply_to":"bab921fb_9ff8747e","updated":"2015-04-22 22:08:06.000000000","message":"So, am I right to assume that your advice is to turn lambdas into a helper functions?","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"f097aaf42355c6b5ab7c282eb6d447460a2f0498","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_e04c0272","line":79,"in_reply_to":"dac4157f_3ab46bcf","updated":"2015-04-22 09:42:36.000000000","message":"I guess you meant neutron.agent.linux.utils.wait_until_true. Thank you for advice, I\u0027ll try to use it.","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"2ee894dd579155c2c07a61854d08a94266ff24cb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            # Wait at most 10 seconds to spawn workers"},{"line_number":77,"context_line":"            condition \u003d lambda: self.workers \u003d\u003d len(self._get_workers())"},{"line_number":78,"context_line":"            try:"},{"line_number":79,"context_line":"                self._conditional_wait(condition, 10)"},{"line_number":80,"context_line":"            except Exception:"},{"line_number":81,"context_line":"                self.fail(\"Timed out waiting for %d workers to start\""},{"line_number":82,"context_line":"                          % self.workers)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_9ff8747e","line":79,"in_reply_to":"dac4157f_e04c0272","updated":"2015-04-22 20:45:45.000000000","message":"I would not recommend wrapping lambdas in parameters.\nIt may look kinda cool, but really confusing when reading.","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"61f3ce377d01cd60067795cf3784f0337d7515b9","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        condition \u003d lambda: (os.path.isfile(self.temp_file)"},{"line_number":137,"context_line":"                             and os.stat(self.temp_file).st_size \u003e 0)"},{"line_number":138,"context_line":"        try:"},{"line_number":139,"context_line":"            self._conditional_wait(condition, 5)"},{"line_number":140,"context_line":"        except Exception:"},{"line_number":141,"context_line":"            self.fail(\"Timed out waiting for file %s to be created and \""},{"line_number":142,"context_line":"                      \"become non-empty.\" % self.temp_file)"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_a5005c02","line":139,"updated":"2015-04-22 09:20:05.000000000","message":"Same here - utils.wait_until_true can be used.","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"3c85c086cbd48faf4393a0aaa67d684a9b8c5ab6","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        condition \u003d lambda: (os.path.isfile(self.temp_file)"},{"line_number":137,"context_line":"                             and os.stat(self.temp_file).st_size \u003e 0)"},{"line_number":138,"context_line":"        try:"},{"line_number":139,"context_line":"            self._conditional_wait(condition, 5)"},{"line_number":140,"context_line":"        except Exception:"},{"line_number":141,"context_line":"            self.fail(\"Timed out waiting for file %s to be created and \""},{"line_number":142,"context_line":"                      \"become non-empty.\" % self.temp_file)"}],"source_content_type":"text/x-python","patch_set":18,"id":"bab921fb_69ab918a","line":139,"in_reply_to":"dac4157f_a5005c02","updated":"2015-04-23 14:01:05.000000000","message":"Done","commit_id":"bd9e321e29cf0ba0b1ff5d0556f0d24bff74d08c"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"88da9901c6cf5153bf9ec9f9c22eedd588a40d94","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                launcher.wait()"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                rpc.start()"},{"line_number":212,"context_line":"                rpc.wait()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def test_restart_rpc_on_sighup_multiple_workers(self):"},{"line_number":215,"context_line":"        self._test_restart_service_on_sighup(service\u003dself._serve_rpc,"}],"source_content_type":"text/x-python","patch_set":22,"id":"7a016987_1cd24235","line":212,"updated":"2015-05-26 06:11:05.000000000","message":"can L210-212 be used?","commit_id":"e9828569538d9d57b4b8a4d938ed433857fb1f09"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"9a303621237137a1892b9997d9cb26112ded0775","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                launcher.wait()"},{"line_number":210,"context_line":"            else:"},{"line_number":211,"context_line":"                rpc.start()"},{"line_number":212,"context_line":"                rpc.wait()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def test_restart_rpc_on_sighup_multiple_workers(self):"},{"line_number":215,"context_line":"        self._test_restart_service_on_sighup(service\u003dself._serve_rpc,"}],"source_content_type":"text/x-python","patch_set":22,"id":"7a016987_6a35d3f5","line":212,"in_reply_to":"7a016987_1cd24235","updated":"2015-05-26 15:38:56.000000000","message":"I\u0027m not quite sure I understand your question, but RPC workers are started differently in case of workers\u003d0 and workers\u003e0 because of https://github.com/openstack/neutron/blob/master/neutron/service.py#L156-L162","commit_id":"e9828569538d9d57b4b8a4d938ed433857fb1f09"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d3125bbfb8be95b90cb3a121075806b40cba8846","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        self.health_checker \u003d None"},{"line_number":45,"context_line":"        self.pipein, self.pipeout \u003d os.pipe()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def tearDown(self):"},{"line_number":48,"context_line":"        if self.service_pid:"},{"line_number":49,"context_line":"            # Make sure all processes are stopped"},{"line_number":50,"context_line":"            os.kill(self.service_pid, signal.SIGKILL)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_cc818391","line":47,"updated":"2015-06-02 10:49:04.000000000","message":"tearDown is only executed if test succeed, is it intended?\n\nself.addCleanup(f) ensures f will be called even if setUp/test fails.","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"65644fba916c58f645f936108638c9517f25e70a","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        self.health_checker \u003d None"},{"line_number":45,"context_line":"        self.pipein, self.pipeout \u003d os.pipe()"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def tearDown(self):"},{"line_number":48,"context_line":"        if self.service_pid:"},{"line_number":49,"context_line":"            # Make sure all processes are stopped"},{"line_number":50,"context_line":"            os.kill(self.service_pid, signal.SIGKILL)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_c3f94e4f","line":47,"in_reply_to":"5afe65bd_cc818391","updated":"2015-06-02 13:19:04.000000000","message":"Thank you, done","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d3125bbfb8be95b90cb3a121075806b40cba8846","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def _get_workers(self):"},{"line_number":101,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        f \u003d os.popen(\u0027ps ax -o pid,ppid,command\u0027)"},{"line_number":104,"context_line":"        # Skip ps header"},{"line_number":105,"context_line":"        f.readline()"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_0c78bbc3","line":103,"updated":"2015-06-02 10:49:04.000000000","message":"os.popen is deprecated[1] since py26 we should subprocess[2] and in our case (OpenStack) execute\n\n\n[1] https://docs.python.org/2/library/os.html#os.popen\n[2] https://docs.python.org/2/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"65644fba916c58f645f936108638c9517f25e70a","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def _get_workers(self):"},{"line_number":101,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        f \u003d os.popen(\u0027ps ax -o pid,ppid,command\u0027)"},{"line_number":104,"context_line":"        # Skip ps header"},{"line_number":105,"context_line":"        f.readline()"},{"line_number":106,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_839a3613","line":103,"in_reply_to":"5afe65bd_0c78bbc3","updated":"2015-06-02 13:19:04.000000000","message":"Done","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d3125bbfb8be95b90cb3a121075806b40cba8846","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        with open(self.temp_file, \u0027r\u0027) as f:"},{"line_number":150,"context_line":"            try:"},{"line_number":151,"context_line":"                res \u003d f.readline()"},{"line_number":152,"context_line":"                self.assertEqual(FAKE_RESET_MSG * (workers + 1), res)"},{"line_number":153,"context_line":"            except BaseException:"},{"line_number":154,"context_line":"                self.fail(traceback.print_exc())"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_0f3b0dd3","line":152,"updated":"2015-06-02 10:49:04.000000000","message":"iiuc, self.assertEqual should outside of the try/except.\n\nWhy doing:\n\n  try:\n    res \u003d f.readline()\n  except BaseException:\n    self.fail(traceback.print_exc())\n\nwhen you can do:\n\n  res \u003d f.readline()","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"65644fba916c58f645f936108638c9517f25e70a","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        with open(self.temp_file, \u0027r\u0027) as f:"},{"line_number":150,"context_line":"            try:"},{"line_number":151,"context_line":"                res \u003d f.readline()"},{"line_number":152,"context_line":"                self.assertEqual(FAKE_RESET_MSG * (workers + 1), res)"},{"line_number":153,"context_line":"            except BaseException:"},{"line_number":154,"context_line":"                self.fail(traceback.print_exc())"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5afe65bd_238a62c0","line":152,"in_reply_to":"5afe65bd_0f3b0dd3","updated":"2015-06-02 13:19:04.000000000","message":"Done","commit_id":"810fe111d1ef1d7c11e3db5ac8153047c866ba12"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"9545802304551c7c07de5cf3b44f8c687890ace2","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        expected_size \u003d len(FAKE_RESET_MSG) * (workers + 1)"},{"line_number":137,"context_line":"        condition \u003d lambda: (os.path.isfile(self.temp_file)"},{"line_number":138,"context_line":"                             and os.stat(self.temp_file).st_size \u003d\u003d"},{"line_number":139,"context_line":"                             expected_size)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"        utils.wait_until_true("},{"line_number":142,"context_line":"            condition, timeout\u003d5, sleep\u003d0.1,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5afe65bd_7eeeb123","line":139,"updated":"2015-06-02 13:19:10.000000000","message":"This additional size check should make tests more stable and prevent the following situation:\n\nwe have, say, 2 workers;\nwe have waited for the file to become nonempty and read the first line from it;\nbut there is only one \"reset\" there because children processes haven\u0027t written anything there yet.","commit_id":"e16870a517b6b2106b4d731b097ccb03b7577498"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f1ec862e2a6130b0c4ac896c38ee03ae467c9d30","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \"\"\"Run a given service."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        :param callback: callback that will start the required service"},{"line_number":57,"context_line":"        :param workers: number of service workers"},{"line_number":58,"context_line":"        \"\"\""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        self.workers \u003d workers"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_d8bfcb5e","line":57,"updated":"2015-06-08 12:02:14.000000000","message":"You may want to add info on what is returned since it seems to be a public side of it.","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"0a9ed49e741dce7260f4358bfc21c7cef5c11a15","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \"\"\"Run a given service."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        :param callback: callback that will start the required service"},{"line_number":57,"context_line":"        :param workers: number of service workers"},{"line_number":58,"context_line":"        \"\"\""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        self.workers \u003d workers"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_ca2fbc86","line":57,"in_reply_to":"3afb71cf_d8bfcb5e","updated":"2015-06-09 12:10:05.000000000","message":"Done","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f1ec862e2a6130b0c4ac896c38ee03ae467c9d30","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # Get list of all processes with skipped ps header."},{"line_number":103,"context_line":"        f \u003d utils.execute("},{"line_number":104,"context_line":"            [\"ps\", \"ax\", \"-o\", \"pid,ppid,command\"]).rstrip().split(\"\\n\")[1:]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_18f8c348","line":103,"updated":"2015-06-08 12:02:14.000000000","message":"It should be achievable using https://pythonhosted.org/psutil/#processes","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"560225ea54859d1bad4056381b1033d3c30b96c7","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # Get list of all processes with skipped ps header."},{"line_number":103,"context_line":"        f \u003d utils.execute("},{"line_number":104,"context_line":"            [\"ps\", \"ax\", \"-o\", \"pid,ppid,command\"]).rstrip().split(\"\\n\")[1:]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_2fe0a228","line":103,"in_reply_to":"3afb71cf_18f8c348","updated":"2015-06-09 11:54:22.000000000","message":"wish I knew about it earlier :)","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"be9dbb9057716bdcc614b24e6a1d8975f922541e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # Get list of all processes with skipped ps header."},{"line_number":103,"context_line":"        f \u003d utils.execute("},{"line_number":104,"context_line":"            [\"ps\", \"ax\", \"-o\", \"pid,ppid,command\"]).rstrip().split(\"\\n\")[1:]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_8ff55658","line":103,"in_reply_to":"3afb71cf_2fe0a228","updated":"2015-06-09 12:02:50.000000000","message":"Sometimes I feel guilty for leaving test reviews to afterwards. I should start from them instead.","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"0a9ed49e741dce7260f4358bfc21c7cef5c11a15","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        \"\"\"Get the list of processes in which WSGI server is running.\"\"\""},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        # Get list of all processes with skipped ps header."},{"line_number":103,"context_line":"        f \u003d utils.execute("},{"line_number":104,"context_line":"            [\"ps\", \"ax\", \"-o\", \"pid,ppid,command\"]).rstrip().split(\"\\n\")[1:]"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        processes \u003d [tuple(int(p) for p in l.strip().split()[:2])"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_0a415438","line":103,"in_reply_to":"3afb71cf_8ff55658","updated":"2015-06-09 12:10:05.000000000","message":"Done","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f1ec862e2a6130b0c4ac896c38ee03ae467c9d30","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        with mock.patch(\"neutron.service.RpcWorker.reset\") as reset_method:"},{"line_number":235,"context_line":"            reset_method.side_effect \u003d self._fake_reset"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"            rpc \u003d service.RpcWorker(self.plugin)"},{"line_number":238,"context_line":"            if workers \u003e 0:"},{"line_number":239,"context_line":"                launcher \u003d common_service.ProcessLauncher(wait_interval\u003d1.0)"},{"line_number":240,"context_line":"                launcher.launch_service(rpc, workers\u003dworkers)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_b3f3100a","line":237,"updated":"2015-06-08 12:02:14.000000000","message":"Seems like that code duplicates what is in neutron/service.py. I\u0027m not very happy about it, since it will be harder to maintain the test in the future. Can we avoid duplication here?","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"0a9ed49e741dce7260f4358bfc21c7cef5c11a15","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        with mock.patch(\"neutron.service.RpcWorker.reset\") as reset_method:"},{"line_number":235,"context_line":"            reset_method.side_effect \u003d self._fake_reset"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"            rpc \u003d service.RpcWorker(self.plugin)"},{"line_number":238,"context_line":"            if workers \u003e 0:"},{"line_number":239,"context_line":"                launcher \u003d common_service.ProcessLauncher(wait_interval\u003d1.0)"},{"line_number":240,"context_line":"                launcher.launch_service(rpc, workers\u003dworkers)"}],"source_content_type":"text/x-python","patch_set":28,"id":"3afb71cf_aa76c0a1","line":237,"in_reply_to":"3afb71cf_b3f3100a","updated":"2015-06-09 12:10:05.000000000","message":"Good point here, thanks. Done","commit_id":"7849e10b850cc764828de89b3908efb42bb5d286"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eea45b3b777346581cd8cd5417519065f56c95ed","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import httplib2"},{"line_number":17,"context_line":"import mock"},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"import psutil"},{"line_number":20,"context_line":"import signal"},{"line_number":21,"context_line":"import socket"},{"line_number":22,"context_line":"import time"}],"source_content_type":"text/x-python","patch_set":29,"id":"3afb71cf_656b59f2","line":19,"updated":"2015-06-09 12:45:57.000000000","message":"it\u0027s not a stdlib library, so should have gone to the next section, with oslo_config.","commit_id":"112ce25ad068242805c61ba3568e2b246c825c64"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"c66eb7d8e216596df924b34bd66fae6360fe7529","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import httplib2"},{"line_number":17,"context_line":"import mock"},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"import psutil"},{"line_number":20,"context_line":"import signal"},{"line_number":21,"context_line":"import socket"},{"line_number":22,"context_line":"import time"}],"source_content_type":"text/x-python","patch_set":29,"id":"3afb71cf_2913fb5c","line":19,"in_reply_to":"3afb71cf_656b59f2","updated":"2015-06-09 13:10:17.000000000","message":"oops, sorry, I put it here automatically","commit_id":"112ce25ad068242805c61ba3568e2b246c825c64"}],"neutron/tests/unit/test_service.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a71d9cd3acf5a39e0e6fa3321ceecd2d820cfbac","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class TestRpcWorker(base.BaseTestCase):"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    @mock.patch(\"neutron.policy.refresh\")"},{"line_number":25,"context_line":"    @mock.patch(\"neutron.common.config.setup_logging\")"},{"line_number":26,"context_line":"    def test_reset(self, setup_logging_mock, refresh_mock):"},{"line_number":27,"context_line":"        _plugin \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_6952fddf","line":24,"updated":"2015-05-29 13:22:14.000000000","message":"The fact that we duplicate tests suggests that there is a place for a common interface/parent class to inherit from for all service workers.","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"44f65d365b942770ac5203f02731d1f62e27ee8b","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class TestRpcWorker(base.BaseTestCase):"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    @mock.patch(\"neutron.policy.refresh\")"},{"line_number":25,"context_line":"    @mock.patch(\"neutron.common.config.setup_logging\")"},{"line_number":26,"context_line":"    def test_reset(self, setup_logging_mock, refresh_mock):"},{"line_number":27,"context_line":"        _plugin \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_6f31ddec","line":24,"in_reply_to":"7a016987_6952fddf","updated":"2015-05-29 14:30:55.000000000","message":"Yes, it sounds like a good idea. I have only one concern: wouldn\u0027t a refactoring make this patch over-complicated? Maybe it\u0027s better to be done as separate patches?","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"853ce2c6fc6377efda535e63ca1b63124feb506d","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class TestRpcWorker(base.BaseTestCase):"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    @mock.patch(\"neutron.policy.refresh\")"},{"line_number":25,"context_line":"    @mock.patch(\"neutron.common.config.setup_logging\")"},{"line_number":26,"context_line":"    def test_reset(self, setup_logging_mock, refresh_mock):"},{"line_number":27,"context_line":"        _plugin \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_e463afaf","line":24,"in_reply_to":"7a016987_6f31ddec","updated":"2015-06-01 12:28:57.000000000","message":"Sure, I\u0027m fine with having a separate patch for that. I\u0027m all for multiple patches.","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"}],"neutron/wsgi.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"bcbdb412063d9215f9870ba129d17c6d41e008ad","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        # are discovered to be broken."},{"line_number":106,"context_line":"        if CONF.database.connection:"},{"line_number":107,"context_line":"            api.get_engine().pool.dispose()"},{"line_number":108,"context_line":"        dup_sock \u003d self._service._socket.dup()"},{"line_number":109,"context_line":"        if CONF.use_ssl:"},{"line_number":110,"context_line":"            dup_sock \u003d self._service._wrap_ssl(dup_sock)"},{"line_number":111,"context_line":"        self._server \u003d self._service.pool.spawn(self._service._run,"}],"source_content_type":"text/x-python","patch_set":15,"id":"7aaa499b_4e32c360","line":108,"updated":"2015-04-15 12:37:26.000000000","message":"please add a comment about the problem you\u0027re working around here.","commit_id":"52fa44d09fe41336cef3f049e2d1efb61b98ef95"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a71d9cd3acf5a39e0e6fa3321ceecd2d820cfbac","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        # object becomes not usable which leads to \"Bad file descriptor\""},{"line_number":110,"context_line":"        # errors on service restart."},{"line_number":111,"context_line":"        # Duplicate a socket object to keep a file descriptor usable."},{"line_number":112,"context_line":"        dup_sock \u003d self._service._socket.dup()"},{"line_number":113,"context_line":"        if CONF.use_ssl:"},{"line_number":114,"context_line":"            dup_sock \u003d self._service.wrap_ssl(dup_sock)"},{"line_number":115,"context_line":"        self._server \u003d self._service.pool.spawn(self._service._run,"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_89dbf97b","line":112,"updated":"2015-05-29 13:22:14.000000000","message":"I am pretty sure I miss some essential detail, but could you please clarify why we don\u0027t create a new socket as part of self._server.start() or ._run() that, I assume, are called during reload?\n\nI would expect that _get_socket() would return us a new one each time we respawn.\n\nAlso, can you clarify what\u0027s going on with old sockets? Are they freed to the system?","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"},{"author":{"_account_id":7293,"name":"Elena Ezhova","email":"ezhovaelena19@gmail.com","username":"elena"},"change_message_id":"44f65d365b942770ac5203f02731d1f62e27ee8b","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        # object becomes not usable which leads to \"Bad file descriptor\""},{"line_number":110,"context_line":"        # errors on service restart."},{"line_number":111,"context_line":"        # Duplicate a socket object to keep a file descriptor usable."},{"line_number":112,"context_line":"        dup_sock \u003d self._service._socket.dup()"},{"line_number":113,"context_line":"        if CONF.use_ssl:"},{"line_number":114,"context_line":"            dup_sock \u003d self._service.wrap_ssl(dup_sock)"},{"line_number":115,"context_line":"        self._server \u003d self._service.pool.spawn(self._service._run,"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_6411ff51","line":112,"in_reply_to":"7a016987_89dbf97b","updated":"2015-05-29 14:30:55.000000000","message":"Hmmmm, in fact, _get_socket is never called during reset of a service.\n\nThe current reset process is the following:\n    1. reload config files\n    2. service.stop()\n    3. service.wait()\n    4. service.reset()\n    5. service.start()\nwhere service is a ServiceWorker instance in our case.\nSo, as you can see, we never call Server.start() here, which is the only place _get_socket() is called. And self._service._run() just passes given parameters to an eventlet server.\n\nAs for your last question I can\u0027t yet say for sure what happens with old sockets (though I guess that fd is closed only after all duplicate sockets ate closed). I relied on a working solution from nova, introduced in https://review.openstack.org/#/c/104887/1 . But I\u0027ll investigate it further.","commit_id":"55cd36d3657828ec54b0e457f14d5ca13b415d07"}]}