From ee9f84b286668570882e1756f34f7bab3062ca5a Mon Sep 17 00:00:00 2001 From: kyudin Date: Thu, 18 Feb 2016 12:18:14 +0300 Subject: [PATCH 1/6] python3 fixes --- circus/plugins/watchdog.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index e48a07e8e..7be8c6b32 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -136,7 +136,8 @@ def _discover_monitored_pids(self): self.pid_status = dict() all_watchers = self.call("list") for watcher_name in all_watchers['watchers']: - if self._match_watcher_name(watcher_name): + # This can be done via regex, but regex can be reconfigured and watchdog shouldn't kill itself + if self._match_watcher_name(watcher_name) and watcher_name != 'plugin:' + self.name: processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: @@ -182,7 +183,9 @@ def _decode_received_udp_message(self, data): :return: decoded message :rtype: dict or None """ - result = re.match(self.msg_regex, data) + """regex is created with string, but bytes are received from the socket, + that cause exception. Probably this can be done better, that just decode data""" + result = re.match(self.msg_regex, data.decode()) if result is not None: return result.groupdict() @@ -217,6 +220,8 @@ def look_after(self): max_timeout = self.loop_rate * self.max_count too_old_time = time.time() - max_timeout + # Instead of del in loop, that will cause exception in Python3, add to list and del from pid_status after loop + pids_to_del = list() for pid, detail in self.pid_status.items(): if detail['last_activity'] < too_old_time: logger.info("watcher:%s, pid:%s is not responding. Kill it !", @@ -233,4 +238,6 @@ def look_after(self): # Trusting watcher to eventually stop the process after # graceful timeout - del self.pid_status[pid] + pids_to_del.append(pid) + for pid in pids_to_del: + del self.pid_status[pid] From ecbcdd288155f385e67ac82a5b41020e52e69796 Mon Sep 17 00:00:00 2001 From: kyudin Date: Thu, 18 Feb 2016 12:40:25 +0300 Subject: [PATCH 2/6] fix comments and len of string --- circus/plugins/watchdog.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index 7be8c6b32..832bc091f 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -135,9 +135,10 @@ def _discover_monitored_pids(self): """ self.pid_status = dict() all_watchers = self.call("list") + """Probably it's also bad, that watchdog kills itself. + """ for watcher_name in all_watchers['watchers']: - # This can be done via regex, but regex can be reconfigured and watchdog shouldn't kill itself - if self._match_watcher_name(watcher_name) and watcher_name != 'plugin:' + self.name: + if self._match_watcher_name(watcher_name): processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: @@ -183,8 +184,11 @@ def _decode_received_udp_message(self, data): :return: decoded message :rtype: dict or None """ - """regex is created with string, but bytes are received from the socket, - that cause exception. Probably this can be done better, that just decode data""" + """regex is created with string, + but bytes are received from the socket, + that cause exception. + Probably this can be done better, + that just decode data""" result = re.match(self.msg_regex, data.decode()) if result is not None: return result.groupdict() @@ -220,7 +224,9 @@ def look_after(self): max_timeout = self.loop_rate * self.max_count too_old_time = time.time() - max_timeout - # Instead of del in loop, that will cause exception in Python3, add to list and del from pid_status after loop + """Instead of del in loop, that will cause exception in Python3, + add to list and del from pid_status after loop. + """ pids_to_del = list() for pid, detail in self.pid_status.items(): if detail['last_activity'] < too_old_time: From ea38643b7640f6e2d45f3eaedb1201e7d4dfb8a9 Mon Sep 17 00:00:00 2001 From: kyudin Date: Thu, 18 Feb 2016 15:57:25 +0300 Subject: [PATCH 3/6] add tests and return check for watchdog discovering --- circus/plugins/watchdog.py | 6 ++-- circus/tests/test_plugin_watchdog.py | 45 +++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index 832bc091f..f730d7a41 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -135,10 +135,10 @@ def _discover_monitored_pids(self): """ self.pid_status = dict() all_watchers = self.call("list") - """Probably it's also bad, that watchdog kills itself. - """ for watcher_name in all_watchers['watchers']: - if self._match_watcher_name(watcher_name): + # Do not discover watchdog + if self._match_watcher_name(watcher_name) and \ + watcher_name != 'plugin:' + self.name: processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: diff --git a/circus/tests/test_plugin_watchdog.py b/circus/tests/test_plugin_watchdog.py index a96ed6578..a3100bfb5 100644 --- a/circus/tests/test_plugin_watchdog.py +++ b/circus/tests/test_plugin_watchdog.py @@ -31,13 +31,24 @@ def run_dummy_watchdogged(test_file): process.run() return 1 +class WatchDoggedWithoutSend(Process): + def run(self): + self._write('STARTWD') + for _ in range(5): + time.sleep(0.5) + self._write('STOPWD') + +def run_watchdog_without_send(test_file): + process = WatchDoggedWithoutSend(test_file) + process.run() + return 1 def get_pid_status(queue, plugin): queue.put(plugin.pid_status) fqn = 'circus.tests.test_plugin_watchdog.run_dummy_watchdogged' - +dqn = 'circus.tests.test_plugin_watchdog.run_watchdog_without_send' class TestPluginWatchDog(TestCircus): @@ -72,4 +83,36 @@ def test_watchdog_discovery_not_found(self): self.assertEqual(len(pid_status), 0, pid_status) yield self.stop_arbiter() + @gen_test + def test_watchdog_kill_processes(self): + yield self.start_arbiter(dqn) + async_poll_for(self.test_file, 'START') + pubsub = self.arbiter.pubsub_endpoint + + config = {'loop_rate': 0.1} + with warnings.catch_warnings(): + pid_status = yield arp(WatchDog, config, + get_pid_status, + endpoint=self.arbiter.endpoint, + pubsub_endpoint=pubsub, + duration=1000) + self.assertEqual(len(pid_status), 0, pid_status) + yield self.stop_arbiter() + + @gen_test + def test_watchdog_handle_hearthbeat(self): + yield self.start_arbiter(fqn) + async_poll_for(self.test_file, 'START') + pubsub = self.arbiter.pubsub_endpoint + + config = {'loop_rate': 0.1} + with warnings.catch_warnings(): + pid_status = yield arp(WatchDog, config, + get_pid_status, + endpoint=self.arbiter.endpoint, + pubsub_endpoint=pubsub, + duration=600) + self.assertEqual(len(pid_status), 1, pid_status) + yield self.stop_arbiter() + test_suite = EasyTestSuite(__name__) From ab0cce382af07cc1ab9d6ea19c88790bfc95d4ad Mon Sep 17 00:00:00 2001 From: kyudin Date: Thu, 18 Feb 2016 16:23:40 +0300 Subject: [PATCH 4/6] comment test for now and remove watchdog discover check --- circus/plugins/watchdog.py | 5 ++--- circus/tests/test_plugin_watchdog.py | 6 ++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index f730d7a41..3edcc2af0 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -136,9 +136,8 @@ def _discover_monitored_pids(self): self.pid_status = dict() all_watchers = self.call("list") for watcher_name in all_watchers['watchers']: - # Do not discover watchdog - if self._match_watcher_name(watcher_name) and \ - watcher_name != 'plugin:' + self.name: + # TODO: Do not discover watchdog + if self._match_watcher_name(watcher_name): processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: diff --git a/circus/tests/test_plugin_watchdog.py b/circus/tests/test_plugin_watchdog.py index a3100bfb5..206acd4d9 100644 --- a/circus/tests/test_plugin_watchdog.py +++ b/circus/tests/test_plugin_watchdog.py @@ -31,6 +31,7 @@ def run_dummy_watchdogged(test_file): process.run() return 1 + class WatchDoggedWithoutSend(Process): def run(self): self._write('STARTWD') @@ -38,11 +39,13 @@ def run(self): time.sleep(0.5) self._write('STOPWD') + def run_watchdog_without_send(test_file): process = WatchDoggedWithoutSend(test_file) process.run() return 1 + def get_pid_status(queue, plugin): queue.put(plugin.pid_status) @@ -50,6 +53,7 @@ def get_pid_status(queue, plugin): fqn = 'circus.tests.test_plugin_watchdog.run_dummy_watchdogged' dqn = 'circus.tests.test_plugin_watchdog.run_watchdog_without_send' + class TestPluginWatchDog(TestCircus): @gen_test @@ -99,6 +103,7 @@ def test_watchdog_kill_processes(self): self.assertEqual(len(pid_status), 0, pid_status) yield self.stop_arbiter() + """ @gen_test def test_watchdog_handle_hearthbeat(self): yield self.start_arbiter(fqn) @@ -114,5 +119,6 @@ def test_watchdog_handle_hearthbeat(self): duration=600) self.assertEqual(len(pid_status), 1, pid_status) yield self.stop_arbiter() + """ test_suite = EasyTestSuite(__name__) From 556724b5e9470300025a8f1173d36d91aeafe6fc Mon Sep 17 00:00:00 2001 From: kyudin Date: Sat, 20 Feb 2016 10:15:55 +0300 Subject: [PATCH 5/6] do not discover watchdog, remove/fix comments --- circus/plugins/watchdog.py | 16 ++++------ circus/tests/test_plugin_watchdog.py | 48 ---------------------------- 2 files changed, 6 insertions(+), 58 deletions(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index 3edcc2af0..31173d6f4 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -48,6 +48,7 @@ class WatchDog(CircusPlugin): used when killing the processes """ name = 'watchdog' + plugin_name = 'plugin:' + name def __init__(self, endpoint, pubsub_endpoint, check_delay, ssh_server, **config): @@ -136,8 +137,9 @@ def _discover_monitored_pids(self): self.pid_status = dict() all_watchers = self.call("list") for watcher_name in all_watchers['watchers']: - # TODO: Do not discover watchdog - if self._match_watcher_name(watcher_name): + # do not discover watchdog + if self._match_watcher_name(watcher_name) and \ + watcher_name != self.plugin_name: processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: @@ -183,11 +185,6 @@ def _decode_received_udp_message(self, data): :return: decoded message :rtype: dict or None """ - """regex is created with string, - but bytes are received from the socket, - that cause exception. - Probably this can be done better, - that just decode data""" result = re.match(self.msg_regex, data.decode()) if result is not None: return result.groupdict() @@ -223,9 +220,8 @@ def look_after(self): max_timeout = self.loop_rate * self.max_count too_old_time = time.time() - max_timeout - """Instead of del in loop, that will cause exception in Python3, - add to list and del from pid_status after loop. - """ + # Instead of del in loop, that will cause exception in Python3, + # add to list and del from pid_status after loop. pids_to_del = list() for pid, detail in self.pid_status.items(): if detail['last_activity'] < too_old_time: diff --git a/circus/tests/test_plugin_watchdog.py b/circus/tests/test_plugin_watchdog.py index 206acd4d9..428de0eec 100644 --- a/circus/tests/test_plugin_watchdog.py +++ b/circus/tests/test_plugin_watchdog.py @@ -32,26 +32,11 @@ def run_dummy_watchdogged(test_file): return 1 -class WatchDoggedWithoutSend(Process): - def run(self): - self._write('STARTWD') - for _ in range(5): - time.sleep(0.5) - self._write('STOPWD') - - -def run_watchdog_without_send(test_file): - process = WatchDoggedWithoutSend(test_file) - process.run() - return 1 - - def get_pid_status(queue, plugin): queue.put(plugin.pid_status) fqn = 'circus.tests.test_plugin_watchdog.run_dummy_watchdogged' -dqn = 'circus.tests.test_plugin_watchdog.run_watchdog_without_send' class TestPluginWatchDog(TestCircus): @@ -87,38 +72,5 @@ def test_watchdog_discovery_not_found(self): self.assertEqual(len(pid_status), 0, pid_status) yield self.stop_arbiter() - @gen_test - def test_watchdog_kill_processes(self): - yield self.start_arbiter(dqn) - async_poll_for(self.test_file, 'START') - pubsub = self.arbiter.pubsub_endpoint - - config = {'loop_rate': 0.1} - with warnings.catch_warnings(): - pid_status = yield arp(WatchDog, config, - get_pid_status, - endpoint=self.arbiter.endpoint, - pubsub_endpoint=pubsub, - duration=1000) - self.assertEqual(len(pid_status), 0, pid_status) - yield self.stop_arbiter() - - """ - @gen_test - def test_watchdog_handle_hearthbeat(self): - yield self.start_arbiter(fqn) - async_poll_for(self.test_file, 'START') - pubsub = self.arbiter.pubsub_endpoint - - config = {'loop_rate': 0.1} - with warnings.catch_warnings(): - pid_status = yield arp(WatchDog, config, - get_pid_status, - endpoint=self.arbiter.endpoint, - pubsub_endpoint=pubsub, - duration=600) - self.assertEqual(len(pid_status), 1, pid_status) - yield self.stop_arbiter() - """ test_suite = EasyTestSuite(__name__) From 4493a8a63422c7334fb1b520826679abdf65b9c9 Mon Sep 17 00:00:00 2001 From: kyudin Date: Sat, 20 Feb 2016 10:30:08 +0300 Subject: [PATCH 6/6] flake8 fixes --- circus/plugins/watchdog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circus/plugins/watchdog.py b/circus/plugins/watchdog.py index 31173d6f4..0d4aa74c2 100644 --- a/circus/plugins/watchdog.py +++ b/circus/plugins/watchdog.py @@ -139,7 +139,7 @@ def _discover_monitored_pids(self): for watcher_name in all_watchers['watchers']: # do not discover watchdog if self._match_watcher_name(watcher_name) and \ - watcher_name != self.plugin_name: + watcher_name != self.plugin_name: processes = self.call("list", name=watcher_name) if 'pids' in processes: for pid in processes['pids']: