)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add service restart function in oslo-incubator"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add signal hanlder of SIGHUP and restart function for service to restart."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"It depends on blueprint cfg-reload-config-files of oslo.config."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"AAAAPn%2F%2Fj1A%3D","line":9,"updated":"2013-06-27 20:06:47.000000000","message":"*handler","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"}],"openstack/common/service.py":[{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"d577928e24c99333b7ec22c3b59a6c1fac79d657","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        if signo \u003d\u003d signal.SIGUSR1:"},{"line_number":120,"context_line":"            raise SignalExit(signo, 2)"},{"line_number":121,"context_line":"        else:"},{"line_number":122,"context_line":"            raise SignalExit(signo, 1)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def wait(self):"},{"line_number":125,"context_line":"        #Set a loop here, exit when SIGTERM OR SIGINT signal. Stay in loop when"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPn%2F%2F7dM%3D","line":122,"updated":"2013-06-18 01:32:20.000000000","message":"someone is suggesting using other signal for restart, please see the blueprint.","commit_id":"5e19efcb9a457ab07d637bbb105cef21c117accf"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"d577928e24c99333b7ec22c3b59a6c1fac79d657","unresolved":false,"context_lines":[{"line_number":150,"context_line":"            #will run Wait again if exec code equals 2"},{"line_number":151,"context_line":"            #which means SIGUSR1 signal"},{"line_number":152,"context_line":"            if(exc.code \u003d\u003d 2):"},{"line_number":153,"context_line":"                super(ServiceLauncher, self).restart()"},{"line_number":154,"context_line":"            else:"},{"line_number":155,"context_line":"                return status"},{"line_number":156,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAPn%2F%2F7dQ%3D","line":153,"updated":"2013-06-18 01:32:20.000000000","message":"why call super?","commit_id":"5e19efcb9a457ab07d637bbb105cef21c117accf"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"26bc18799540a165c29735e00d4472fcf41233ce","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                self.stop()"},{"line_number":150,"context_line":"            #will run Wait again if exec code equals 2"},{"line_number":151,"context_line":"            #which means SIGUSR1 signal"},{"line_number":152,"context_line":"            if(exc.code \u003d\u003d 2):"},{"line_number":153,"context_line":"                self.restart()"},{"line_number":154,"context_line":"            else:"},{"line_number":155,"context_line":"                return status"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAPn%2F%2Fyk0%3D","line":152,"updated":"2013-06-21 07:31:47.000000000","message":"we can directly check exc.signo \u003d\u003d signal.SIGHUP here","commit_id":"458082656af2e5a0680a4b04805276479232370e"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        \"\"\""},{"line_number":75,"context_line":"        self._saved_service \u003d service"},{"line_number":76,"context_line":"        #Save the service for restart"},{"line_number":77,"context_line":"        service.backdoor_port \u003d self.backdoor_port"},{"line_number":78,"context_line":"        self._services.add_thread(self.run_service, service)"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fj0o%3D","line":76,"updated":"2013-06-27 20:06:47.000000000","message":"This comment, if it\u0027s necessary at all, should be above the line it refers to.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        raise SignalExit(signo)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def wait(self):"},{"line_number":122,"context_line":"        #Set a loop here, exit when receieve SIGTERM OR SIGINT signal."},{"line_number":123,"context_line":"        #Stay in loop when receieve SIGHUP signal"},{"line_number":124,"context_line":"        while True:"},{"line_number":125,"context_line":"            signal.signal(signal.SIGTERM, self._handle_signal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjzk%3D","line":122,"updated":"2013-06-27 20:06:47.000000000","message":"*receive","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def wait(self):"},{"line_number":122,"context_line":"        #Set a loop here, exit when receieve SIGTERM OR SIGINT signal."},{"line_number":123,"context_line":"        #Stay in loop when receieve SIGHUP signal"},{"line_number":124,"context_line":"        while True:"},{"line_number":125,"context_line":"            signal.signal(signal.SIGTERM, self._handle_signal)"},{"line_number":126,"context_line":"            signal.signal(signal.SIGINT, self._handle_signal)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjzg%3D","line":123,"updated":"2013-06-27 20:06:47.000000000","message":"*receive","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                if rpc:"},{"line_number":147,"context_line":"                    rpc.cleanup()"},{"line_number":148,"context_line":"                self.stop()"},{"line_number":149,"context_line":"            #will run Wait again if receieve SIGHUP signal"},{"line_number":150,"context_line":"            if(signo \u003d\u003d signal.SIGHUP):"},{"line_number":151,"context_line":"                self.restart()"},{"line_number":152,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fj0A%3D","line":149,"updated":"2013-06-27 20:06:47.000000000","message":"This comment is not strictly accurate - it\u0027s not running wait() again, it\u0027s just not breaking the loop.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    rpc.cleanup()"},{"line_number":148,"context_line":"                self.stop()"},{"line_number":149,"context_line":"            #will run Wait again if receieve SIGHUP signal"},{"line_number":150,"context_line":"            if(signo \u003d\u003d signal.SIGHUP):"},{"line_number":151,"context_line":"                self.restart()"},{"line_number":152,"context_line":"            else:"},{"line_number":153,"context_line":"                return status"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjy4%3D","line":150,"updated":"2013-06-27 20:06:47.000000000","message":"The parentheses here are not necessary.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"e6ba4ff88a64317e272de93a9f2994f8dcfb500c","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        cfg.CONF.reload_config_files()"},{"line_number":92,"context_line":"        self._saved_service.create_event()"},{"line_number":93,"context_line":"        self.services.create_event()"},{"line_number":94,"context_line":"        self.launch_service(self._saved_service)"},{"line_number":95,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F%2BCM%3D","line":92,"updated":"2013-07-18 09:39:53.000000000","message":"`_saved_service` will hold the instance for the last service that was launched. \n\nShould this operate over the `self.services.services` list? \n\nWhat about implementing the restart in the Service class as well? Some of these calls could be moved there.","commit_id":"76f6e968cff351d2c95d3efc49c20f9c883fe998"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"e6ba4ff88a64317e272de93a9f2994f8dcfb500c","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        cfg.CONF.reload_config_files()"},{"line_number":92,"context_line":"        self._saved_service.create_event()"},{"line_number":93,"context_line":"        self.services.create_event()"},{"line_number":94,"context_line":"        self.launch_service(self._saved_service)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"class SignalExit(SystemExit):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F%2BBQ%3D","line":94,"updated":"2013-07-18 09:39:53.000000000","message":"The stop call in line 137 does not remove services from the Services instance. Calling launch_service here will re-add self._saved_service the the that instance.","commit_id":"76f6e968cff351d2c95d3efc49c20f9c883fe998"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"e6ba4ff88a64317e272de93a9f2994f8dcfb500c","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        raise SignalExit(signo)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def wait(self):"},{"line_number":113,"context_line":"        #Set a loop here, exit when receive SIGTERM OR SIGINT signal."},{"line_number":114,"context_line":"        #Stay in loop when receive SIGHUP signal"},{"line_number":115,"context_line":"        while True:"},{"line_number":116,"context_line":"            signal.signal(signal.SIGTERM, self._handle_signal)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F%2BEk%3D","line":113,"updated":"2013-07-18 09:39:53.000000000","message":"pls, add NOTE(....)","commit_id":"76f6e968cff351d2c95d3efc49c20f9c883fe998"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def wait(self):"},{"line_number":110,"context_line":"        #NOTE(Fengqian)Set a loop here, exit when receive"},{"line_number":111,"context_line":"        #SIGTERM OR SIGINT signal. Stay in loop when receive SIGHUP signal"},{"line_number":112,"context_line":"        while True:"},{"line_number":113,"context_line":"            signal.signal(signal.SIGTERM, self._handle_signal)"},{"line_number":114,"context_line":"            signal.signal(signal.SIGINT, self._handle_signal)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3q0%3D","line":111,"updated":"2013-07-20 13:00:11.000000000","message":"I don\u0027t think the comment is warranted - it\u0027s pretty obvious what the code does","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":143,"context_line":"                    self.restart()"},{"line_number":144,"context_line":"                else:"},{"line_number":145,"context_line":"                    return status"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"class ServiceWrapper(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3q4%3D","line":145,"updated":"2013-07-20 13:00:11.000000000","message":"Having this in the finally clause doesn\u0027t make much sense to me ... what would happen if some other exception was raised?","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"25c1fe8f9a707b9d42dc050f9e4dba62d022fe11","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":143,"context_line":"                    self.restart()"},{"line_number":144,"context_line":"                else:"},{"line_number":145,"context_line":"                    return status"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"class ServiceWrapper(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F0KI%3D","line":145,"in_reply_to":"AAAAQn%2F%2F3GY%3D","updated":"2013-07-22 20:21:55.000000000","message":"The \"return status\" in the finally clause is what I\u0027m asking about\n\nAFAICT, that will cause exceptions to be swallowed silently","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"ffe1f798233c17b6d9f8c36167f1281b87f53cfe","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":143,"context_line":"                    self.restart()"},{"line_number":144,"context_line":"                else:"},{"line_number":145,"context_line":"                    return status"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"class ServiceWrapper(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3GY%3D","line":145,"in_reply_to":"AAAAQn%2F%2F3q4%3D","updated":"2013-07-21 13:19:25.000000000","message":"i am not quit sure about your question. If some other exception was raised, it will act like before. We only  handle SIGHUP signal here.","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.forktimes \u003d []"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"class ProcessLauncher(object):"},{"line_number":157,"context_line":"    def __init__(self):"},{"line_number":158,"context_line":"        self.children \u003d {}"},{"line_number":159,"context_line":"        self.sigcaught \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3rA%3D","line":156,"updated":"2013-07-20 13:00:11.000000000","message":"No SIGHUP handling for ProcessLauncher ?","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"4fd8bb9cbd39b05b7a5dff694da60b340e26e95a","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.forktimes \u003d []"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"class ProcessLauncher(object):"},{"line_number":157,"context_line":"    def __init__(self):"},{"line_number":158,"context_line":"        self.children \u003d {}"},{"line_number":159,"context_line":"        self.sigcaught \u003d None"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3FE%3D","line":156,"in_reply_to":"AAAAQn%2F%2F3rA%3D","updated":"2013-07-21 13:42:01.000000000","message":"For now, i only add the service restart function. May add the certain part for ProcessLauncher. If so, i think it would be better to submit in another  patch.","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        self.create_event()"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"    def create_event(self):"},{"line_number":336,"context_line":"        self._done \u003d event.Event()"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def start(self):"},{"line_number":339,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3rE%3D","line":336,"updated":"2013-07-20 13:00:11.000000000","message":"I\u0027d add a reset() method that would call self._done.reset()","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"25c1fe8f9a707b9d42dc050f9e4dba62d022fe11","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        self.create_event()"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"    def create_event(self):"},{"line_number":336,"context_line":"        self._done \u003d event.Event()"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def start(self):"},{"line_number":339,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F0I8%3D","line":336,"in_reply_to":"AAAAQn%2F%2F3OQ%3D","updated":"2013-07-22 20:21:55.000000000","message":"Ok, I\u0027d just rename the method i.e.\n\n  def reset(self):\n      # NOTE(Fengqian.gao): docs for Event.reset() recommend against using it\n      self._done \u003d event.Event()","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"ffe1f798233c17b6d9f8c36167f1281b87f53cfe","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        self.create_event()"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"    def create_event(self):"},{"line_number":336,"context_line":"        self._done \u003d event.Event()"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def start(self):"},{"line_number":339,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3OQ%3D","line":336,"in_reply_to":"AAAAQn%2F%2F3rE%3D","updated":"2013-07-21 13:19:25.000000000","message":"At first, i was thinking using reset function. But i changed my idea after saw the comments in event.py about reset function:\n\n \"# this is kind of a misfeature and doesn\u0027t work perfectly well,\n # it\u0027s better to create a new event rather than reset an old one\"\n\nNow, i will use reset function then.","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        self.tg.wait()"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    def restart(self):"},{"line_number":384,"context_line":"        self.create_event()"},{"line_number":385,"context_line":"        restart_service \u003d self.services[-1]"},{"line_number":386,"context_line":"        if restart_service:"},{"line_number":387,"context_line":"            restart_service.create_event()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3rM%3D","line":384,"updated":"2013-07-20 13:00:11.000000000","message":"Why not just:\n\n  self.done.reset()\n\n?","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"ffe1f798233c17b6d9f8c36167f1281b87f53cfe","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        self.tg.wait()"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    def restart(self):"},{"line_number":384,"context_line":"        self.create_event()"},{"line_number":385,"context_line":"        restart_service \u003d self.services[-1]"},{"line_number":386,"context_line":"        if restart_service:"},{"line_number":387,"context_line":"            restart_service.create_event()"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3GA%3D","line":384,"in_reply_to":"AAAAQn%2F%2F3rM%3D","updated":"2013-07-21 13:19:25.000000000","message":"The same reason","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"14a7ec56b310538bd5d9ed84415d8d20e37584a0","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        restart_service \u003d self.services[-1]"},{"line_number":386,"context_line":"        if restart_service:"},{"line_number":387,"context_line":"            restart_service.create_event()"},{"line_number":388,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @staticmethod"},{"line_number":391,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3rI%3D","line":388,"updated":"2013-07-20 13:00:11.000000000","message":"Why only do this for one service?\n\nWhat about the existing greenthread for this service?","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"25c1fe8f9a707b9d42dc050f9e4dba62d022fe11","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        restart_service \u003d self.services[-1]"},{"line_number":386,"context_line":"        if restart_service:"},{"line_number":387,"context_line":"            restart_service.create_event()"},{"line_number":388,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @staticmethod"},{"line_number":391,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F0IM%3D","line":388,"in_reply_to":"AAAAQn%2F%2F3FQ%3D","updated":"2013-07-22 20:21:55.000000000","message":"\"if all objects in services set were valid\" ... why would they not be valid?\n\n\"stopped when receive SIGHUP signal\" ... that assumes that restart() is only caused as a result of SIGHUP. The method should work as expected (and not leak) threads no matter why it was called.","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"4fd8bb9cbd39b05b7a5dff694da60b340e26e95a","unresolved":false,"context_lines":[{"line_number":385,"context_line":"        restart_service \u003d self.services[-1]"},{"line_number":386,"context_line":"        if restart_service:"},{"line_number":387,"context_line":"            restart_service.create_event()"},{"line_number":388,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    @staticmethod"},{"line_number":391,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAQn%2F%2F3FQ%3D","line":388,"in_reply_to":"AAAAQn%2F%2F3rI%3D","updated":"2013-07-21 13:42:01.000000000","message":"i only restart the last service in services set. Because i was wondering if all the objects in services set were valid or not. \n\nI think the existing greenthread for services are stopped when receive SIGHUP signal.  In servicelauncher.wait function, it will run self.stop first then restart the service.","commit_id":"d919487abe558698c3c52cc6e7f22dfc8d14e32d"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                    except Exception:"},{"line_number":137,"context_line":"                    # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":138,"context_line":"                        LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":139,"context_line":"                        break"},{"line_number":140,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":141,"context_line":"                    self.restart()"},{"line_number":142,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyYY%3D","line":139,"updated":"2013-07-23 08:35:34.000000000","message":"Why avoid the restart here?","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"                        LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":139,"context_line":"                        break"},{"line_number":140,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":141,"context_line":"                    self.restart()"},{"line_number":142,"context_line":"                else:"},{"line_number":143,"context_line":"                    break"},{"line_number":144,"context_line":"        return status"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyYQ%3D","line":141,"updated":"2013-07-23 08:35:34.000000000","message":"If an exception other than SignalExit/SystemExit occurs, you don\u0027t want to restart\n\ni.e. the finally block is the wrong place to do this","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                if signo \u003d\u003d signal.SIGHUP:"},{"line_number":141,"context_line":"                    self.restart()"},{"line_number":142,"context_line":"                else:"},{"line_number":143,"context_line":"                    break"},{"line_number":144,"context_line":"        return status"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyYk%3D","line":143,"updated":"2013-07-23 08:35:34.000000000","message":"The loop control is too far away from the loop for my liking\n\nI read this and I think \"what loop are we breaking out of?\"\n\nI can\u0027t help think this should be something like:\n\n  while True:\n      status, signo \u003d self._wait_for_exit_or_signal()\n      if signo !\u003d signal.SIGHUP:\n          return status\n\n      self.restart()","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":338,"context_line":"                    self.running \u003d True"},{"line_number":339,"context_line":"                    self.sigcaught \u003d None"},{"line_number":340,"context_line":"                    continue"},{"line_number":341,"context_line":"            break"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"        for pid in self.children:"},{"line_number":344,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyXU%3D","line":341,"updated":"2013-07-23 08:35:34.000000000","message":"Again, I really don\u0027t like the loop control here? Which loop are we controlling ... the while True or while self.running loops? Hard to tell at a glance\n\nI feel like this could be re-factored into something similar:\n\n  while True:\n      signo \u003d ...\n      if signo !\u003d signal.SIGHUP:\n          break\n\n      # restart children","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":364,"context_line":"        self._done \u003d event.Event()"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"    def reset(self):"},{"line_number":367,"context_line":"        #NOTE(Fengqian) docs for Event.reset() recommend against using it"},{"line_number":368,"context_line":"        self._done \u003d event.Event()"},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"    def start(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyXI%3D","line":367,"updated":"2013-07-23 08:35:34.000000000","message":"Missing a space and colon:\n\n  # NOTE(markmc): this is the format","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"c6279ba1137b75601e008c8df1230d241efedde9","unresolved":false,"context_lines":[{"line_number":414,"context_line":"        self.done \u003d event.Event()"},{"line_number":415,"context_line":"        for restart_service in self.services:"},{"line_number":416,"context_line":"            restart_service.reset()"},{"line_number":417,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    @staticmethod"},{"line_number":420,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FyXE%3D","line":417,"updated":"2013-07-23 08:35:34.000000000","message":"We\u0027re still leaking the old threads here AFAICT","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6ab97624af8b8fb23f62ca0ef5ea455dc8151d01","unresolved":false,"context_lines":[{"line_number":414,"context_line":"        self.done \u003d event.Event()"},{"line_number":415,"context_line":"        for restart_service in self.services:"},{"line_number":416,"context_line":"            restart_service.reset()"},{"line_number":417,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    @staticmethod"},{"line_number":420,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FsdE%3D","line":417,"in_reply_to":"AAAAQn%2F%2FvO4%3D","updated":"2013-07-24 21:52:07.000000000","message":"Ok, fair enough","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"01291aec2533af0f0b174680f43fdfcd95fe3dfb","unresolved":false,"context_lines":[{"line_number":414,"context_line":"        self.done \u003d event.Event()"},{"line_number":415,"context_line":"        for restart_service in self.services:"},{"line_number":416,"context_line":"            restart_service.reset()"},{"line_number":417,"context_line":"            self.tg.add_thread(self.run_service, restart_service, self.done)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    @staticmethod"},{"line_number":420,"context_line":"    def run_service(service, done):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2FvO4%3D","line":417,"in_reply_to":"AAAAQn%2F%2FyXE%3D","updated":"2013-07-24 06:08:33.000000000","message":"Hi, mark,\nI couldn\u0027t get it here:(. Are there any old threads  still exist after self.stop() called?\n\nThanks for your patience.\nFengqian","commit_id":"fac01fd2cb8a8e5d06127a75eb9b10e8067d8ac4"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        CONF.log_opt_values(LOG, std_logging.DEBUG)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            super(ServiceLauncher, self).wait()"},{"line_number":123,"context_line":"        except SignalExit as exc:"},{"line_number":124,"context_line":"            signame \u003d {signal.SIGTERM: \u0027SIGTERM\u0027,"},{"line_number":125,"context_line":"                       signal.SIGINT: \u0027SIGINT\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrsE%3D","line":122,"updated":"2013-07-25 05:21:46.000000000","message":"Do:\n\n  return None, 0","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"86b769b9f5e4e965b508f4aa960618d8b279044d","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        CONF.log_opt_values(LOG, std_logging.DEBUG)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            super(ServiceLauncher, self).wait()"},{"line_number":123,"context_line":"        except SignalExit as exc:"},{"line_number":124,"context_line":"            signame \u003d {signal.SIGTERM: \u0027SIGTERM\u0027,"},{"line_number":125,"context_line":"                       signal.SIGINT: \u0027SIGINT\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frp0%3D","line":122,"in_reply_to":"AAAAQn%2F%2FrsE%3D","updated":"2013-07-25 05:40:23.000000000","message":"Why do i need to return here? It will return at finally, right?","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                       signal.SIGHUP: \u0027SIGHUP\u0027}[exc.signo]"},{"line_number":127,"context_line":"            LOG.info(_(\u0027Caught %s, exiting\u0027), signame)"},{"line_number":128,"context_line":"            signo \u003d exc.signo"},{"line_number":129,"context_line":"            status \u003d exc.code"},{"line_number":130,"context_line":"        except SystemExit as exc:"},{"line_number":131,"context_line":"            status \u003d exc.code"},{"line_number":132,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrsI%3D","line":129,"updated":"2013-07-25 05:21:46.000000000","message":"Do:\n\n  return exc.code, exc.signo","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"86b769b9f5e4e965b508f4aa960618d8b279044d","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                       signal.SIGHUP: \u0027SIGHUP\u0027}[exc.signo]"},{"line_number":127,"context_line":"            LOG.info(_(\u0027Caught %s, exiting\u0027), signame)"},{"line_number":128,"context_line":"            signo \u003d exc.signo"},{"line_number":129,"context_line":"            status \u003d exc.code"},{"line_number":130,"context_line":"        except SystemExit as exc:"},{"line_number":131,"context_line":"            status \u003d exc.code"},{"line_number":132,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frpw%3D","line":129,"in_reply_to":"AAAAQn%2F%2FrsI%3D","updated":"2013-07-25 05:40:23.000000000","message":"same as above","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"            signo \u003d exc.signo"},{"line_number":129,"context_line":"            status \u003d exc.code"},{"line_number":130,"context_line":"        except SystemExit as exc:"},{"line_number":131,"context_line":"            status \u003d exc.code"},{"line_number":132,"context_line":"        finally:"},{"line_number":133,"context_line":"            self.stop()"},{"line_number":134,"context_line":"            if rpc:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrsM%3D","line":131,"updated":"2013-07-25 05:21:46.000000000","message":"Do:\n\n  return exc.code, 0","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"86b769b9f5e4e965b508f4aa960618d8b279044d","unresolved":false,"context_lines":[{"line_number":128,"context_line":"            signo \u003d exc.signo"},{"line_number":129,"context_line":"            status \u003d exc.code"},{"line_number":130,"context_line":"        except SystemExit as exc:"},{"line_number":131,"context_line":"            status \u003d exc.code"},{"line_number":132,"context_line":"        finally:"},{"line_number":133,"context_line":"            self.stop()"},{"line_number":134,"context_line":"            if rpc:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frps%3D","line":131,"in_reply_to":"AAAAQn%2F%2FrsM%3D","updated":"2013-07-25 05:40:23.000000000","message":"same as above","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                except Exception:"},{"line_number":138,"context_line":"                # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":139,"context_line":"                    LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":140,"context_line":"            return (status, signo)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def wait(self):"},{"line_number":143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrsY%3D","line":140,"updated":"2013-07-25 05:21:46.000000000","message":"Again ... no!\n\nTry this:\n\n def test():\n     try:\n         raise RuntimeError\n     finally:\n         return \"foo\"\n\n print test()","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"337a8c0d8bf3400225a039a32c7a0fd24af022a2","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                except Exception:"},{"line_number":138,"context_line":"                # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":139,"context_line":"                    LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":140,"context_line":"            return (status, signo)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def wait(self):"},{"line_number":143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrW8%3D","line":140,"in_reply_to":"AAAAQn%2F%2FrZE%3D","updated":"2013-07-25 08:49:13.000000000","message":"No, that\u0027s totally broken\n\nWhat happens if e.g. this happens:\n\n  def _wait_for_exit_or_signal(self):\n      (status, signo initlization)\n      try:\n          raise AttributeError\n      except SignalExit:\n          ...\n          (set status and signo)\n      except SystemExit:\n          ...\n          (set status)\n      finally:\n          ...\n          return (status, signo)\n\nThe AttributeError gets lost!\n\nBasically, I don\u0027t think there\u0027s ever a good reason to put a return statement in a finally clause","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"396c78154dba2d7d05f3a4b701d4519f7ea759d4","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                except Exception:"},{"line_number":138,"context_line":"                # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":139,"context_line":"                    LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":140,"context_line":"            return (status, signo)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def wait(self):"},{"line_number":143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrZE%3D","line":140,"in_reply_to":"AAAAQn%2F%2FraI%3D","updated":"2013-07-25 08:36:17.000000000","message":"So, in my previous code, i think it should ok there.\n\n def _wait_for_exit_or_signal(self):\n         (status, signo initlization)\n          try:\n                 .........\n          except SignalExit:\n                  .......\n                  (set status and signo)\n           except SystemExit:\n                    ...........\n                     (set status)\n           finally:\n                    .................\n                    return (status, signo)\n\n\nPlease correct me if i am wrong or misunderstanding your  words.\n\nThanks\nFengqian","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"06c2054bd159ebbcaf0733b0f4657efd9ddc2903","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                except Exception:"},{"line_number":138,"context_line":"                # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":139,"context_line":"                    LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":140,"context_line":"            return (status, signo)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def wait(self):"},{"line_number":143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FraI%3D","line":140,"in_reply_to":"AAAAQn%2F%2Frpo%3D","updated":"2013-07-25 08:25:37.000000000","message":"Exactly!","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"86b769b9f5e4e965b508f4aa960618d8b279044d","unresolved":false,"context_lines":[{"line_number":137,"context_line":"                except Exception:"},{"line_number":138,"context_line":"                # We\u0027re shutting down, so it doesn\u0027t matter at this point."},{"line_number":139,"context_line":"                    LOG.exception(_(\u0027Exception during rpc cleanup.\u0027))"},{"line_number":140,"context_line":"            return (status, signo)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def wait(self):"},{"line_number":143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frpo%3D","line":140,"in_reply_to":"AAAAQn%2F%2FrsY%3D","updated":"2013-07-25 05:40:23.000000000","message":"This will print \u0027foo\u0027, right?","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":222,"context_line":"            status \u003d 2"},{"line_number":223,"context_line":"        finally:"},{"line_number":224,"context_line":"            launcher.stop()"},{"line_number":225,"context_line":"            return(status, signo)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def _child_process(self, service):"},{"line_number":228,"context_line":"        self._child_process_handle_signal()"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frr0%3D","line":225,"updated":"2013-07-25 05:21:46.000000000","message":"Again, this return is broken - follow the same model as above","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        wrap.children.remove(pid)"},{"line_number":315,"context_line":"        return wrap"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def _respawn_child(self):"},{"line_number":318,"context_line":"        while self.running:"},{"line_number":319,"context_line":"            wrap \u003d self._wait_child()"},{"line_number":320,"context_line":"            if not wrap:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2Frrc%3D","line":317,"updated":"2013-07-25 05:21:46.000000000","message":"Call it respan_children() - there are multiple","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"88a136f30f3469f14c3f21f8dd1455909cd1855d","unresolved":false,"context_lines":[{"line_number":372,"context_line":"        self._done \u003d event.Event()"},{"line_number":373,"context_line":""},{"line_number":374,"context_line":"    def reset(self):"},{"line_number":375,"context_line":"        #NOTE(Fengqian): docs for Event.reset() recommend against using it"},{"line_number":376,"context_line":"        self._done \u003d event.Event()"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    def start(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAQn%2F%2FrrU%3D","line":375,"updated":"2013-07-25 05:21:46.000000000","message":"Still missing the space at the start","commit_id":"5bbd11109e4214eac82e25a4220158adfd443c13"}],"tests/unit/test_service.py":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"class ServiceRestartTest(utils.BaseTestCase):"},{"line_number":194,"context_line":"    def _get_pid(self):"},{"line_number":195,"context_line":"        f \u003d os.popen(\u0027ps ax -o pid\u0027)"},{"line_number":196,"context_line":"        f.readline()"},{"line_number":197,"context_line":"        for line in f.readlines():"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FjxE%3D","line":194,"updated":"2013-06-27 20:06:47.000000000","message":"I feel like this function is misnamed.  Isn\u0027t it checking whether self.pid is alive?","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":210,"context_line":"                launcher.wait()"},{"line_number":211,"context_line":"            except SystemExit as exc:"},{"line_number":212,"context_line":"                status \u003d exc.code"},{"line_number":213,"context_line":"            except BaseException:"},{"line_number":214,"context_line":"                try:"},{"line_number":215,"context_line":"                    traceback.print_exc()"},{"line_number":216,"context_line":"                except BaseException:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjuc%3D","line":213,"updated":"2013-06-27 20:06:47.000000000","message":"Is all this really necessary?  If an uncaught exception happens it\u0027s going to stop the process with a non-zero return value anyway.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    def _wait(self, cond, timeout):"},{"line_number":223,"context_line":"        start \u003d time.time()"},{"line_number":224,"context_line":"        while True:"},{"line_number":225,"context_line":"            if cond():"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"            if time.time() - start \u003e timeout:"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FjuM%3D","line":224,"updated":"2013-06-27 20:06:47.000000000","message":"This could be simplified a bit by changing it to \"while not cond():\" and removing the extra if below.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":229,"context_line":"            time.sleep(.1)"},{"line_number":230,"context_line":""},{"line_number":231,"context_line":"    def setUp(self):"},{"line_number":232,"context_line":"        #Same as ServiceLauncherTest.setUp()"},{"line_number":233,"context_line":"        super(ServiceRestartTest, self).setUp()"},{"line_number":234,"context_line":"        CONF.unregister_opts(notifier_api.notifier_opts)"},{"line_number":235,"context_line":"        CONF(args\u003d[], default_config_files\u003d[])"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjt4%3D","line":232,"updated":"2013-06-27 20:06:47.000000000","message":"Based on this comment, should this class inherit from ServiceLauncherTest instead of duplicating all of its code?  Or maybe the common code could be factored out into a base class for both.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"        pid \u003d self._get_pid()"},{"line_number":262,"context_line":"        LOG.info(\u0027pid: %r\u0027 % pid)"},{"line_number":263,"context_line":"        self.assertTrue(pid)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        os.kill(self.pid, signal.SIGHUP)"},{"line_number":266,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2Fjtk%3D","line":263,"updated":"2013-06-27 20:06:47.000000000","message":"This is related to my earlier comment, but I don\u0027t really like the way this reads.  I think _get_pid should be renamed and return True or False.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"9d3cf6bd4984db3259772de2a7d604a1e1260f85","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        pid_restart \u003d self._get_pid()"},{"line_number":270,"context_line":"        LOG.info(\u0027pid_restart: %r\u0027 % pid_restart)"},{"line_number":271,"context_line":"        self.assertTrue(pid_restart)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"class LauncherTest(utils.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FjtA%3D","line":271,"updated":"2013-06-27 20:06:47.000000000","message":"Should check that the new pid doesn\u0027t match the old one.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"99af78501a31479f4f1accf1ca7889be5cf8f1c2","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        pid_restart \u003d self._get_pid()"},{"line_number":270,"context_line":"        LOG.info(\u0027pid_restart: %r\u0027 % pid_restart)"},{"line_number":271,"context_line":"        self.assertTrue(pid_restart)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"class LauncherTest(utils.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FdPg%3D","line":271,"in_reply_to":"AAAAPn%2F%2FiiQ%3D","updated":"2013-07-01 15:32:14.000000000","message":"Whoops, you\u0027re right.  I blame this comment on Friday afternoon. :-)","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":7642,"name":"Fengqian Gao","email":"fengqian.gao@intel.com","username":"Fengqian"},"change_message_id":"c12074108e2e7ddc4422644840b1b3396fa8c126","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        pid_restart \u003d self._get_pid()"},{"line_number":270,"context_line":"        LOG.info(\u0027pid_restart: %r\u0027 % pid_restart)"},{"line_number":271,"context_line":"        self.assertTrue(pid_restart)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"class LauncherTest(utils.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FiiQ%3D","line":271,"in_reply_to":"AAAAPn%2F%2FjtA%3D","updated":"2013-07-01 01:48:27.000000000","message":"In this test case, i think the process will not exit. From the codes, ServiceLauncher.wait() will stop each threads and re-launch service.  Process ID should be the same.","commit_id":"7ddd63aa31caef06c62b769dda690261b63eb611"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"67dff07a01fce3f94defeaf94ccfd0f4bf33f9fd","unresolved":false,"context_lines":[{"line_number":223,"context_line":"        # Make sure worker pids match"},{"line_number":224,"context_line":"        end_workers \u003d self._get_workers()"},{"line_number":225,"context_line":"        LOG.info(\u0027workers: %r\u0027 % end_workers)"},{"line_number":226,"context_line":"        self.assertEqual(start_workers, end_workers)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"class ServiceRestartTest(ServiceTestBase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2Fqto%3D","line":226,"updated":"2013-07-25 13:36:50.000000000","message":"test_child_sighup and test_parent_signal_sighup have common code. Maybe you can refactor this in the next patch","commit_id":"825ace5581fbb416944acae62f51c489ed93b9c9"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"change_message_id":"67dff07a01fce3f94defeaf94ccfd0f4bf33f9fd","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                if stat in [\u0027Z\u0027, \u0027T\u0027, \u0027Z+\u0027]:"},{"line_number":239,"context_line":"                    return False"},{"line_number":240,"context_line":"                else:"},{"line_number":241,"context_line":"                    return True"},{"line_number":242,"context_line":"        return False"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def _spawn_service(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAQn%2F%2FquM%3D","line":241,"updated":"2013-07-25 13:36:50.000000000","message":"line 238~241:\n\n return stat not in [\u0027Z\u0027, \u0027T\u0027, \u0027Z+\u0027]","commit_id":"825ace5581fbb416944acae62f51c489ed93b9c9"}]}