From 7c2b9e01017028915558c27eba4584e8519d433e Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Tue, 9 Jun 2020 17:56:40 -0400 Subject: [PATCH 01/49] Add hook getter and setter to the class --- pyqs/decorator.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pyqs/decorator.py b/pyqs/decorator.py index 5f9e3d6..0450f45 100644 --- a/pyqs/decorator.py +++ b/pyqs/decorator.py @@ -54,12 +54,22 @@ def wrapper(*args, **kwargs): class task(object): + _hooks = {} + def __init__(self, queue=None, delay_seconds=None, - custom_function_path=None): + custom_function_path=None, hooks=None): self.queue_name = queue self.delay_seconds = delay_seconds self.function_path = custom_function_path + @classmethod + def set_hooks(self, hooks): + self._hooks = hooks + + @classmethod + def get_hooks(self): + return self._hooks + def __call__(self, *args, **kwargs): func_to_wrap = args[0] function = func_to_wrap @@ -69,4 +79,6 @@ def __call__(self, *args, **kwargs): function = self.function_path func_to_wrap.delay = task_delayer( function, self.queue_name, self.delay_seconds, override=override) + func_to_wrap.pre_process_hook = self.get_hooks().get("pre_process") + func_to_wrap.post_process_hook = self.get_hooks().get("post_process") return func_to_wrap From 0ce9364c32f4ac254e7ae60f2d01058b630c0af0 Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Tue, 9 Jun 2020 17:58:54 -0400 Subject: [PATCH 02/49] Add hook context and call hooks --- pyqs/worker.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pyqs/worker.py b/pyqs/worker.py index d1ba0b4..c53bd49 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -188,6 +188,16 @@ def process_message(self): task = getattr(task_module, task_name) + context = { + "task_name": task_name, + "args": args, + "kwargs": kwargs, + "full_task_path": full_task_path, + "fetch_time": fetch_time, + "queue_url": queue_url, + "timeout": timeout + } + current_time = time.time() if int(current_time - fetch_time) >= timeout: logger.warning( @@ -201,6 +211,8 @@ def process_message(self): return True try: start_time = time.time() + if task.pre_process_hook: + task.pre_process_hook(context) task(*args, **kwargs) except Exception: end_time = time.time() @@ -214,6 +226,10 @@ def process_message(self): traceback.format_exc(), ) ) + if task.post_process_hook: + context["status"] = "exception" + context["exception"] = traceback.format_exc() + task.post_process_hook(context) return True else: end_time = time.time() @@ -230,6 +246,9 @@ def process_message(self): repr(kwargs), ) ) + if task.post_process_hook: + context["status"] = "success" + task.post_process_hook(context) return True From 750fb45661c018c310487feeaaaedf41e4c8c0c7 Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Mon, 15 Jun 2020 12:10:25 -0400 Subject: [PATCH 03/49] Change API to be a global event registry that can handle multiple hooks per event --- pyqs/decorator.py | 14 +------------- pyqs/events.py | 19 +++++++++++++++++++ pyqs/worker.py | 22 +++++++++++++--------- 3 files changed, 33 insertions(+), 22 deletions(-) create mode 100644 pyqs/events.py diff --git a/pyqs/decorator.py b/pyqs/decorator.py index 0450f45..5f9e3d6 100644 --- a/pyqs/decorator.py +++ b/pyqs/decorator.py @@ -54,22 +54,12 @@ def wrapper(*args, **kwargs): class task(object): - _hooks = {} - def __init__(self, queue=None, delay_seconds=None, - custom_function_path=None, hooks=None): + custom_function_path=None): self.queue_name = queue self.delay_seconds = delay_seconds self.function_path = custom_function_path - @classmethod - def set_hooks(self, hooks): - self._hooks = hooks - - @classmethod - def get_hooks(self): - return self._hooks - def __call__(self, *args, **kwargs): func_to_wrap = args[0] function = func_to_wrap @@ -79,6 +69,4 @@ def __call__(self, *args, **kwargs): function = self.function_path func_to_wrap.delay = task_delayer( function, self.queue_name, self.delay_seconds, override=override) - func_to_wrap.pre_process_hook = self.get_hooks().get("pre_process") - func_to_wrap.post_process_hook = self.get_hooks().get("post_process") return func_to_wrap diff --git a/pyqs/events.py b/pyqs/events.py new file mode 100644 index 0000000..9cd03b4 --- /dev/null +++ b/pyqs/events.py @@ -0,0 +1,19 @@ +class Events: + def __init__(self): + self.pre_process = [] + self.post_process = [] + + +# Singleton +EVENTS = Events() + + +def register(name, callback): + if hasattr(EVENTS, name): + getattr(EVENTS, name).append(callback) + else: + raise Exception(f"{name} is not a valid pyqs event.") + + +def get_events(): + return EVENTS diff --git a/pyqs/worker.py b/pyqs/worker.py index c53bd49..a72ebae 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -19,6 +19,7 @@ import boto3 from pyqs.utils import get_aws_region_name, decode_message +from pyqs.events import get_events MESSAGE_DOWNLOAD_BATCH_SIZE = 10 LONG_POLLING_INTERVAL = 20 @@ -211,8 +212,9 @@ def process_message(self): return True try: start_time = time.time() - if task.pre_process_hook: - task.pre_process_hook(context) + pre_process_hooks = get_events().pre_process + for hook in pre_process_hooks: + hook(context) task(*args, **kwargs) except Exception: end_time = time.time() @@ -226,10 +228,11 @@ def process_message(self): traceback.format_exc(), ) ) - if task.post_process_hook: - context["status"] = "exception" - context["exception"] = traceback.format_exc() - task.post_process_hook(context) + post_process_hooks = get_events().post_process + context["status"] = "exception" + context["exception"] = traceback.format_exc() + for hook in post_process_hooks: + hook(context) return True else: end_time = time.time() @@ -246,9 +249,10 @@ def process_message(self): repr(kwargs), ) ) - if task.post_process_hook: - context["status"] = "success" - task.post_process_hook(context) + post_process_hooks = get_events().post_process + context["status"] = "success" + for hook in post_process_hooks: + hook(context) return True From f557af1c6078cc93dcc05e194017eea86adfc0b9 Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Tue, 16 Jun 2020 12:24:42 -0400 Subject: [PATCH 04/49] Clean up events API & add unit tests --- .gitignore | 1 + pyqs/events.py | 36 ++++++++++++++++----- tests/test_events.py | 74 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 tests/test_events.py diff --git a/.gitignore b/.gitignore index 33d7c93..51be5a5 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ dist/* *.egg-info/* build/* htmlcov/* +.idea/* *~ *# diff --git a/pyqs/events.py b/pyqs/events.py index 9cd03b4..2befea9 100644 --- a/pyqs/events.py +++ b/pyqs/events.py @@ -1,19 +1,41 @@ +""" +pyqs events registry: register callback functions on pyqs events + +Usage: +from pyqs.events import register_event + +register_event("pre_process", lambda context: print(context)) +""" + + class Events: def __init__(self): self.pre_process = [] self.post_process = [] + def clear(self): + self.pre_process = [] + self.post_process = [] -# Singleton -EVENTS = Events() +# Global singleton +_EVENTS = Events() -def register(name, callback): - if hasattr(EVENTS, name): - getattr(EVENTS, name).append(callback) + +class NoEventException(Exception): + pass + + +def register_event(name, callback): + if hasattr(_EVENTS, name): + getattr(_EVENTS, name).append(callback) else: - raise Exception(f"{name} is not a valid pyqs event.") + raise NoEventException(f"{name} is not a valid pyqs event.") def get_events(): - return EVENTS + return _EVENTS + + +def clear_events(): + _EVENTS.clear() diff --git a/tests/test_events.py b/tests/test_events.py new file mode 100644 index 0000000..17d3239 --- /dev/null +++ b/tests/test_events.py @@ -0,0 +1,74 @@ +from functools import wraps +from nose.tools import assert_raises +from pyqs import events + + +def clear_events_registry(fn): + """Clear the global events registry before each test.""" + + @wraps(fn) + def wrapper(*args, **kwargs): + events.clear_events() + return fn(*args, **kwargs) + return wrapper + + +@clear_events_registry +def test_register_event(): + def print_pre_process(context): + print(context) + + events.register_event("pre_process", print_pre_process) + events.get_events().pre_process.should.equal([print_pre_process]) + + +@clear_events_registry +def test_register_multiple_same_events(): + def print_pre_process(context): + print(context) + + def print_numbers(context): + print(1 + 2) + + events.register_event("pre_process", print_pre_process) + events.register_event("pre_process", print_numbers) + events.get_events().pre_process.should.equal([print_pre_process, print_numbers]) + + +@clear_events_registry +def test_register_different_events(): + def print_pre_process(context): + print(context) + + def print_post_process(context): + print(context) + + events.register_event("pre_process", print_pre_process) + events.register_event("post_process", print_post_process) + events.get_events().pre_process.should.equal([print_pre_process]) + events.get_events().post_process.should.equal([print_post_process]) + + +@clear_events_registry +def test_register_multiple_different_events(): + def print_pre_process(context): + print(context) + + def print_post_process(context): + print(context) + + def print_numbers(context): + print(1 + 2) + + events.register_event("pre_process", print_pre_process) + events.register_event("pre_process", print_numbers) + events.register_event("post_process", print_post_process) + events.register_event("post_process", print_numbers) + events.get_events().pre_process.should.equal([print_pre_process, print_numbers]) + events.get_events().post_process.should.equal([print_post_process, print_numbers]) + + +@clear_events_registry +def test_register_non_existent_event(): + non_existent_event = "non_existent_event" + assert_raises(events.NoEventException, events.register_event, non_existent_event, lambda x: x) From d2d19a74738b3d1b473cf9d0a5b6ce1a25b5fe3f Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Tue, 16 Jun 2020 15:48:49 -0400 Subject: [PATCH 05/49] Refactor running hooks & add functional tests --- pyqs/worker.py | 29 +++--- tests/tasks.py | 5 ++ tests/test_events.py | 12 +-- tests/test_worker.py | 208 ++++++++++++++++++++++++++++++++++++++++++- tests/utils.py | 13 +++ 5 files changed, 242 insertions(+), 25 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index a72ebae..2767ce9 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import copy import fnmatch import importlib import logging @@ -189,7 +190,7 @@ def process_message(self): task = getattr(task_module, task_name) - context = { + pre_process_context = { "task_name": task_name, "args": args, "kwargs": kwargs, @@ -199,6 +200,9 @@ def process_message(self): "timeout": timeout } + # Modify the contexts separately so the original context isn't modified by later processing + post_process_context = copy.copy(pre_process_context) + current_time = time.time() if int(current_time - fetch_time) >= timeout: logger.warning( @@ -212,9 +216,7 @@ def process_message(self): return True try: start_time = time.time() - pre_process_hooks = get_events().pre_process - for hook in pre_process_hooks: - hook(context) + self._run_hooks("pre_process", pre_process_context) task(*args, **kwargs) except Exception: end_time = time.time() @@ -228,11 +230,9 @@ def process_message(self): traceback.format_exc(), ) ) - post_process_hooks = get_events().post_process - context["status"] = "exception" - context["exception"] = traceback.format_exc() - for hook in post_process_hooks: - hook(context) + post_process_context["status"] = "exception" + post_process_context["exception"] = traceback.format_exc() + self._run_hooks("post_process", post_process_context) return True else: end_time = time.time() @@ -249,12 +249,15 @@ def process_message(self): repr(kwargs), ) ) - post_process_hooks = get_events().post_process - context["status"] = "success" - for hook in post_process_hooks: - hook(context) + post_process_context["status"] = "success" + self._run_hooks("post_process", post_process_context) return True + def _run_hooks(self, hook_name, context): + hooks = getattr(get_events(), hook_name) + for hook in hooks: + hook(context) + class ManagerWorker(object): diff --git a/tests/tasks.py b/tests/tasks.py index a92ddd7..3ebdd73 100644 --- a/tests/tasks.py +++ b/tests/tasks.py @@ -36,3 +36,8 @@ def delayed_task(): @task(custom_function_path="custom_function.path", queue="foobar") def custom_path_task(): pass + + +@task() +def exception_task(message, extra=None): + raise Exception('this task raises an exception!') diff --git a/tests/test_events.py b/tests/test_events.py index 17d3239..60f84e8 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -1,16 +1,6 @@ -from functools import wraps from nose.tools import assert_raises from pyqs import events - - -def clear_events_registry(fn): - """Clear the global events registry before each test.""" - - @wraps(fn) - def wrapper(*args, **kwargs): - events.clear_events() - return fn(*args, **kwargs) - return wrapper +from tests.utils import clear_events_registry @clear_events_registry diff --git a/tests/test_worker.py b/tests/test_worker.py index 996eb39..bfd51da 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -17,13 +17,52 @@ MESSAGE_DOWNLOAD_BATCH_SIZE, ) from pyqs.utils import decode_message +from pyqs.events import register_event from tests.tasks import task_results -from tests.utils import MockLoggingHandler +from tests.utils import MockLoggingHandler, clear_events_registry BATCHSIZE = 10 INTERVAL = 0.1 +def _add_message_to_internal_queue(task_name): + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Build the SQS message + message = { + 'Body': json.dumps({ + 'task': task_name, + 'args': [], + 'kwargs': { + 'message': 'Test message', + }, + }), + "ReceiptHandle": "receipt-1234", + } + # Add message to queue + internal_queue = Queue() + internal_queue.put( + { + "message": message, + "queue": queue_url, + "start_time": time.time(), + "timeout": 30, + } + ) + return internal_queue + + +def _check_internal_queue_is_empty(internal_queue): + try: + internal_queue.get(timeout=1) + except Empty: + pass + else: + raise AssertionError("The internal queue should be empty") + + @mock_sqs def test_worker_fills_internal_queue(): """ @@ -688,3 +727,170 @@ def test_worker_to_large_batch_size(): worker = ManagerWorker(QUEUE_PREFIX, CONCURRENCY, INTERVAL, BATCHSIZE) worker.batchsize.should.equal(MESSAGE_DOWNLOAD_BATCH_SIZE) + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_pre_process_callback(): + """ + Test worker runs registered callbacks when processing a message + """ + + # Declare this so it can be checked as a side effect to pre_process_with_side_effect + pre_process_context = None + + def pre_process_with_side_effect(context): + nonlocal pre_process_context + pre_process_context = context + + # When we have a registered pre_process callback + register_event("pre_process", pre_process_with_side_effect) + + # And we process a message + internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) + worker.process_message() + + # We should run the callback with the task context + pre_process_context['task_name'].should.equal('index_incrementer') + pre_process_context['args'].should.equal([]) + pre_process_context['kwargs'].should.equal({'message': 'Test message'}) + pre_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') + pre_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + pre_process_context['timeout'].should.equal(30) + + assert 'fetch_time' in pre_process_context + assert 'status' not in pre_process_context + + # And the internal queue should be empty + _check_internal_queue_is_empty(internal_queue) + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_post_process_callback_success(): + """ + Test worker runs registered callbacks when processing a message and it succeeds + """ + + # Declare this so it can be checked as a side effect to post_process_with_side_effect + post_process_context = None + + def post_process_with_side_effect(context): + nonlocal post_process_context + post_process_context = context + + # When we have a registered post_process callback + register_event("post_process", post_process_with_side_effect) + + # And we process a message + internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) + worker.process_message() + + # We should run the callback with the task context + post_process_context['task_name'].should.equal('index_incrementer') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') + post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('success') + + assert 'fetch_time' in post_process_context + assert 'exception' not in post_process_context + + # And the internal queue should be empty + _check_internal_queue_is_empty(internal_queue) + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_post_process_callback_exception(): + """ + Test worker runs registered callbacks when processing a message and it fails + """ + + # Declare this so it can be checked as a side effect to post_process_with_side_effect + post_process_context = None + + def post_process_with_side_effect(context): + nonlocal post_process_context + post_process_context = context + + # When we have a registered post_process callback + register_event("post_process", post_process_with_side_effect) + + # And we process a message + internal_queue = _add_message_to_internal_queue('tests.tasks.exception_task') + worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) + worker.process_message() + + # We should run the callback with the task context + post_process_context['task_name'].should.equal('exception_task') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal('tests.tasks.exception_task') + post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('exception') + + assert 'fetch_time' in post_process_context + assert 'exception' in post_process_context + + # And the internal queue should be empty + _check_internal_queue_is_empty(internal_queue) + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_pre_and_post_process(): + """ + Test worker runs registered callbacks when processing a message + """ + + # Declare these so they can be checked as a side effect to the callbacks + pre_process_context = None + post_process_context = None + + def pre_process_with_side_effect(context): + nonlocal pre_process_context + pre_process_context = context + + def post_process_with_side_effect(context): + nonlocal post_process_context + post_process_context = context + + # When we have a registered pre_process and post_process callback + register_event("pre_process", pre_process_with_side_effect) + register_event("post_process", post_process_with_side_effect) + + # And we process a message + internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) + worker.process_message() + + # We should run the callbacks with the right task contexts + pre_process_context['task_name'].should.equal('index_incrementer') + pre_process_context['args'].should.equal([]) + pre_process_context['kwargs'].should.equal({'message': 'Test message'}) + pre_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') + pre_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + pre_process_context['timeout'].should.equal(30) + + assert 'fetch_time' in pre_process_context + assert 'status' not in pre_process_context + + post_process_context['task_name'].should.equal('index_incrementer') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') + post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('success') + + assert 'fetch_time' in post_process_context + assert 'exception' not in post_process_context + + # And the internal queue should be empty + _check_internal_queue_is_empty(internal_queue) diff --git a/tests/utils.py b/tests/utils.py index 3e9a9b4..0243beb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -2,9 +2,12 @@ from __future__ import unicode_literals import logging +from functools import wraps from threading import Thread +from pyqs import events + class MockLoggingHandler(logging.Handler): """Mock logging handler to check for expected logs.""" @@ -55,3 +58,13 @@ def run(self): def join(self): Thread.join(self) return self._return + + +def clear_events_registry(fn): + """Clear the global events registry before each test.""" + + @wraps(fn) + def wrapper(*args, **kwargs): + events.clear_events() + return fn(*args, **kwargs) + return wrapper From 0d456c0f0128dc50f86af17e820a2ea4227dfb53 Mon Sep 17 00:00:00 2001 From: Jim Shields Date: Thu, 25 Jun 2020 13:22:39 -0400 Subject: [PATCH 06/49] Fix style & Python 2 issues --- pyqs/events.py | 4 +- pyqs/worker.py | 3 +- tests/test_events.py | 19 ++++++-- tests/test_worker.py | 105 +++++++++++++++++++++++++++++-------------- 4 files changed, 91 insertions(+), 40 deletions(-) diff --git a/pyqs/events.py b/pyqs/events.py index 2befea9..e0d988c 100644 --- a/pyqs/events.py +++ b/pyqs/events.py @@ -30,7 +30,9 @@ def register_event(name, callback): if hasattr(_EVENTS, name): getattr(_EVENTS, name).append(callback) else: - raise NoEventException(f"{name} is not a valid pyqs event.") + raise NoEventException( + "{name} is not a valid pyqs event.".format(name=name) + ) def get_events(): diff --git a/pyqs/worker.py b/pyqs/worker.py index 2767ce9..73484e2 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -200,7 +200,8 @@ def process_message(self): "timeout": timeout } - # Modify the contexts separately so the original context isn't modified by later processing + # Modify the contexts separately so the original + # context isn't modified by later processing post_process_context = copy.copy(pre_process_context) current_time = time.time() diff --git a/tests/test_events.py b/tests/test_events.py index 60f84e8..7823b4e 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -22,7 +22,9 @@ def print_numbers(context): events.register_event("pre_process", print_pre_process) events.register_event("pre_process", print_numbers) - events.get_events().pre_process.should.equal([print_pre_process, print_numbers]) + events.get_events().pre_process.should.equal([ + print_pre_process, print_numbers + ]) @clear_events_registry @@ -54,11 +56,20 @@ def print_numbers(context): events.register_event("pre_process", print_numbers) events.register_event("post_process", print_post_process) events.register_event("post_process", print_numbers) - events.get_events().pre_process.should.equal([print_pre_process, print_numbers]) - events.get_events().post_process.should.equal([print_post_process, print_numbers]) + events.get_events().pre_process.should.equal([ + print_pre_process, print_numbers + ]) + events.get_events().post_process.should.equal([ + print_post_process, print_numbers + ]) @clear_events_registry def test_register_non_existent_event(): non_existent_event = "non_existent_event" - assert_raises(events.NoEventException, events.register_event, non_existent_event, lambda x: x) + assert_raises( + events.NoEventException, + events.register_event, + non_existent_event, + lambda x: x + ) diff --git a/tests/test_worker.py b/tests/test_worker.py index bfd51da..ef4bae3 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -736,27 +736,35 @@ def test_worker_processes_tasks_with_pre_process_callback(): Test worker runs registered callbacks when processing a message """ - # Declare this so it can be checked as a side effect to pre_process_with_side_effect - pre_process_context = None + # Declare this so it can be checked as a side effect + # to pre_process_with_side_effect + contexts = [] def pre_process_with_side_effect(context): - nonlocal pre_process_context - pre_process_context = context + contexts.append(context) # When we have a registered pre_process callback register_event("pre_process", pre_process_with_side_effect) # And we process a message - internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + internal_queue = _add_message_to_internal_queue( + 'tests.tasks.index_incrementer' + ) worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) worker.process_message() + pre_process_context = contexts[0] + # We should run the callback with the task context pre_process_context['task_name'].should.equal('index_incrementer') pre_process_context['args'].should.equal([]) pre_process_context['kwargs'].should.equal({'message': 'Test message'}) - pre_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') - pre_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + pre_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + pre_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) pre_process_context['timeout'].should.equal(30) assert 'fetch_time' in pre_process_context @@ -770,30 +778,39 @@ def pre_process_with_side_effect(context): @mock_sqs def test_worker_processes_tasks_with_post_process_callback_success(): """ - Test worker runs registered callbacks when processing a message and it succeeds + Test worker runs registered callbacks when + processing a message and it succeeds """ - # Declare this so it can be checked as a side effect to post_process_with_side_effect - post_process_context = None + # Declare this so it can be checked as a side effect + # to post_process_with_side_effect + contexts = [] def post_process_with_side_effect(context): - nonlocal post_process_context - post_process_context = context + contexts.append(context) # When we have a registered post_process callback register_event("post_process", post_process_with_side_effect) # And we process a message - internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + internal_queue = _add_message_to_internal_queue( + 'tests.tasks.index_incrementer' + ) worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) worker.process_message() + post_process_context = contexts[0] + # We should run the callback with the task context post_process_context['task_name'].should.equal('index_incrementer') post_process_context['args'].should.equal([]) post_process_context['kwargs'].should.equal({'message': 'Test message'}) - post_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') - post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) post_process_context['timeout'].should.equal(30) post_process_context['status'].should.equal('success') @@ -808,30 +825,39 @@ def post_process_with_side_effect(context): @mock_sqs def test_worker_processes_tasks_with_post_process_callback_exception(): """ - Test worker runs registered callbacks when processing a message and it fails + Test worker runs registered callbacks when processing + a message and it fails """ - # Declare this so it can be checked as a side effect to post_process_with_side_effect - post_process_context = None + # Declare this so it can be checked as a side effect + # to post_process_with_side_effect + contexts = [] def post_process_with_side_effect(context): - nonlocal post_process_context - post_process_context = context + contexts.append(context) # When we have a registered post_process callback register_event("post_process", post_process_with_side_effect) # And we process a message - internal_queue = _add_message_to_internal_queue('tests.tasks.exception_task') + internal_queue = _add_message_to_internal_queue( + 'tests.tasks.exception_task' + ) worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) worker.process_message() + post_process_context = contexts[0] + # We should run the callback with the task context post_process_context['task_name'].should.equal('exception_task') post_process_context['args'].should.equal([]) post_process_context['kwargs'].should.equal({'message': 'Test message'}) - post_process_context['full_task_path'].should.equal('tests.tasks.exception_task') - post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['full_task_path'].should.equal( + 'tests.tasks.exception_task' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) post_process_context['timeout'].should.equal(30) post_process_context['status'].should.equal('exception') @@ -850,42 +876,53 @@ def test_worker_processes_tasks_with_pre_and_post_process(): """ # Declare these so they can be checked as a side effect to the callbacks - pre_process_context = None - post_process_context = None + contexts = [] def pre_process_with_side_effect(context): - nonlocal pre_process_context - pre_process_context = context + contexts.append(context) def post_process_with_side_effect(context): - nonlocal post_process_context - post_process_context = context + contexts.append(context) # When we have a registered pre_process and post_process callback register_event("pre_process", pre_process_with_side_effect) register_event("post_process", post_process_with_side_effect) # And we process a message - internal_queue = _add_message_to_internal_queue('tests.tasks.index_incrementer') + internal_queue = _add_message_to_internal_queue( + 'tests.tasks.index_incrementer' + ) worker = ProcessWorker(internal_queue, INTERVAL, parent_id=1) worker.process_message() + pre_process_context = contexts[0] + # We should run the callbacks with the right task contexts pre_process_context['task_name'].should.equal('index_incrementer') pre_process_context['args'].should.equal([]) pre_process_context['kwargs'].should.equal({'message': 'Test message'}) - pre_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') - pre_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + pre_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + pre_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) pre_process_context['timeout'].should.equal(30) assert 'fetch_time' in pre_process_context assert 'status' not in pre_process_context + post_process_context = contexts[1] + post_process_context['task_name'].should.equal('index_incrementer') post_process_context['args'].should.equal([]) post_process_context['kwargs'].should.equal({'message': 'Test message'}) - post_process_context['full_task_path'].should.equal('tests.tasks.index_incrementer') - post_process_context['queue_url'].should.equal('https://queue.amazonaws.com/123456789012/tester') + post_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) post_process_context['timeout'].should.equal(30) post_process_context['status'].should.equal('success') From 5df7bbc6b37d8723955e5297a4354852c3bc88c1 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Thu, 25 Jun 2020 16:13:13 -0500 Subject: [PATCH 07/49] 0.1.5 --- CHANGELOG.rst | 6 ++++++ README.rst | 23 +++++++++++++++++++++++ pyqs/__init__.py | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0400906..cc81ae9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,12 @@ Changelog --------- +0.1.5 +~~~~~ + +- Add events hooks for pre and post processors. + + 0.1.4 ~~~~~ diff --git a/README.rst b/README.rst index f251a8d..8da2b16 100644 --- a/README.rst +++ b/README.rst @@ -100,6 +100,29 @@ messages. $ pyqs send_email --concurrency 10 +Hooks +~~~~~ + +PyQS has an event registry which can be used to run a function before or after every tasks runs. + +.. code:: python + from pyqs import task, events + + def print_pre_process(context): + print({"pre_process": context}) + + def print_post_process(context): + print({"pre_process": context}) + + events.register_event("pre_process", print_pre_process) + events.register_event("post_process", print_post_process) + + @task(queue="my_queue") + def send_email(subject): + pass + + + Operational Notes ~~~~~~~~~~~~~~~~~ diff --git a/pyqs/__init__.py b/pyqs/__init__.py index bd44c8a..557d2b4 100644 --- a/pyqs/__init__.py +++ b/pyqs/__init__.py @@ -1,4 +1,4 @@ from .decorator import task # noqa __title__ = 'pyqs' -__version__ = '0.1.4' +__version__ = '0.1.5' From d6c88f3295c27fc3ae0c529b81c31b29550978d5 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Thu, 25 Jun 2020 16:20:41 -0500 Subject: [PATCH 08/49] Fix rst. --- CHANGELOG.rst | 1 - README.rst | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cc81ae9..a6003ce 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,7 +6,6 @@ Changelog - Add events hooks for pre and post processors. - 0.1.4 ~~~~~ diff --git a/README.rst b/README.rst index 8da2b16..6f176ac 100644 --- a/README.rst +++ b/README.rst @@ -106,6 +106,7 @@ Hooks PyQS has an event registry which can be used to run a function before or after every tasks runs. .. code:: python + from pyqs import task, events def print_pre_process(context): @@ -121,8 +122,6 @@ PyQS has an event registry which can be used to run a function before or after e def send_email(subject): pass - - Operational Notes ~~~~~~~~~~~~~~~~~ From b1eeed9d51ecef4d5afca207a15840d692ddf777 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Mon, 18 Jan 2021 10:01:07 -0600 Subject: [PATCH 09/49] Refactor connection creation. --- pyqs/worker.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 73484e2..517a3c2 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -40,10 +40,21 @@ def get_conn(region=None, access_key_id=None, secret_access_key=None): class BaseWorker(Process): def __init__(self, *args, **kwargs): + self._connection = None self.parent_id = kwargs.pop('parent_id') super(BaseWorker, self).__init__(*args, **kwargs) self.should_exit = Event() + def _get_connection(self): + if self._connection: + return self._connection + + if self.connection_args is None: + self._connection = get_conn() + else: + self._connection = get_conn(**self.connection_args) + return self._connection + def shutdown(self): logger.info( "Received shutdown signal, shutting down PID {}!".format( @@ -67,10 +78,9 @@ def __init__(self, queue_url, internal_queue, batchsize, if connection_args is None: connection_args = {} self.connection_args = connection_args - self.conn = get_conn(**self.connection_args) self.queue_url = queue_url - sqs_queue = self.conn.get_queue_attributes( + sqs_queue = get_conn(**self.connection_args).get_queue_attributes( QueueUrl=queue_url, AttributeNames=['All'])['Attributes'] self.visibility_timeout = int(sqs_queue['VisibilityTimeout']) @@ -90,7 +100,7 @@ def run(self): self.internal_queue.cancel_join_thread() def read_message(self): - messages = self.conn.receive_message( + messages = self._get_connection().receive_message( QueueUrl=self.queue_url, MaxNumberOfMessages=self.batchsize, WaitTimeSeconds=LONG_POLLING_INTERVAL, @@ -143,10 +153,7 @@ class ProcessWorker(BaseWorker): def __init__(self, internal_queue, interval, connection_args=None, *args, **kwargs): super(ProcessWorker, self).__init__(*args, **kwargs) - if connection_args is None: - self.conn = get_conn() - else: - self.conn = get_conn(**connection_args) + self.connection_args = connection_args self.internal_queue = internal_queue self.interval = interval self._messages_to_process_before_shutdown = 100 @@ -237,7 +244,7 @@ def process_message(self): return True else: end_time = time.time() - self.conn.delete_message( + self._get_connection().delete_message( QueueUrl=queue_url, ReceiptHandle=message['ReceiptHandle'] ) From 5be371276abe363817993e506421399b8af69db7 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Mon, 18 Jan 2021 14:29:41 -0600 Subject: [PATCH 10/49] 0.1.6 --- CHANGELOG.rst | 5 +++++ pyqs/__init__.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a6003ce..4523348 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,11 @@ Changelog --------- +0.1.6 +~~~~~ + +- Fix broken pickle of botocore clients. + 0.1.5 ~~~~~ diff --git a/pyqs/__init__.py b/pyqs/__init__.py index 557d2b4..4f424be 100644 --- a/pyqs/__init__.py +++ b/pyqs/__init__.py @@ -1,4 +1,4 @@ from .decorator import task # noqa __title__ = 'pyqs' -__version__ = '0.1.5' +__version__ = '0.1.6' From d0c8e3dee69f358598001795ced00a9a7112a575 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Mon, 18 Jan 2021 14:33:25 -0600 Subject: [PATCH 11/49] Cleanup make publish. --- Makefile | 27 ++++----------------------- development.txt | 2 ++ pyproject.toml | 3 +++ 3 files changed, 9 insertions(+), 23 deletions(-) create mode 100644 pyproject.toml diff --git a/Makefile b/Makefile index 0da598a..68c12d5 100644 --- a/Makefile +++ b/Makefile @@ -41,26 +41,7 @@ clean: @find . -name __pycache__ -delete @rm -rf .coverage *.egg-info *.log build dist MANIFEST yc -publish: clean tag - @if [ -e "$$HOME/.pypirc" ]; then \ - echo "Uploading to '$(CUSTOM_PIP_INDEX)'"; \ - python setup.py register -r "$(CUSTOM_PIP_INDEX)"; \ - python setup.py sdist upload -r "$(CUSTOM_PIP_INDEX)"; \ - else \ - echo "You should create a file called '.pypirc' under your home dir.\n"; \ - echo "That's the right place to configure 'pypi' repos.\n"; \ - exit 1; \ - fi - -tag: - @if [ $$(git rev-list $$(git describe --abbrev=0 --tags)..HEAD --count) -gt 0 ]; then \ - if [ $$(git log -n 1 --oneline $$(git describe --abbrev=0 --tags)..HEAD CHANGELOG.rst | wc -l) -gt 0 ]; then \ - git tag $$(python setup.py --version) && git push --tags || echo 'Version already released, update your version!'; \ - else \ - echo "CHANGELOG not updated since last release!"; \ - exit 1; \ - fi; \ - else \ - echo "No commits since last release!"; \ - exit 1;\ - fi +publish: clean + rm -rf dist + python -m pep517.build --source --binary . + twine upload dist/* diff --git a/development.txt b/development.txt index 71b8fc8..1f38b8b 100644 --- a/development.txt +++ b/development.txt @@ -2,7 +2,9 @@ coverage==4.4.1 mock==1.0.1 moto==1.3.13 nose==1.3.0 +pep517==0.9.1 pre-commit==0.7.6 sure==1.2.2 +twine==3.3.0 functools32;python_version=='2.7' pycodestyle==2.4.0 diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..9787c3b --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,3 @@ +[build-system] +requires = ["setuptools", "wheel"] +build-backend = "setuptools.build_meta" From 4889961e5aae63248a9df655cc319610bca006db Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 7 Apr 2021 16:12:41 -0400 Subject: [PATCH 12/49] Add initial SimpleProcessWorker implementation --- pyqs/worker.py | 158 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/pyqs/worker.py b/pyqs/worker.py index 517a3c2..ae63014 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -267,6 +267,164 @@ def _run_hooks(self, hook_name, context): hook(context) +class SimpleProcessWorker(BaseWorker): + + def __init__(self, queue_url, batchsize, + connection_args=None, *args, **kwargs): + super(SimpleProcessWorker, self).__init__(*args, **kwargs) + if connection_args is None: + connection_args = {} + self.connection_args = connection_args + self.queue_url = queue_url + + sqs_queue = get_conn(**self.connection_args).get_queue_attributes( + QueueUrl=queue_url, AttributeNames=['All'])['Attributes'] + self.visibility_timeout = int(sqs_queue['VisibilityTimeout']) + + self.batchsize = batchsize + self._messages_to_process_before_shutdown = 100 + self.messages_processed = 0 + + def run(self): + # Set the child process to not receive any keyboard interrupts + signal.signal(signal.SIGINT, signal.SIG_IGN) + + logger.info( + "Running SimpleProcessWorker: {}, pid: {}".format( + self.queue_url, os.getpid())) + + while not self.should_exit.is_set() and self.parent_is_alive(): + self.read_message() + + if self.messages_processed \ + >= self._messages_to_process_before_shutdown: + self.shutdown() + + def read_message(self): + messages = self._get_connection().receive_message( + QueueUrl=self.queue_url, + MaxNumberOfMessages=self.batchsize, + WaitTimeSeconds=LONG_POLLING_INTERVAL, + ).get('Messages', []) + + logger.debug( + "Successfully got {} messages from SQS queue {}".format( + len(messages), self.queue_url)) # noqa + + start = time.time() + + for message in messages: + end = time.time() + if int(end - start) >= self.visibility_timeout: + # Don't add any more messages since they have + # re-appeared in the sqs queue Instead just reset and get + # fresh messages from the sqs queue + msg = ( + "Clearing Local messages since we exceeded " + "their visibility_timeout" + ) + logger.warning(msg) + break + + packed_message = { + "queue": self.queue_url, + "message": message, + "start_time": start, + "timeout": self.visibility_timeout, + } + + self.process_message(packed_message) + + def process_message(self, packed_message): + + message = packed_message['message'] + queue_url = packed_message['queue'] + fetch_time = packed_message['start_time'] + timeout = packed_message['timeout'] + message_body = decode_message(message) + full_task_path = message_body['task'] + args = message_body['args'] + kwargs = message_body['kwargs'] + + task_name = full_task_path.split(".")[-1] + task_path = ".".join(full_task_path.split(".")[:-1]) + + task_module = importlib.import_module(task_path) + + task = getattr(task_module, task_name) + + pre_process_context = { + "task_name": task_name, + "args": args, + "kwargs": kwargs, + "full_task_path": full_task_path, + "fetch_time": fetch_time, + "queue_url": queue_url, + "timeout": timeout + } + + # Modify the contexts separately so the original + # context isn't modified by later processing + post_process_context = copy.copy(pre_process_context) + + current_time = time.time() + if int(current_time - fetch_time) >= timeout: + logger.warning( + "Discarding task {} with args: {} and kwargs: {} due to " + "exceeding visibility timeout".format( # noqa + full_task_path, + repr(args), + repr(kwargs), + ) + ) + self.messages_processed += 1 + + start_time = time.time() + + try: + self._run_hooks("pre_process", pre_process_context) + task(*args, **kwargs) + except Exception: + end_time = time.time() + logger.exception( + "Task {} raised error in {:.4f} seconds: with args: {} " + "and kwargs: {}: {}".format( + full_task_path, + end_time - start_time, + args, + kwargs, + traceback.format_exc(), + ) + ) + post_process_context["status"] = "exception" + post_process_context["exception"] = traceback.format_exc() + self._run_hooks("post_process", post_process_context) + self.messages_processed += 1 + else: + end_time = time.time() + self._get_connection().delete_message( + QueueUrl=queue_url, + ReceiptHandle=message['ReceiptHandle'] + ) + logger.info( + "Processed task {} in {:.4f} seconds with args: {} " + "and kwargs: {}".format( + full_task_path, + end_time - start_time, + repr(args), + repr(kwargs), + ) + ) + post_process_context["status"] = "success" + self._run_hooks("post_process", post_process_context) + self.messages_processed += 1 + + def _run_hooks(self, hook_name, context): + hooks = getattr(get_events(), hook_name) + for hook in hooks: + hook(context) + + class ManagerWorker(object): def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, From 9bf631a5a6322cdee70cc37c8457e1b339dfdb70 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 7 Apr 2021 16:14:05 -0400 Subject: [PATCH 13/49] Add SimpleManagerWorker intial implementation --- pyqs/worker.py | 127 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/pyqs/worker.py b/pyqs/worker.py index ae63014..e19ee8b 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -594,3 +594,130 @@ def _replace_worker_children(self): ) worker.start() self.worker_children.append(worker) + + +class SimpleManagerWorker(object): + + def __init__(self, queue_prefixes, worker_concurrency, batchsize, + region=None, access_key_id=None, + secret_access_key=None): + self.connection_args = { + "region": region, + "access_key_id": access_key_id, + "secret_access_key": secret_access_key, + } + self.batchsize = batchsize + if batchsize > MESSAGE_DOWNLOAD_BATCH_SIZE: + self.batchsize = MESSAGE_DOWNLOAD_BATCH_SIZE + if batchsize <= 0: + self.batchsize = 1 + self.queue_prefixes = queue_prefixes + self.queue_urls = self.get_queue_urls_from_queue_prefixes( + self.queue_prefixes) + self.worker_children = [] + self._pid = os.getpid() + self._initialize_worker_children(worker_concurrency) + self._running = True + self._register_signals() + + def _register_signals(self): + for SIG in [signal.SIGINT, signal.SIGTERM, signal.SIGQUIT, + signal.SIGHUP]: + self.register_shutdown_signal(SIG) + + def _initialize_worker_children(self, number): + for queue_url in self.queue_urls: + for index in range(number): + self.worker_children.append( + SimpleProcessWorker( + queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + ) + + def get_queue_urls_from_queue_prefixes(self, queue_prefixes): + conn = get_conn(**self.connection_args) + queue_urls = conn.list_queues().get('QueueUrls', []) + matching_urls = [] + + logger.info("Loading Queues:") + for prefix in queue_prefixes: + logger.info("[Queue]\t{}".format(prefix)) + matching_urls.extend([ + queue_url for queue_url in queue_urls if + fnmatch.fnmatch(queue_url.rsplit("/", 1)[1], prefix) + ]) + logger.info("Found matching SQS Queues: {}".format(matching_urls)) + return matching_urls + + def check_for_new_queues(self): + queue_urls = self.get_queue_urls_from_queue_prefixes( + self.queue_prefixes) + new_queue_urls = set(queue_urls) - set(self.queue_urls) + for new_queue_url in new_queue_urls: + logger.info("Found new queue\t{}".format(new_queue_url)) + worker = SimpleProcessWorker( + new_queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + worker.start() + self.worker_children.append(worker) + + def start(self): + for child in self.worker_children: + child.start() + + def stop(self): + for child in self.worker_children: + child.shutdown() + for child in self.worker_children: + child.join() + + def sleep(self): + counter = 0 + while self._running: + counter = counter + 1 + if counter % 1000 == 0: + self.process_counts() + self.replace_workers() + if counter % 30000 == 0: + counter = 0 + self.check_for_new_queues() + time.sleep(0.001) + self._exit() + + def register_shutdown_signal(self, SIG): + signal.signal(SIG, self._graceful_shutdown) + + def _graceful_shutdown(self, signum, frame): + logger.info('Received shutdown signal %s', signum) + self._running = False + + def _exit(self): + logger.info('Graceful shutdown. Sending shutdown signal to children.') + self.stop() + sys.exit(0) + + def process_counts(self): + worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) + logger.debug("Worker Processes: {}".format(worker_count)) + + def replace_workers(self): + self._replace_worker_children() + + def _replace_worker_children(self): + for index, worker in enumerate(self.worker_children): + if not worker.is_alive(): + logger.info( + "Worker Process {} is no longer responding, " + "spawning a new worker.".format(worker.pid)) + self.worker_children.pop(index) + worker = SimpleProcessWorker( + worker.queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + worker.start() + self.worker_children.append(worker) From c9bb5421e4d0403938d2e464f402b7380d06abc5 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 7 Apr 2021 16:16:36 -0400 Subject: [PATCH 14/49] Add a simple-worker flag to use the SimpleManagerWorker class --- pyqs/main.py | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index 33b55b5..fd95b26 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -7,7 +7,7 @@ import sys from argparse import ArgumentParser -from .worker import ManagerWorker +from .worker import ManagerWorker, SimpleManagerWorker from . import __version__ logger = logging.getLogger("pyqs") @@ -107,6 +107,13 @@ def main(): action="store", ) + parser.add_argument( + '--simple-worker', + dest='simple_worker', + default=False, + action='store_true' + ) + args = parser.parse_args() _main( @@ -118,7 +125,8 @@ def main(): secret_access_key=args.secret_access_key, interval=args.interval, batchsize=args.batchsize, - prefetch_multiplier=args.prefetch_multiplier + prefetch_multiplier=args.prefetch_multiplier, + simple_worker=args.simple_worker ) @@ -130,17 +138,27 @@ def _add_cwd_to_path(): def _main(queue_prefixes, concurrency=5, logging_level="WARN", region=None, access_key_id=None, secret_access_key=None, - interval=1, batchsize=10, prefetch_multiplier=2): + interval=1, batchsize=10, prefetch_multiplier=2, + simple_worker=False): logging.basicConfig( format="[%(levelname)s]: %(message)s", level=getattr(logging, logging_level), ) logger.info("Starting PyQS version {}".format(__version__)) - manager = ManagerWorker( - queue_prefixes, concurrency, interval, batchsize, - prefetch_multiplier=prefetch_multiplier, region=region, - access_key_id=access_key_id, secret_access_key=secret_access_key, - ) + + if simple_worker: + manager = SimpleManagerWorker( + queue_prefixes, concurrency, batchsize, + region=region, access_key_id=access_key_id, + secret_access_key=secret_access_key, + ) + else: + manager = ManagerWorker( + queue_prefixes, concurrency, interval, batchsize, + prefetch_multiplier=prefetch_multiplier, region=region, + access_key_id=access_key_id, secret_access_key=secret_access_key, + ) + _add_cwd_to_path() manager.start() manager.sleep() From f3af494d7f95a6f4a31f5799a37c7637cbef0128 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 7 Apr 2021 16:20:33 -0400 Subject: [PATCH 15/49] Fix manager worker tests --- tests/test_manager_worker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index 3089195..55371c4 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -111,14 +111,14 @@ def test_real_main_method(ArgumentParser, _main): ArgumentParser.return_value.parse_args.return_value = Mock( concurrency=3, queues=["email1"], interval=1, batchsize=10, logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, + access_key_id=None, secret_access_key=None, simple_worker=False, ) main() _main.assert_called_once_with( queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=10, logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, + access_key_id=None, secret_access_key=None, simple_worker=False, ) From 394246ddbcb61566aa83582ad1f99a405c1e457d Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Fri, 9 Apr 2021 13:28:12 -0400 Subject: [PATCH 16/49] Remove visibility timeout check from SimpleProcessWorkerbut keep the visibility timeout info --- pyqs/worker.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index e19ee8b..d8216d0 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -314,18 +314,6 @@ def read_message(self): start = time.time() for message in messages: - end = time.time() - if int(end - start) >= self.visibility_timeout: - # Don't add any more messages since they have - # re-appeared in the sqs queue Instead just reset and get - # fresh messages from the sqs queue - msg = ( - "Clearing Local messages since we exceeded " - "their visibility_timeout" - ) - logger.warning(msg) - break - packed_message = { "queue": self.queue_url, "message": message, @@ -367,18 +355,6 @@ def process_message(self, packed_message): # context isn't modified by later processing post_process_context = copy.copy(pre_process_context) - current_time = time.time() - if int(current_time - fetch_time) >= timeout: - logger.warning( - "Discarding task {} with args: {} and kwargs: {} due to " - "exceeding visibility timeout".format( # noqa - full_task_path, - repr(args), - repr(kwargs), - ) - ) - self.messages_processed += 1 - start_time = time.time() try: From c2041a1a7ba6c8763822556d75860c642a640cf5 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Fri, 9 Apr 2021 14:27:43 -0400 Subject: [PATCH 17/49] Default batchsize to be 1 if simple-worker flag is passed --- pyqs/main.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index fd95b26..54906a3 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -13,6 +13,20 @@ logger = logging.getLogger("pyqs") +def _set_batchsize(args): + batchsize = args.batchsize + if batchsize: + return batchsize + + simple_worker = args.simple_worker + if simple_worker: + # Default batchsize for SimpleProcessWorker + return 1 + + # Default batchsize for ProcessWorker + return 10 + + def main(): parser = ArgumentParser(description=""" Run PyQS workers for the given queues @@ -90,7 +104,7 @@ def main(): "--batchsize", dest="batchsize", type=int, - default=10, + default=None, help='How many messages to download at a time from SQS.', action="store", ) @@ -124,7 +138,7 @@ def main(): access_key_id=args.access_key_id, secret_access_key=args.secret_access_key, interval=args.interval, - batchsize=args.batchsize, + batchsize=_set_batchsize(args), prefetch_multiplier=args.prefetch_multiplier, simple_worker=args.simple_worker ) From e02acd2999fc3f91c35916cd051751d4412aa46c Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Fri, 9 Apr 2021 16:02:05 -0400 Subject: [PATCH 18/49] Rename SimpleManagerWorker to BaseManager --- pyqs/main.py | 13 +++-- pyqs/worker.py | 152 ++++++++++++++++++++++++------------------------- 2 files changed, 84 insertions(+), 81 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index 54906a3..6464ba2 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -7,11 +7,14 @@ import sys from argparse import ArgumentParser -from .worker import ManagerWorker, SimpleManagerWorker +from .worker import ManagerWorker, BaseManager from . import __version__ logger = logging.getLogger("pyqs") +SIMPLE_WORKER_DEFAULT_BATCH_SIZE = 1 +DEFAULT_BATCH_SIZE = 10 + def _set_batchsize(args): batchsize = args.batchsize @@ -21,10 +24,10 @@ def _set_batchsize(args): simple_worker = args.simple_worker if simple_worker: # Default batchsize for SimpleProcessWorker - return 1 + return SIMPLE_WORKER_DEFAULT_BATCH_SIZE # Default batchsize for ProcessWorker - return 10 + return DEFAULT_BATCH_SIZE def main(): @@ -152,7 +155,7 @@ def _add_cwd_to_path(): def _main(queue_prefixes, concurrency=5, logging_level="WARN", region=None, access_key_id=None, secret_access_key=None, - interval=1, batchsize=10, prefetch_multiplier=2, + interval=1, batchsize=DEFAULT_BATCH_SIZE, prefetch_multiplier=2, simple_worker=False): logging.basicConfig( format="[%(levelname)s]: %(message)s", @@ -161,7 +164,7 @@ def _main(queue_prefixes, concurrency=5, logging_level="WARN", logger.info("Starting PyQS version {}".format(__version__)) if simple_worker: - manager = SimpleManagerWorker( + manager = BaseManager( queue_prefixes, concurrency, batchsize, region=region, access_key_id=access_key_id, secret_access_key=secret_access_key, diff --git a/pyqs/worker.py b/pyqs/worker.py index d8216d0..242a934 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -401,10 +401,10 @@ def _run_hooks(self, hook_name, context): hook(context) -class ManagerWorker(object): +class BaseManager(object): - def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, - prefetch_multiplier=2, region=None, access_key_id=None, + def __init__(self, queue_prefixes, worker_concurrency, batchsize, + region=None, access_key_id=None, secret_access_key=None): self.connection_args = { "region": region, @@ -416,16 +416,11 @@ def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, self.batchsize = MESSAGE_DOWNLOAD_BATCH_SIZE if batchsize <= 0: self.batchsize = 1 - self.interval = interval - self.prefetch_multiplier = prefetch_multiplier self.queue_prefixes = queue_prefixes self.queue_urls = self.get_queue_urls_from_queue_prefixes( self.queue_prefixes) - self.setup_internal_queue(worker_concurrency) - self.reader_children = [] self.worker_children = [] self._pid = os.getpid() - self._initialize_reader_children() self._initialize_worker_children(worker_concurrency) self._running = True self._register_signals() @@ -435,25 +430,16 @@ def _register_signals(self): signal.SIGHUP]: self.register_shutdown_signal(SIG) - def _initialize_reader_children(self): - for queue_url in self.queue_urls: - self.reader_children.append( - ReadWorker( - queue_url, self.internal_queue, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) - ) - def _initialize_worker_children(self, number): - for index in range(number): - self.worker_children.append( - ProcessWorker( - self.internal_queue, self.interval, - connection_args=self.connection_args, - parent_id=self._pid, + for queue_url in self.queue_urls: + for index in range(number): + self.worker_children.append( + SimpleProcessWorker( + queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) ) - ) def get_queue_urls_from_queue_prefixes(self, queue_prefixes): conn = get_conn(**self.connection_args) @@ -476,30 +462,19 @@ def check_for_new_queues(self): new_queue_urls = set(queue_urls) - set(self.queue_urls) for new_queue_url in new_queue_urls: logger.info("Found new queue\t{}".format(new_queue_url)) - worker = ReadWorker( - new_queue_url, self.internal_queue, self.batchsize, + worker = SimpleProcessWorker( + new_queue_url, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) worker.start() - self.reader_children.append(worker) - - def setup_internal_queue(self, worker_concurrency): - self.internal_queue = Queue( - worker_concurrency * self.prefetch_multiplier * self.batchsize) + self.worker_children.append(worker) def start(self): - for child in self.reader_children: - child.start() for child in self.worker_children: child.start() def stop(self): - for child in self.reader_children: - child.shutdown() - for child in self.reader_children: - child.join() - for child in self.worker_children: child.shutdown() for child in self.worker_children: @@ -531,31 +506,12 @@ def _exit(self): sys.exit(0) def process_counts(self): - reader_count = sum(map(lambda x: x.is_alive(), self.reader_children)) worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) - logger.debug("Reader Processes: {}".format(reader_count)) logger.debug("Worker Processes: {}".format(worker_count)) def replace_workers(self): - self._replace_reader_children() self._replace_worker_children() - def _replace_reader_children(self): - for index, reader in enumerate(self.reader_children): - if not reader.is_alive(): - logger.info( - "Reader Process {} is no longer responding, " - "spawning a new reader.".format(reader.pid)) - queue_url = reader.queue_url - self.reader_children.pop(index) - worker = ReadWorker( - queue_url, self.internal_queue, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) - worker.start() - self.reader_children.append(worker) - def _replace_worker_children(self): for index, worker in enumerate(self.worker_children): if not worker.is_alive(): @@ -563,8 +519,8 @@ def _replace_worker_children(self): "Worker Process {} is no longer responding, " "spawning a new worker.".format(worker.pid)) self.worker_children.pop(index) - worker = ProcessWorker( - self.internal_queue, self.interval, + worker = SimpleProcessWorker( + worker.queue_url, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) @@ -572,10 +528,10 @@ def _replace_worker_children(self): self.worker_children.append(worker) -class SimpleManagerWorker(object): +class ManagerWorker(object): - def __init__(self, queue_prefixes, worker_concurrency, batchsize, - region=None, access_key_id=None, + def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, + prefetch_multiplier=2, region=None, access_key_id=None, secret_access_key=None): self.connection_args = { "region": region, @@ -587,11 +543,16 @@ def __init__(self, queue_prefixes, worker_concurrency, batchsize, self.batchsize = MESSAGE_DOWNLOAD_BATCH_SIZE if batchsize <= 0: self.batchsize = 1 + self.interval = interval + self.prefetch_multiplier = prefetch_multiplier self.queue_prefixes = queue_prefixes self.queue_urls = self.get_queue_urls_from_queue_prefixes( self.queue_prefixes) + self.setup_internal_queue(worker_concurrency) + self.reader_children = [] self.worker_children = [] self._pid = os.getpid() + self._initialize_reader_children() self._initialize_worker_children(worker_concurrency) self._running = True self._register_signals() @@ -601,16 +562,25 @@ def _register_signals(self): signal.SIGHUP]: self.register_shutdown_signal(SIG) - def _initialize_worker_children(self, number): + def _initialize_reader_children(self): for queue_url in self.queue_urls: - for index in range(number): - self.worker_children.append( - SimpleProcessWorker( - queue_url, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) + self.reader_children.append( + ReadWorker( + queue_url, self.internal_queue, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, ) + ) + + def _initialize_worker_children(self, number): + for index in range(number): + self.worker_children.append( + ProcessWorker( + self.internal_queue, self.interval, + connection_args=self.connection_args, + parent_id=self._pid, + ) + ) def get_queue_urls_from_queue_prefixes(self, queue_prefixes): conn = get_conn(**self.connection_args) @@ -633,19 +603,30 @@ def check_for_new_queues(self): new_queue_urls = set(queue_urls) - set(self.queue_urls) for new_queue_url in new_queue_urls: logger.info("Found new queue\t{}".format(new_queue_url)) - worker = SimpleProcessWorker( - new_queue_url, self.batchsize, + worker = ReadWorker( + new_queue_url, self.internal_queue, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) worker.start() - self.worker_children.append(worker) + self.reader_children.append(worker) + + def setup_internal_queue(self, worker_concurrency): + self.internal_queue = Queue( + worker_concurrency * self.prefetch_multiplier * self.batchsize) def start(self): + for child in self.reader_children: + child.start() for child in self.worker_children: child.start() def stop(self): + for child in self.reader_children: + child.shutdown() + for child in self.reader_children: + child.join() + for child in self.worker_children: child.shutdown() for child in self.worker_children: @@ -677,12 +658,31 @@ def _exit(self): sys.exit(0) def process_counts(self): + reader_count = sum(map(lambda x: x.is_alive(), self.reader_children)) worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) + logger.debug("Reader Processes: {}".format(reader_count)) logger.debug("Worker Processes: {}".format(worker_count)) def replace_workers(self): + self._replace_reader_children() self._replace_worker_children() + def _replace_reader_children(self): + for index, reader in enumerate(self.reader_children): + if not reader.is_alive(): + logger.info( + "Reader Process {} is no longer responding, " + "spawning a new reader.".format(reader.pid)) + queue_url = reader.queue_url + self.reader_children.pop(index) + worker = ReadWorker( + queue_url, self.internal_queue, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + worker.start() + self.reader_children.append(worker) + def _replace_worker_children(self): for index, worker in enumerate(self.worker_children): if not worker.is_alive(): @@ -690,8 +690,8 @@ def _replace_worker_children(self): "Worker Process {} is no longer responding, " "spawning a new worker.".format(worker.pid)) self.worker_children.pop(index) - worker = SimpleProcessWorker( - worker.queue_url, self.batchsize, + worker = ProcessWorker( + self.internal_queue, self.interval, connection_args=self.connection_args, parent_id=self._pid, ) From 1ef4c9fd2ab850589bc16adb0e928869ce60bb7c Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 16:15:09 -0400 Subject: [PATCH 19/49] Subclass SimpleManagerWorker and ManagerWorker from BaseManager --- pyqs/worker.py | 108 ++++++++++++++----------------------------------- 1 file changed, 31 insertions(+), 77 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 242a934..0d1f9b1 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -403,7 +403,7 @@ def _run_hooks(self, hook_name, context): class BaseManager(object): - def __init__(self, queue_prefixes, worker_concurrency, batchsize, + def __init__(self, queue_prefixes, batchsize, region=None, access_key_id=None, secret_access_key=None): self.connection_args = { @@ -421,7 +421,6 @@ def __init__(self, queue_prefixes, worker_concurrency, batchsize, self.queue_prefixes) self.worker_children = [] self._pid = os.getpid() - self._initialize_worker_children(worker_concurrency) self._running = True self._register_signals() @@ -430,17 +429,6 @@ def _register_signals(self): signal.SIGHUP]: self.register_shutdown_signal(SIG) - def _initialize_worker_children(self, number): - for queue_url in self.queue_urls: - for index in range(number): - self.worker_children.append( - SimpleProcessWorker( - queue_url, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) - ) - def get_queue_urls_from_queue_prefixes(self, queue_prefixes): conn = get_conn(**self.connection_args) queue_urls = conn.list_queues().get('QueueUrls', []) @@ -528,39 +516,45 @@ def _replace_worker_children(self): self.worker_children.append(worker) -class ManagerWorker(object): +class SimpleManagerWorker(BaseManager): + + def __init__(self, queue_prefixes, worker_concurrency, batchsize, + region=None, access_key_id=None, secret_access_key=None): + + super(SimpleManagerWorker, self).__init__(queue_prefixes, batchsize, + region, access_key_id, + secret_access_key) + + self._initialize_worker_children(worker_concurrency) + + def _initialize_worker_children(self, number): + for queue_url in self.queue_urls: + for index in range(number): + self.worker_children.append( + SimpleProcessWorker( + queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + ) + + +class ManagerWorker(BaseManager): def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, prefetch_multiplier=2, region=None, access_key_id=None, secret_access_key=None): - self.connection_args = { - "region": region, - "access_key_id": access_key_id, - "secret_access_key": secret_access_key, - } - self.batchsize = batchsize - if batchsize > MESSAGE_DOWNLOAD_BATCH_SIZE: - self.batchsize = MESSAGE_DOWNLOAD_BATCH_SIZE - if batchsize <= 0: - self.batchsize = 1 + + super(ManagerWorker, self).__init__(queue_prefixes, batchsize, + region, access_key_id, + secret_access_key) self.interval = interval self.prefetch_multiplier = prefetch_multiplier - self.queue_prefixes = queue_prefixes - self.queue_urls = self.get_queue_urls_from_queue_prefixes( - self.queue_prefixes) - self.setup_internal_queue(worker_concurrency) self.reader_children = [] - self.worker_children = [] - self._pid = os.getpid() + + self.setup_internal_queue(worker_concurrency) self._initialize_reader_children() self._initialize_worker_children(worker_concurrency) - self._running = True - self._register_signals() - - def _register_signals(self): - for SIG in [signal.SIGINT, signal.SIGTERM, signal.SIGQUIT, - signal.SIGHUP]: - self.register_shutdown_signal(SIG) def _initialize_reader_children(self): for queue_url in self.queue_urls: @@ -582,21 +576,6 @@ def _initialize_worker_children(self, number): ) ) - def get_queue_urls_from_queue_prefixes(self, queue_prefixes): - conn = get_conn(**self.connection_args) - queue_urls = conn.list_queues().get('QueueUrls', []) - matching_urls = [] - - logger.info("Loading Queues:") - for prefix in queue_prefixes: - logger.info("[Queue]\t{}".format(prefix)) - matching_urls.extend([ - queue_url for queue_url in queue_urls if - fnmatch.fnmatch(queue_url.rsplit("/", 1)[1], prefix) - ]) - logger.info("Found matching SQS Queues: {}".format(matching_urls)) - return matching_urls - def check_for_new_queues(self): queue_urls = self.get_queue_urls_from_queue_prefixes( self.queue_prefixes) @@ -632,31 +611,6 @@ def stop(self): for child in self.worker_children: child.join() - def sleep(self): - counter = 0 - while self._running: - counter = counter + 1 - if counter % 1000 == 0: - self.process_counts() - self.replace_workers() - if counter % 30000 == 0: - counter = 0 - self.check_for_new_queues() - time.sleep(0.001) - self._exit() - - def register_shutdown_signal(self, SIG): - signal.signal(SIG, self._graceful_shutdown) - - def _graceful_shutdown(self, signum, frame): - logger.info('Received shutdown signal %s', signum) - self._running = False - - def _exit(self): - logger.info('Graceful shutdown. Sending shutdown signal to children.') - self.stop() - sys.exit(0) - def process_counts(self): reader_count = sum(map(lambda x: x.is_alive(), self.reader_children)) worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) From ab0d385c43e13af139225b141d318219693d8c26 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 17:15:57 -0400 Subject: [PATCH 20/49] Update BaseManager to SimpleManagerWorker for main --- pyqs/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index 6464ba2..0e5c781 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -7,7 +7,7 @@ import sys from argparse import ArgumentParser -from .worker import ManagerWorker, BaseManager +from .worker import ManagerWorker, SimpleManagerWorker from . import __version__ logger = logging.getLogger("pyqs") @@ -164,7 +164,7 @@ def _main(queue_prefixes, concurrency=5, logging_level="WARN", logger.info("Starting PyQS version {}".format(__version__)) if simple_worker: - manager = BaseManager( + manager = SimpleManagerWorker( queue_prefixes, concurrency, batchsize, region=region, access_key_id=access_key_id, secret_access_key=secret_access_key, From fdd9d4096f16b94698ad26c2fa1e355a19edfe73 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 17:17:44 -0400 Subject: [PATCH 21/49] Move SimpleProcessWorker methods to SimpleManagerWorker sub class --- pyqs/worker.py | 57 ++++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 0d1f9b1..f57d916 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -445,18 +445,7 @@ def get_queue_urls_from_queue_prefixes(self, queue_prefixes): return matching_urls def check_for_new_queues(self): - queue_urls = self.get_queue_urls_from_queue_prefixes( - self.queue_prefixes) - new_queue_urls = set(queue_urls) - set(self.queue_urls) - for new_queue_url in new_queue_urls: - logger.info("Found new queue\t{}".format(new_queue_url)) - worker = SimpleProcessWorker( - new_queue_url, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) - worker.start() - self.worker_children.append(worker) + raise NotImplementedError def start(self): for child in self.worker_children: @@ -500,21 +489,6 @@ def process_counts(self): def replace_workers(self): self._replace_worker_children() - def _replace_worker_children(self): - for index, worker in enumerate(self.worker_children): - if not worker.is_alive(): - logger.info( - "Worker Process {} is no longer responding, " - "spawning a new worker.".format(worker.pid)) - self.worker_children.pop(index) - worker = SimpleProcessWorker( - worker.queue_url, self.batchsize, - connection_args=self.connection_args, - parent_id=self._pid, - ) - worker.start() - self.worker_children.append(worker) - class SimpleManagerWorker(BaseManager): @@ -538,6 +512,35 @@ def _initialize_worker_children(self, number): ) ) + def check_for_new_queues(self): + queue_urls = self.get_queue_urls_from_queue_prefixes( + self.queue_prefixes) + new_queue_urls = set(queue_urls) - set(self.queue_urls) + for new_queue_url in new_queue_urls: + logger.info("Found new queue\t{}".format(new_queue_url)) + worker = SimpleProcessWorker( + new_queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + worker.start() + self.worker_children.append(worker) + + def _replace_worker_children(self): + for index, worker in enumerate(self.worker_children): + if not worker.is_alive(): + logger.info( + "Worker Process {} is no longer responding, " + "spawning a new worker.".format(worker.pid)) + self.worker_children.pop(index) + worker = SimpleProcessWorker( + worker.queue_url, self.batchsize, + connection_args=self.connection_args, + parent_id=self._pid, + ) + worker.start() + self.worker_children.append(worker) + class ManagerWorker(BaseManager): From f0e98b38ef7119b733b7bda7347bd6dcb81b9c42 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 17:21:07 -0400 Subject: [PATCH 22/49] Add methods to BaseManager that sub classes should override and implement --- pyqs/worker.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyqs/worker.py b/pyqs/worker.py index f57d916..335c111 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -489,6 +489,9 @@ def process_counts(self): def replace_workers(self): self._replace_worker_children() + def _replace_worker_children(self): + raise NotImplementedError + class SimpleManagerWorker(BaseManager): From f3b9d66146b3b874e5e7a92eb22524573700654f Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 17:50:32 -0400 Subject: [PATCH 23/49] Move worker_children state down to the subclasses --- pyqs/worker.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 335c111..a6dd3fd 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -419,7 +419,6 @@ def __init__(self, queue_prefixes, batchsize, self.queue_prefixes = queue_prefixes self.queue_urls = self.get_queue_urls_from_queue_prefixes( self.queue_prefixes) - self.worker_children = [] self._pid = os.getpid() self._running = True self._register_signals() @@ -448,14 +447,10 @@ def check_for_new_queues(self): raise NotImplementedError def start(self): - for child in self.worker_children: - child.start() + raise NotImplementedError def stop(self): - for child in self.worker_children: - child.shutdown() - for child in self.worker_children: - child.join() + raise NotImplementedError def sleep(self): counter = 0 @@ -483,8 +478,7 @@ def _exit(self): sys.exit(0) def process_counts(self): - worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) - logger.debug("Worker Processes: {}".format(worker_count)) + raise NotImplementedError def replace_workers(self): self._replace_worker_children() @@ -502,6 +496,7 @@ def __init__(self, queue_prefixes, worker_concurrency, batchsize, region, access_key_id, secret_access_key) + self.worker_children = [] self._initialize_worker_children(worker_concurrency) def _initialize_worker_children(self, number): @@ -529,6 +524,20 @@ def check_for_new_queues(self): worker.start() self.worker_children.append(worker) + def start(self): + for child in self.worker_children: + child.start() + + def stop(self): + for child in self.worker_children: + child.shutdown() + for child in self.worker_children: + child.join() + + def process_counts(self): + worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) + logger.debug("Worker Processes: {}".format(worker_count)) + def _replace_worker_children(self): for index, worker in enumerate(self.worker_children): if not worker.is_alive(): @@ -556,6 +565,7 @@ def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, secret_access_key) self.interval = interval self.prefetch_multiplier = prefetch_multiplier + self.worker_children = [] self.reader_children = [] self.setup_internal_queue(worker_concurrency) From e587c132e5f871fde291992261ae8943e57dac4c Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 12 Apr 2021 18:00:07 -0400 Subject: [PATCH 24/49] Make replace_workers something that subclasses have to implement --- pyqs/worker.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index a6dd3fd..4843cf4 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -481,9 +481,6 @@ def process_counts(self): raise NotImplementedError def replace_workers(self): - self._replace_worker_children() - - def _replace_worker_children(self): raise NotImplementedError @@ -538,7 +535,7 @@ def process_counts(self): worker_count = sum(map(lambda x: x.is_alive(), self.worker_children)) logger.debug("Worker Processes: {}".format(worker_count)) - def _replace_worker_children(self): + def replace_workers(self): for index, worker in enumerate(self.worker_children): if not worker.is_alive(): logger.info( From 0c49f79ab1fe242833e2f52aae5e5e94fddc2203 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 14 Apr 2021 13:51:13 -0400 Subject: [PATCH 25/49] Move up _run_hooks to the parent class BaseWorker --- pyqs/worker.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 4843cf4..127af0b 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -69,6 +69,11 @@ def parent_is_alive(self): return False return True + def _run_hooks(self, hook_name, context): + hooks = getattr(get_events(), hook_name) + for hook in hooks: + hook(context) + class ReadWorker(BaseWorker): @@ -261,11 +266,6 @@ def process_message(self): self._run_hooks("post_process", post_process_context) return True - def _run_hooks(self, hook_name, context): - hooks = getattr(get_events(), hook_name) - for hook in hooks: - hook(context) - class SimpleProcessWorker(BaseWorker): @@ -395,11 +395,6 @@ def process_message(self, packed_message): self._run_hooks("post_process", post_process_context) self.messages_processed += 1 - def _run_hooks(self, hook_name, context): - hooks = getattr(get_events(), hook_name) - for hook in hooks: - hook(context) - class BaseManager(object): From 4bafcfa96b6cc219b77da923baf14593425f1e43 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Wed, 14 Apr 2021 16:32:53 -0400 Subject: [PATCH 26/49] Move common pre_process_context code from child classes to BaseWorker parent --- pyqs/worker.py | 137 ++++++++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 76 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 127af0b..e679825 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -74,6 +74,33 @@ def _run_hooks(self, hook_name, context): for hook in hooks: hook(context) + def _create_pre_process_context(self, packed_message): + message = packed_message['message'] + message_body = decode_message(message) + full_task_path = message_body['task'] + + pre_process_context = { + "task_name": full_task_path.split(".")[-1], + "args": message_body['args'], + "kwargs": message_body['kwargs'], + "full_task_path": full_task_path, + "fetch_time": packed_message['start_time'], + "queue_url": packed_message['queue'], + "timeout": packed_message['timeout'], + "receipt_handle": message['ReceiptHandle'] + } + + return pre_process_context + + def _get_task(self, full_task_path): + + task_name = full_task_path.split(".")[-1] + task_path = ".".join(full_task_path.split(".")[:-1]) + task_module = importlib.import_module(task_path) + task = getattr(task_module, task_name) + + return task + class ReadWorker(BaseWorker): @@ -186,60 +213,41 @@ def process_message(self): except Empty: # Return False if we did not attempt to process any messages return False - message = packed_message['message'] - queue_url = packed_message['queue'] - fetch_time = packed_message['start_time'] - timeout = packed_message['timeout'] - message_body = decode_message(message) - full_task_path = message_body['task'] - args = message_body['args'] - kwargs = message_body['kwargs'] - - task_name = full_task_path.split(".")[-1] - task_path = ".".join(full_task_path.split(".")[:-1]) - - task_module = importlib.import_module(task_path) - - task = getattr(task_module, task_name) - - pre_process_context = { - "task_name": task_name, - "args": args, - "kwargs": kwargs, - "full_task_path": full_task_path, - "fetch_time": fetch_time, - "queue_url": queue_url, - "timeout": timeout - } - # Modify the contexts separately so the original - # context isn't modified by later processing - post_process_context = copy.copy(pre_process_context) + pre_process_context = self._create_pre_process_context(packed_message) current_time = time.time() - if int(current_time - fetch_time) >= timeout: + if int(current_time - pre_process_context["fetch_time"]) \ + >= pre_process_context["timeout"]: logger.warning( "Discarding task {} with args: {} and kwargs: {} due to " "exceeding visibility timeout".format( # noqa - full_task_path, - repr(args), - repr(kwargs), + pre_process_context["full_task_path"], + repr(pre_process_context["args"]), + repr(pre_process_context["kwargs"]), ) ) return True + + task = self._get_task(pre_process_context["full_task_path"]) + + # Modify the contexts separately so the original + # context isn't modified by later processing + post_process_context = copy.copy(pre_process_context) + try: start_time = time.time() self._run_hooks("pre_process", pre_process_context) - task(*args, **kwargs) + task(*pre_process_context["args"], **pre_process_context["kwargs"]) except Exception: end_time = time.time() logger.exception( "Task {} raised error in {:.4f} seconds: with args: {} " "and kwargs: {}: {}".format( - full_task_path, + pre_process_context["full_task_path"], end_time - start_time, - args, - kwargs, + pre_process_context["args"], + pre_process_context["kwargs"], traceback.format_exc(), ) ) @@ -250,16 +258,16 @@ def process_message(self): else: end_time = time.time() self._get_connection().delete_message( - QueueUrl=queue_url, - ReceiptHandle=message['ReceiptHandle'] + QueueUrl=pre_process_context["queue_url"], + ReceiptHandle=pre_process_context["receipt_handle"] ) logger.info( "Processed task {} in {:.4f} seconds with args: {} " "and kwargs: {}".format( - full_task_path, + pre_process_context["full_task_path"], end_time - start_time, - repr(args), - repr(kwargs), + pre_process_context["args"], + pre_process_context["kwargs"], ) ) post_process_context["status"] = "success" @@ -325,50 +333,27 @@ def read_message(self): def process_message(self, packed_message): - message = packed_message['message'] - queue_url = packed_message['queue'] - fetch_time = packed_message['start_time'] - timeout = packed_message['timeout'] - message_body = decode_message(message) - full_task_path = message_body['task'] - args = message_body['args'] - kwargs = message_body['kwargs'] - - task_name = full_task_path.split(".")[-1] - task_path = ".".join(full_task_path.split(".")[:-1]) + pre_process_context = self._create_pre_process_context(packed_message) - task_module = importlib.import_module(task_path) - - task = getattr(task_module, task_name) - - pre_process_context = { - "task_name": task_name, - "args": args, - "kwargs": kwargs, - "full_task_path": full_task_path, - "fetch_time": fetch_time, - "queue_url": queue_url, - "timeout": timeout - } + task = self._get_task(pre_process_context["full_task_path"]) # Modify the contexts separately so the original # context isn't modified by later processing post_process_context = copy.copy(pre_process_context) - start_time = time.time() - try: + start_time = time.time() self._run_hooks("pre_process", pre_process_context) - task(*args, **kwargs) + task(*pre_process_context["args"], **pre_process_context["kwargs"]) except Exception: end_time = time.time() logger.exception( "Task {} raised error in {:.4f} seconds: with args: {} " "and kwargs: {}: {}".format( - full_task_path, + pre_process_context["full_task_path"], end_time - start_time, - args, - kwargs, + pre_process_context["args"], + pre_process_context["kwargs"], traceback.format_exc(), ) ) @@ -379,16 +364,16 @@ def process_message(self, packed_message): else: end_time = time.time() self._get_connection().delete_message( - QueueUrl=queue_url, - ReceiptHandle=message['ReceiptHandle'] + QueueUrl=pre_process_context["queue_url"], + ReceiptHandle=pre_process_context["receipt_handle"] ) logger.info( "Processed task {} in {:.4f} seconds with args: {} " "and kwargs: {}".format( - full_task_path, + pre_process_context["full_task_path"], end_time - start_time, - repr(args), - repr(kwargs), + pre_process_context["args"], + pre_process_context["kwargs"], ) ) post_process_context["status"] = "success" From fbc716e678543d0c697e852ef696d8c39ed11ff1 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 10:11:29 -0400 Subject: [PATCH 27/49] Move messages_processed into instance state for ProcessWorker --- pyqs/worker.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index e679825..b138064 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -189,22 +189,24 @@ def __init__(self, internal_queue, interval, connection_args=None, *args, self.internal_queue = internal_queue self.interval = interval self._messages_to_process_before_shutdown = 100 + self.messages_processed = 0 def run(self): # Set the child process to not receive any keyboard interrupts signal.signal(signal.SIGINT, signal.SIG_IGN) logger.info("Running ProcessWorker, pid: {}".format(os.getpid())) - messages_processed = 0 + while not self.should_exit.is_set() and self.parent_is_alive(): processed = self.process_message() if processed: - messages_processed += 1 + self.messages_processed += 1 time.sleep(self.interval) else: # If we have no messages wait a moment before rechecking. time.sleep(0.001) - if messages_processed >= self._messages_to_process_before_shutdown: + if self.messages_processed \ + >= self._messages_to_process_before_shutdown: self.shutdown() def process_message(self): From 891150c6a40afb053915f9b6c8d8d81a8f3ba08e Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 10:46:09 -0400 Subject: [PATCH 28/49] Add interval back to Simple* classes --- pyqs/main.py | 2 +- pyqs/worker.py | 26 +++++++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index 0e5c781..7dec331 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -165,7 +165,7 @@ def _main(queue_prefixes, concurrency=5, logging_level="WARN", if simple_worker: manager = SimpleManagerWorker( - queue_prefixes, concurrency, batchsize, + queue_prefixes, concurrency, interval, batchsize, region=region, access_key_id=access_key_id, secret_access_key=secret_access_key, ) diff --git a/pyqs/worker.py b/pyqs/worker.py index b138064..bf559c8 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -279,7 +279,7 @@ def process_message(self): class SimpleProcessWorker(BaseWorker): - def __init__(self, queue_url, batchsize, + def __init__(self, queue_url, interval, batchsize, connection_args=None, *args, **kwargs): super(SimpleProcessWorker, self).__init__(*args, **kwargs) if connection_args is None: @@ -291,6 +291,7 @@ def __init__(self, queue_url, batchsize, QueueUrl=queue_url, AttributeNames=['All'])['Attributes'] self.visibility_timeout = int(sqs_queue['VisibilityTimeout']) + self.interval = interval self.batchsize = batchsize self._messages_to_process_before_shutdown = 100 self.messages_processed = 0 @@ -385,7 +386,7 @@ def process_message(self, packed_message): class BaseManager(object): - def __init__(self, queue_prefixes, batchsize, + def __init__(self, queue_prefixes, interval, batchsize, region=None, access_key_id=None, secret_access_key=None): self.connection_args = { @@ -393,6 +394,7 @@ def __init__(self, queue_prefixes, batchsize, "access_key_id": access_key_id, "secret_access_key": secret_access_key, } + self.interval = interval self.batchsize = batchsize if batchsize > MESSAGE_DOWNLOAD_BATCH_SIZE: self.batchsize = MESSAGE_DOWNLOAD_BATCH_SIZE @@ -468,11 +470,12 @@ def replace_workers(self): class SimpleManagerWorker(BaseManager): - def __init__(self, queue_prefixes, worker_concurrency, batchsize, + def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, region=None, access_key_id=None, secret_access_key=None): - super(SimpleManagerWorker, self).__init__(queue_prefixes, batchsize, - region, access_key_id, + super(SimpleManagerWorker, self).__init__(queue_prefixes, interval, + batchsize, region, + access_key_id, secret_access_key) self.worker_children = [] @@ -483,7 +486,7 @@ def _initialize_worker_children(self, number): for index in range(number): self.worker_children.append( SimpleProcessWorker( - queue_url, self.batchsize, + queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) @@ -496,7 +499,7 @@ def check_for_new_queues(self): for new_queue_url in new_queue_urls: logger.info("Found new queue\t{}".format(new_queue_url)) worker = SimpleProcessWorker( - new_queue_url, self.batchsize, + new_queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) @@ -525,7 +528,7 @@ def replace_workers(self): "spawning a new worker.".format(worker.pid)) self.worker_children.pop(index) worker = SimpleProcessWorker( - worker.queue_url, self.batchsize, + worker.queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, ) @@ -539,10 +542,11 @@ def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, prefetch_multiplier=2, region=None, access_key_id=None, secret_access_key=None): - super(ManagerWorker, self).__init__(queue_prefixes, batchsize, - region, access_key_id, + super(ManagerWorker, self).__init__(queue_prefixes, interval, + batchsize, region, + access_key_id, secret_access_key) - self.interval = interval + self.prefetch_multiplier = prefetch_multiplier self.worker_children = [] self.reader_children = [] From 419452612cec923686363bd48e17a614f817b08e Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 11:06:03 -0400 Subject: [PATCH 29/49] Refactor ProcessWorker and SimpleProcessWorker to better align process_message --- pyqs/worker.py | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index bf559c8..666adf8 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -237,8 +237,8 @@ def process_message(self): # context isn't modified by later processing post_process_context = copy.copy(pre_process_context) + start_time = time.time() try: - start_time = time.time() self._run_hooks("pre_process", pre_process_context) task(*pre_process_context["args"], **pre_process_context["kwargs"]) except Exception: @@ -305,7 +305,22 @@ def run(self): self.queue_url, os.getpid())) while not self.should_exit.is_set() and self.parent_is_alive(): - self.read_message() + messages = self.read_message() + start = time.time() + + for message in messages: + packed_message = { + "queue": self.queue_url, + "message": message, + "start_time": start, + "timeout": self.visibility_timeout, + } + + processed = self.process_message(packed_message) + + if processed: + self.messages_processed += 1 + time.sleep(self.interval) if self.messages_processed \ >= self._messages_to_process_before_shutdown: @@ -322,17 +337,7 @@ def read_message(self): "Successfully got {} messages from SQS queue {}".format( len(messages), self.queue_url)) # noqa - start = time.time() - - for message in messages: - packed_message = { - "queue": self.queue_url, - "message": message, - "start_time": start, - "timeout": self.visibility_timeout, - } - - self.process_message(packed_message) + return messages def process_message(self, packed_message): @@ -344,8 +349,8 @@ def process_message(self, packed_message): # context isn't modified by later processing post_process_context = copy.copy(pre_process_context) + start_time = time.time() try: - start_time = time.time() self._run_hooks("pre_process", pre_process_context) task(*pre_process_context["args"], **pre_process_context["kwargs"]) except Exception: @@ -363,7 +368,7 @@ def process_message(self, packed_message): post_process_context["status"] = "exception" post_process_context["exception"] = traceback.format_exc() self._run_hooks("post_process", post_process_context) - self.messages_processed += 1 + return True else: end_time = time.time() self._get_connection().delete_message( @@ -381,7 +386,7 @@ def process_message(self, packed_message): ) post_process_context["status"] = "success" self._run_hooks("post_process", post_process_context) - self.messages_processed += 1 + return True class BaseManager(object): From ad710a993ea582ba58e51b28700891e60860db34 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 11:21:38 -0400 Subject: [PATCH 30/49] Move _process_task into parent BaseWorker --- pyqs/worker.py | 136 +++++++++++++++++-------------------------------- 1 file changed, 48 insertions(+), 88 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 666adf8..e4a008e 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -101,6 +101,52 @@ def _get_task(self, full_task_path): return task + def _process_task(self, pre_process_context): + task = self._get_task(pre_process_context["full_task_path"]) + + # Modify the contexts separately so the original + # context isn't modified by later processing + post_process_context = copy.copy(pre_process_context) + + start_time = time.time() + try: + self._run_hooks("pre_process", pre_process_context) + task(*pre_process_context["args"], **pre_process_context["kwargs"]) + except Exception: + end_time = time.time() + logger.exception( + "Task {} raised error in {:.4f} seconds: with args: {} " + "and kwargs: {}: {}".format( + pre_process_context["full_task_path"], + end_time - start_time, + pre_process_context["args"], + pre_process_context["kwargs"], + traceback.format_exc(), + ) + ) + post_process_context["status"] = "exception" + post_process_context["exception"] = traceback.format_exc() + self._run_hooks("post_process", post_process_context) + return True + else: + end_time = time.time() + self._get_connection().delete_message( + QueueUrl=pre_process_context["queue_url"], + ReceiptHandle=pre_process_context["receipt_handle"] + ) + logger.info( + "Processed task {} in {:.4f} seconds with args: {} " + "and kwargs: {}".format( + pre_process_context["full_task_path"], + end_time - start_time, + pre_process_context["args"], + pre_process_context["kwargs"], + ) + ) + post_process_context["status"] = "success" + self._run_hooks("post_process", post_process_context) + return True + class ReadWorker(BaseWorker): @@ -231,50 +277,7 @@ def process_message(self): ) return True - task = self._get_task(pre_process_context["full_task_path"]) - - # Modify the contexts separately so the original - # context isn't modified by later processing - post_process_context = copy.copy(pre_process_context) - - start_time = time.time() - try: - self._run_hooks("pre_process", pre_process_context) - task(*pre_process_context["args"], **pre_process_context["kwargs"]) - except Exception: - end_time = time.time() - logger.exception( - "Task {} raised error in {:.4f} seconds: with args: {} " - "and kwargs: {}: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - traceback.format_exc(), - ) - ) - post_process_context["status"] = "exception" - post_process_context["exception"] = traceback.format_exc() - self._run_hooks("post_process", post_process_context) - return True - else: - end_time = time.time() - self._get_connection().delete_message( - QueueUrl=pre_process_context["queue_url"], - ReceiptHandle=pre_process_context["receipt_handle"] - ) - logger.info( - "Processed task {} in {:.4f} seconds with args: {} " - "and kwargs: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - ) - ) - post_process_context["status"] = "success" - self._run_hooks("post_process", post_process_context) - return True + return self._process_task(pre_process_context) class SimpleProcessWorker(BaseWorker): @@ -343,50 +346,7 @@ def process_message(self, packed_message): pre_process_context = self._create_pre_process_context(packed_message) - task = self._get_task(pre_process_context["full_task_path"]) - - # Modify the contexts separately so the original - # context isn't modified by later processing - post_process_context = copy.copy(pre_process_context) - - start_time = time.time() - try: - self._run_hooks("pre_process", pre_process_context) - task(*pre_process_context["args"], **pre_process_context["kwargs"]) - except Exception: - end_time = time.time() - logger.exception( - "Task {} raised error in {:.4f} seconds: with args: {} " - "and kwargs: {}: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - traceback.format_exc(), - ) - ) - post_process_context["status"] = "exception" - post_process_context["exception"] = traceback.format_exc() - self._run_hooks("post_process", post_process_context) - return True - else: - end_time = time.time() - self._get_connection().delete_message( - QueueUrl=pre_process_context["queue_url"], - ReceiptHandle=pre_process_context["receipt_handle"] - ) - logger.info( - "Processed task {} in {:.4f} seconds with args: {} " - "and kwargs: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - ) - ) - post_process_context["status"] = "success" - self._run_hooks("post_process", post_process_context) - return True + return self._process_task(pre_process_context) class BaseManager(object): From 0a062692359b9c8bc57d74a1ef910473c09931d2 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 11:41:30 -0400 Subject: [PATCH 31/49] Created a new base class BaseProcessWorker so that we don't give ReadWorker methods it doesn't need --- pyqs/worker.py | 165 +++++++++++++++++++++++++------------------------ 1 file changed, 85 insertions(+), 80 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index e4a008e..16c228f 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -69,84 +69,6 @@ def parent_is_alive(self): return False return True - def _run_hooks(self, hook_name, context): - hooks = getattr(get_events(), hook_name) - for hook in hooks: - hook(context) - - def _create_pre_process_context(self, packed_message): - message = packed_message['message'] - message_body = decode_message(message) - full_task_path = message_body['task'] - - pre_process_context = { - "task_name": full_task_path.split(".")[-1], - "args": message_body['args'], - "kwargs": message_body['kwargs'], - "full_task_path": full_task_path, - "fetch_time": packed_message['start_time'], - "queue_url": packed_message['queue'], - "timeout": packed_message['timeout'], - "receipt_handle": message['ReceiptHandle'] - } - - return pre_process_context - - def _get_task(self, full_task_path): - - task_name = full_task_path.split(".")[-1] - task_path = ".".join(full_task_path.split(".")[:-1]) - task_module = importlib.import_module(task_path) - task = getattr(task_module, task_name) - - return task - - def _process_task(self, pre_process_context): - task = self._get_task(pre_process_context["full_task_path"]) - - # Modify the contexts separately so the original - # context isn't modified by later processing - post_process_context = copy.copy(pre_process_context) - - start_time = time.time() - try: - self._run_hooks("pre_process", pre_process_context) - task(*pre_process_context["args"], **pre_process_context["kwargs"]) - except Exception: - end_time = time.time() - logger.exception( - "Task {} raised error in {:.4f} seconds: with args: {} " - "and kwargs: {}: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - traceback.format_exc(), - ) - ) - post_process_context["status"] = "exception" - post_process_context["exception"] = traceback.format_exc() - self._run_hooks("post_process", post_process_context) - return True - else: - end_time = time.time() - self._get_connection().delete_message( - QueueUrl=pre_process_context["queue_url"], - ReceiptHandle=pre_process_context["receipt_handle"] - ) - logger.info( - "Processed task {} in {:.4f} seconds with args: {} " - "and kwargs: {}".format( - pre_process_context["full_task_path"], - end_time - start_time, - pre_process_context["args"], - pre_process_context["kwargs"], - ) - ) - post_process_context["status"] = "success" - self._run_hooks("post_process", post_process_context) - return True - class ReadWorker(BaseWorker): @@ -226,7 +148,90 @@ def read_message(self): self.queue_url, message_body)) # noqa -class ProcessWorker(BaseWorker): +class BaseProcessWorker(BaseWorker): + def __init__(self, *args, **kwargs): + super(BaseProcessWorker, self).__init__(*args, **kwargs) + + def _run_hooks(self, hook_name, context): + hooks = getattr(get_events(), hook_name) + for hook in hooks: + hook(context) + + def _create_pre_process_context(self, packed_message): + message = packed_message['message'] + message_body = decode_message(message) + full_task_path = message_body['task'] + + pre_process_context = { + "task_name": full_task_path.split(".")[-1], + "args": message_body['args'], + "kwargs": message_body['kwargs'], + "full_task_path": full_task_path, + "fetch_time": packed_message['start_time'], + "queue_url": packed_message['queue'], + "timeout": packed_message['timeout'], + "receipt_handle": message['ReceiptHandle'] + } + + return pre_process_context + + def _get_task(self, full_task_path): + + task_name = full_task_path.split(".")[-1] + task_path = ".".join(full_task_path.split(".")[:-1]) + task_module = importlib.import_module(task_path) + task = getattr(task_module, task_name) + + return task + + def _process_task(self, pre_process_context): + task = self._get_task(pre_process_context["full_task_path"]) + + # Modify the contexts separately so the original + # context isn't modified by later processing + post_process_context = copy.copy(pre_process_context) + + start_time = time.time() + try: + self._run_hooks("pre_process", pre_process_context) + task(*pre_process_context["args"], **pre_process_context["kwargs"]) + except Exception: + end_time = time.time() + logger.exception( + "Task {} raised error in {:.4f} seconds: with args: {} " + "and kwargs: {}: {}".format( + pre_process_context["full_task_path"], + end_time - start_time, + pre_process_context["args"], + pre_process_context["kwargs"], + traceback.format_exc(), + ) + ) + post_process_context["status"] = "exception" + post_process_context["exception"] = traceback.format_exc() + self._run_hooks("post_process", post_process_context) + return True + else: + end_time = time.time() + self._get_connection().delete_message( + QueueUrl=pre_process_context["queue_url"], + ReceiptHandle=pre_process_context["receipt_handle"] + ) + logger.info( + "Processed task {} in {:.4f} seconds with args: {} " + "and kwargs: {}".format( + pre_process_context["full_task_path"], + end_time - start_time, + pre_process_context["args"], + pre_process_context["kwargs"], + ) + ) + post_process_context["status"] = "success" + self._run_hooks("post_process", post_process_context) + return True + + +class ProcessWorker(BaseProcessWorker): def __init__(self, internal_queue, interval, connection_args=None, *args, **kwargs): @@ -280,7 +285,7 @@ def process_message(self): return self._process_task(pre_process_context) -class SimpleProcessWorker(BaseWorker): +class SimpleProcessWorker(BaseProcessWorker): def __init__(self, queue_url, interval, batchsize, connection_args=None, *args, **kwargs): From c5411e328057659cdf454fc25f0cb19f55901363 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 16:03:26 -0400 Subject: [PATCH 32/49] Fix test_master_shuts_down_busy_process_workers to look at worker processes --- tests/test_manager_worker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index 55371c4..fde8a37 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -418,12 +418,12 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.reader_children[0].pid,)) + target=sleep_and_kill, args=(manager.worker_children[0].pid,)) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, args=(manager.reader_children[0].pid,), + target=sleep_and_kill, args=(manager.worker_children[0].pid,), daemon=True, ) From e81e2dedf11141e2d005824700ab2698fcee1ab4 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 16:03:55 -0400 Subject: [PATCH 33/49] Add tests for SimpleManagerWorker --- tests/test_simple_manager_worker.py | 351 ++++++++++++++++++++++++++++ 1 file changed, 351 insertions(+) create mode 100644 tests/test_simple_manager_worker.py diff --git a/tests/test_simple_manager_worker.py b/tests/test_simple_manager_worker.py new file mode 100644 index 0000000..9664c18 --- /dev/null +++ b/tests/test_simple_manager_worker.py @@ -0,0 +1,351 @@ +import json +import logging +import os +import signal +import time + +import boto3 +from mock import patch, Mock, MagicMock +from moto import mock_sqs + +from pyqs.main import main, _main +from pyqs.worker import SimpleManagerWorker +from tests.utils import ( + MockLoggingHandler, ThreadWithReturnValue2, ThreadWithReturnValue3, +) + + +@mock_sqs +def test_simple_manager_worker_create_proper_children_workers(): + """ + Test simple managing process creates multiple child workers + """ + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="email") + + manager = SimpleManagerWorker( + queue_prefixes=['email'], worker_concurrency=3, interval=2, + batchsize=10, + ) + + len(manager.worker_children).should.equal(3) + + +@mock_sqs +def test_simple_manager_worker_with_queue_prefix(): + """ + Test simple managing process can find queues by prefix + """ + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="email.foobar") + conn.create_queue(QueueName="email.baz") + + manager = SimpleManagerWorker( + queue_prefixes=['email.*'], worker_concurrency=1, interval=1, + batchsize=10, + ) + + len(manager.worker_children).should.equal(2) + children = manager.worker_children + # Pull all the read children and sort by name to make testing easier + sorted_children = sorted(children, key=lambda child: child.queue_url) + + sorted_children[0].queue_url.should.equal( + "https://queue.amazonaws.com/123456789012/email.baz") + sorted_children[1].queue_url.should.equal( + "https://queue.amazonaws.com/123456789012/email.foobar") + + +@mock_sqs +def test_simple_manager_start_and_stop(): + """ + Test simple managing process can start and stop child processes + """ + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="email") + + manager = SimpleManagerWorker( + queue_prefixes=['email'], worker_concurrency=2, interval=1, + batchsize=10, + ) + + len(manager.worker_children).should.equal(2) + + manager.worker_children[0].is_alive().should.equal(False) + manager.worker_children[1].is_alive().should.equal(False) + + manager.start() + + manager.worker_children[0].is_alive().should.equal(True) + manager.worker_children[1].is_alive().should.equal(True) + + manager.stop() + + manager.worker_children[0].is_alive().should.equal(False) + manager.worker_children[1].is_alive().should.equal(False) + + +@patch("pyqs.main.SimpleManagerWorker") +@mock_sqs +def test_main_method(SimpleManagerWorker): + """ + Test creation of simple manager process from _main method + """ + _main(["email1", "email2"], concurrency=2, simple_worker=True) + + SimpleManagerWorker.assert_called_once_with( + ['email1', 'email2'], 2, 1, 10, + region=None, secret_access_key=None, access_key_id=None, + ) + SimpleManagerWorker.return_value.start.assert_called_once_with() + + +@patch("pyqs.main._main") +@patch("pyqs.main.ArgumentParser") +@mock_sqs +def test_real_main_method(ArgumentParser, _main): + """ + Test parsing of arguments from main method + """ + ArgumentParser.return_value.parse_args.return_value = Mock( + concurrency=3, queues=["email1"], interval=1, batchsize=10, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, + ) + main() + + _main.assert_called_once_with( + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=10, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, + ) + + +@mock_sqs +def test_master_spawns_worker_processes(): + """ + Test simple managing process creates child workers + """ + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester") + + # Setup Manager + manager = SimpleManagerWorker(["tester"], 1, 1, 10) + manager.start() + + # Check Workers + len(manager.worker_children).should.equal(1) + + manager.worker_children[0].is_alive().should.be.true + + # Cleanup + manager.stop() + + +@mock_sqs +def test_master_counts_processes(): + """ + Test simple managing process counts child processes + """ + + # Setup Logging + logger = logging.getLogger("pyqs") + del logger.handlers[:] + logger.handlers.append(MockLoggingHandler()) + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester") + + # Setup Manager + manager = SimpleManagerWorker(["tester"], 2, 1, 10) + manager.start() + + # Check Workers + manager.process_counts() + + # Cleanup + manager.stop() + + # Check messages + msg2 = "Worker Processes: 2" + logger.handlers[0].messages['debug'][-1].lower().should.contain( + msg2.lower()) + + +@mock_sqs +def test_master_replaces_worker_processes(): + """ + Test simple managing process replaces worker processes + """ + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester") + + # Setup Manager + manager = SimpleManagerWorker( + queue_prefixes=["tester"], worker_concurrency=1, interval=1, + batchsize=10, + ) + manager.start() + + # Get Worker PID + pid = manager.worker_children[0].pid + + # Kill Worker and wait to replace + manager.worker_children[0].shutdown() + time.sleep(0.1) + manager.replace_workers() + + # Check Replacement + manager.worker_children[0].pid.shouldnt.equal(pid) + + # Cleanup + manager.stop() + + +@mock_sqs +@patch("pyqs.worker.sys") +def test_master_handles_signals(sys): + """ + Test simple managing process handles OS signals + """ + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester") + + # Mock out sys.exit + sys.exit = Mock() + + # Have our inner method send our signal + def process_counts(): + os.kill(os.getpid(), signal.SIGTERM) + + # Setup Manager + manager = SimpleManagerWorker( + queue_prefixes=["tester"], worker_concurrency=1, interval=1, + batchsize=10, + ) + manager.process_counts = process_counts + manager._graceful_shutdown = MagicMock() + + # When we start and trigger a signal + manager.start() + manager.sleep() + + # Then we exit + sys.exit.assert_called_once_with(0) + + +@mock_sqs +def test_master_shuts_down_busy_process_workers(): + """ + Test simple managing process properly cleans up busy Process Workers + """ + # For debugging test + import sys + logger = logging.getLogger("pyqs") + logger.setLevel(logging.DEBUG) + stdout_handler = logging.StreamHandler(sys.stdout) + logger.addHandler(stdout_handler) + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Add Slow tasks + message = json.dumps({ + 'task': 'tests.tasks.sleeper', + 'args': [], + 'kwargs': { + 'message': 5, + }, + }) + + # Fill the queue (we need a lot of messages to trigger the bug) + for _ in range(20): + conn.send_message(QueueUrl=queue_url, MessageBody=message) + + # Create function to watch and kill stuck processes + def sleep_and_kill(pid): + import os + import signal + import time + # This sleep time is long enoug for 100 messages in queue + time.sleep(5) + try: + os.kill(pid, signal.SIGKILL) + except OSError: + # Return that we didn't need to kill the process + return True + else: + # Return that we needed to kill the process + return False + + # Setup Manager + manager = SimpleManagerWorker( + queue_prefixes=["tester"], worker_concurrency=1, interval=0.0, + batchsize=1, + ) + manager.start() + + # Give our processes a moment to start + time.sleep(1) + + # Setup Threading watcher + try: + # Try Python 2 Style + thread = ThreadWithReturnValue2( + target=sleep_and_kill, args=(manager.worker_children[0].pid,)) + thread.daemon = True + except TypeError: + # Use Python 3 Style + thread = ThreadWithReturnValue3( + target=sleep_and_kill, args=(manager.worker_children[0].pid,), + daemon=True, + ) + + thread.start() + + # Stop the Master Process + manager.stop() + + # Check if we had to kill the Reader Worker or it exited gracefully + return_value = thread.join() + if not return_value: + raise Exception("Reader Worker failed to quit!") + + +@mock_sqs +def test_manager_picks_up_new_queues(): + """ + Test that the simple manager will recognize new SQS queues have been added + """ + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + + # Setup Manager + manager = SimpleManagerWorker( + queue_prefixes=["tester"], worker_concurrency=1, interval=1, + batchsize=10, + ) + manager.start() + + # No queues found + len(manager.worker_children).should.equal(0) + + # Create the queue + conn.create_queue(QueueName="tester") + manager.check_for_new_queues() + + # The manager should have seen the new queue was created and add a reader + len(manager.worker_children).should.equal(1) + manager.worker_children[0].queue_url.should.equal( + "https://queue.amazonaws.com/123456789012/tester") + + # Cleanup + manager.stop() From 407dcbee2e7921b7cb21b59962b77d43f557ee64 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 15 Apr 2021 17:17:09 -0400 Subject: [PATCH 34/49] Add tests for different default batchsize for simple worker vs normal worker --- tests/test_manager_worker.py | 27 ++++++++++++++++++++++++--- tests/test_simple_manager_worker.py | 29 +++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index fde8a37..b7b909a 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -109,7 +109,28 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=10, + concurrency=3, queues=["email1"], interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, + ) + main() + + _main.assert_called_once_with( + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, + ) + + +@patch("pyqs.main._main") +@patch("pyqs.main.ArgumentParser") +@mock_sqs +def test_real_main_method_default_batchsize(ArgumentParser, _main): + """ + Test parsing of arguments from main method batch default + """ + ArgumentParser.return_value.parse_args.return_value = Mock( + concurrency=3, queues=["email1"], interval=1, batchsize=None, logging_level="WARN", region='us-east-1', prefetch_multiplier=2, access_key_id=None, secret_access_key=None, simple_worker=False, ) @@ -432,10 +453,10 @@ def sleep_and_kill(pid): # Stop the Master Process manager.stop() - # Check if we had to kill the Reader Worker or it exited gracefully + # Check if we had to kill the Process Worker or it exited gracefully return_value = thread.join() if not return_value: - raise Exception("Reader Worker failed to quit!") + raise Exception("Process Worker failed to quit!") @mock_sqs diff --git a/tests/test_simple_manager_worker.py b/tests/test_simple_manager_worker.py index 9664c18..6d8210d 100644 --- a/tests/test_simple_manager_worker.py +++ b/tests/test_simple_manager_worker.py @@ -108,14 +108,35 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=10, + concurrency=3, queues=["email1"], interval=1, batchsize=5, logging_level="WARN", region='us-east-1', prefetch_multiplier=2, access_key_id=None, secret_access_key=None, simple_worker=True, ) main() _main.assert_called_once_with( - queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=10, + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, + ) + + +@patch("pyqs.main._main") +@patch("pyqs.main.ArgumentParser") +@mock_sqs +def test_real_main_method_default_batchsize(ArgumentParser, _main): + """ + Test parsing of arguments from main method batch default + """ + ArgumentParser.return_value.parse_args.return_value = Mock( + concurrency=3, queues=["email1"], interval=1, batchsize=None, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, + ) + main() + + _main.assert_called_once_with( + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=1, logging_level="WARN", region='us-east-1', prefetch_multiplier=2, access_key_id=None, secret_access_key=None, simple_worker=True, ) @@ -313,10 +334,10 @@ def sleep_and_kill(pid): # Stop the Master Process manager.stop() - # Check if we had to kill the Reader Worker or it exited gracefully + # Check if we had to kill the Process Worker or it exited gracefully return_value = thread.join() if not return_value: - raise Exception("Reader Worker failed to quit!") + raise Exception("Process Worker failed to quit!") @mock_sqs From f62bafed531e9c3a5882e20646dc76031f7e7b41 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 19 Apr 2021 11:20:42 -0400 Subject: [PATCH 35/49] Fix pre-commit issue --- tests/test_simple_worker.py | 649 ++++++++++++++++++++++++++++++++++++ 1 file changed, 649 insertions(+) create mode 100644 tests/test_simple_worker.py diff --git a/tests/test_simple_worker.py b/tests/test_simple_worker.py new file mode 100644 index 0000000..be7f8f2 --- /dev/null +++ b/tests/test_simple_worker.py @@ -0,0 +1,649 @@ +import json +import logging +import time + +import boto3 +from botocore.exceptions import ClientError +from moto import mock_sqs +from mock import patch, Mock +from pyqs.worker import ( + SimpleManagerWorker, BaseProcessWorker, SimpleProcessWorker, + MESSAGE_DOWNLOAD_BATCH_SIZE +) +from pyqs.utils import decode_message +from pyqs.events import register_event +from tests.utils import MockLoggingHandler, clear_events_registry + +BATCHSIZE = 10 +INTERVAL = 0.1 + + +def _create_packed_message(task_name): + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Build the SQS message + message = { + 'Body': json.dumps({ + 'task': task_name, + 'args': [], + 'kwargs': { + 'message': 'Test message', + }, + }), + "ReceiptHandle": "receipt-1234", + } + + packed_message = { + "queue": queue_url, + "message": message, + "start_time": time.time(), + "timeout": 30, + } + + return queue_url, packed_message + + +def _add_messages_to_sqs(task_name, num): + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Build the SQS message + message = json.dumps({ + 'task': task_name, + 'args': [], + 'kwargs': { + 'message': 'Test message', + }, + }) + + for i in range(num): + conn.send_message(QueueUrl=queue_url, MessageBody=message) + + return queue_url + + +@mock_sqs +def test_worker_reads_messages_from_sqs(): + """ + Test simple worker reads from sqs queue + """ + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 1) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + messages = worker.read_message() + + found_message_body = decode_message(messages[0]) + found_message_body.should.equal({ + 'task': 'tests.tasks.index_incrementer', + 'args': [], + 'kwargs': { + 'message': 'Test message', + }, + }) + + +@mock_sqs +def test_worker_throws_error_when_exceeding_max_number_of_messages_for_read(): + """ + Test simple worker reads from sqs queue and throws error when batchsize + greater than 10 + """ + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 1) + + worker = SimpleProcessWorker(queue_url, INTERVAL, 20, parent_id=1) + + error_msg = "Value 20 for parameter MaxNumberOfMessages is invalid" + + try: + worker.read_message() + except ClientError as exc: + str(exc).should.contain(error_msg) + + +@mock_sqs +def test_worker_reads_max_messages_from_sqs(): + """ + Test simple worker reads at maximum 10 message from sqs queue + """ + _add_messages_to_sqs('tests.tasks.index_incrementer', 12) + + manager = SimpleManagerWorker( + queue_prefixes=['tester'], worker_concurrency=1, interval=INTERVAL, + batchsize=20, + ) + worker = manager.worker_children[0] + messages = worker.read_message() + + messages.should.have.length_of(BATCHSIZE) + + +@mock_sqs +def test_worker_fills_internal_queue_from_celery_task(): + """ + Test simple worker reads from sqs queue with celery tasks + """ + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + message = ( + '{"body": "KGRwMApTJ3Rhc2snCnAxClMndGVzdHMudGFza3MuaW5kZXhfa' + 'W5jcmVtZW50ZXInCnAyCnNTJ2Fy\\nZ3MnCnAzCihscDQKc1Mna3dhcmdzJw' + 'pwNQooZHA2ClMnbWVzc2FnZScKcDcKUydUZXN0IG1lc3Nh\\nZ2UyJwpwOAp' + 'zcy4=\\n", "some stuff": "asdfasf"}' + ) + conn.send_message(QueueUrl=queue_url, MessageBody=message) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + messages = worker.read_message() + + found_message_body = decode_message(messages[0]) + found_message_body.should.equal({ + 'task': 'tests.tasks.index_incrementer', + 'args': [], + 'kwargs': { + 'message': 'Test message2', + }, + }) + + +@mock_sqs +def test_worker_processes_tasks_and_logs_correctly(): + """ + Test simple worker processes logs INFO correctly + """ + # Setup logging + logger = logging.getLogger("pyqs") + del logger.handlers[:] + logger.handlers.append(MockLoggingHandler()) + + queue_url, packed_message = _create_packed_message( + 'tests.tasks.index_incrementer' + ) + + # Process message + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + # Check output + kwargs = json.loads(packed_message["message"]['Body'])['kwargs'] + expected_result = ( + u"Processed task tests.tasks.index_incrementer in 0.0000 seconds " + "with args: [] and kwargs: {}".format(kwargs) + ) + logger.handlers[0].messages['info'].should.equal([expected_result]) + + +@mock_sqs +def test_worker_processes_tasks_and_logs_warning_correctly(): + """ + Test simple worker processes logs WARNING correctly + """ + # Setup logging + logger = logging.getLogger("pyqs") + del logger.handlers[:] + logger.handlers.append(MockLoggingHandler()) + + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Build the SQS Message + message = { + 'Body': json.dumps({ + 'task': 'tests.tasks.index_incrementer', + 'args': [], + 'kwargs': { + 'message': 23, + }, + }), + "ReceiptHandle": "receipt-1234", + } + + packed_message = { + "queue": queue_url, + "message": message, + "start_time": time.time(), + "timeout": 30, + } + + # Process message + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + # Check output + kwargs = json.loads(message['Body'])['kwargs'] + msg1 = ( + "Task tests.tasks.index_incrementer raised error in 0.0000 seconds: " + "with args: [] and kwargs: {}: " + "Traceback (most recent call last)".format(kwargs) + ) # noqa + logger.handlers[0].messages['error'][0].lower().should.contain( + msg1.lower()) + msg2 = ( + 'ValueError: Need to be given basestring, ' + 'was given 23' + ) # noqa + logger.handlers[0].messages['error'][0].lower().should.contain( + msg2.lower()) + + +@patch("pyqs.worker.os") +def test_parent_process_death(os): + """ + Test simple worker processes recognize parent process death + """ + os.getppid.return_value = 123 + + worker = BaseProcessWorker(parent_id=1) + worker.parent_is_alive().should.be.false + + +@patch("pyqs.worker.os") +def test_parent_process_alive(os): + """ + Test simple worker processes recognize when parent process is alive + """ + os.getppid.return_value = 1234 + + worker = BaseProcessWorker(parent_id=1234) + worker.parent_is_alive().should.be.true + + +@mock_sqs +@patch("pyqs.worker.os") +def test_read_message_with_parent_process_alive_and_should_not_exit(os): + """ + Test simple worker processes do not exit when parent is alive and shutdown + is not set when reading message + """ + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Setup PPID + os.getppid.return_value = 1 + + # Setup dummy read_message + def read_message(): + raise Exception("Called") + + # When I have a parent process, and shutdown is not set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.read_message = read_message + + # Then read_message() is reached + worker.run.when.called_with().should.throw(Exception, "Called") + + +@mock_sqs +@patch("pyqs.worker.os") +def test_read_message_with_parent_process_alive_and_should_exit(os): + """ + Test simple worker processes exit when parent is alive and shutdown is set + when reading message + """ + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Setup PPID + os.getppid.return_value = 1234 + + # When I have a parent process, and shutdown is set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.read_message = Mock() + worker.shutdown() + + # Then I return from run() + worker.run().should.be.none + + +@mock_sqs +@patch("pyqs.worker.os") +def test_read_message_with_parent_process_dead_and_should_not_exit(os): + """ + Test simple worker processes exit when parent is dead and shutdown is not + set when reading messages + """ + # Setup SQS Queue + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + + # Setup PPID + os.getppid.return_value = 123 + + # When I have no parent process, and shutdown is not set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.read_message = Mock() + + # Then I return from run() + worker.run().should.be.none + + +@mock_sqs +@patch("pyqs.worker.os") +def test_process_message_with_parent_process_alive_and_should_not_exit(os): + """ + Test simple worker processes do not exit when parent is alive and shutdown + is not set when processing message + """ + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 1) + + # Setup PPID + os.getppid.return_value = 1 + + # Setup dummy read_message + def process_message(packed_message): + raise Exception("Called") + + # When I have a parent process, and shutdown is not set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message = process_message + + # Then process_message() is reached + worker.run.when.called_with().should.throw(Exception, "Called") + + +@mock_sqs +@patch("pyqs.worker.os") +def test_process_message_with_parent_process_dead_and_should_not_exit(os): + """ + Test simple worker processes exit when parent is dead and shutdown is not + set when processing message + """ + + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 1) + + # Setup PPID + os.getppid.return_value = 123 + + # When I have a parent process, and shutdown is not set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message = Mock() + + # Then process_message() is reached + worker.run().should.be.none + + +@mock_sqs +@patch("pyqs.worker.os") +def test_process_message_with_parent_process_alive_and_should_exit(os): + """ + Test simple worker processes exit when parent is alive and shutdown is set + set when processing message + """ + + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 1) + + # Setup PPID + os.getppid.return_value = 1 + + # When I have a parent process, and shutdown is not set + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message = Mock() + worker.shutdown() + + # Then process_message() is reached + worker.run().should.be.none + + +@mock_sqs +@patch("pyqs.worker.os") +def test_worker_processes_shuts_down_after_processing_its_max_number_of_msgs( + os): + """ + Test simple worker processes shutdown after processing maximum number + of messages + """ + os.getppid.return_value = 1 + + queue_url = _add_messages_to_sqs('tests.tasks.index_incrementer', 2) + + # When I Process messages + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker._messages_to_process_before_shutdown = 2 + + # Then I return from run() + worker.run().should.be.none + + +@mock_sqs +def test_worker_negative_batch_size(): + """ + Test simple workers with negative batch sizes + """ + BATCHSIZE = -1 + CONCURRENCY = 1 + QUEUE_PREFIX = "tester" + INTERVAL = 0.0 + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester")['QueueUrl'] + + worker = SimpleManagerWorker( + QUEUE_PREFIX, + CONCURRENCY, + INTERVAL, + BATCHSIZE + ) + worker.batchsize.should.equal(1) + + +@mock_sqs +def test_worker_to_large_batch_size(): + """ + Test simple workers with too large of a batch size + """ + BATCHSIZE = 10000 + CONCURRENCY = 1 + QUEUE_PREFIX = "tester" + INTERVAL = 0.0 + conn = boto3.client('sqs', region_name='us-east-1') + conn.create_queue(QueueName="tester")['QueueUrl'] + + worker = SimpleManagerWorker( + QUEUE_PREFIX, + CONCURRENCY, + INTERVAL, + BATCHSIZE + ) + worker.batchsize.should.equal(MESSAGE_DOWNLOAD_BATCH_SIZE) + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_pre_process_callback(): + """ + Test simple worker runs registered callbacks when processing a message + """ + + queue_url, packed_message = _create_packed_message( + 'tests.tasks.index_incrementer' + ) + + # Declare this so it can be checked as a side effect + # to pre_process_with_side_effect + contexts = [] + + def pre_process_with_side_effect(context): + contexts.append(context) + + # When we have a registered pre_process callback + register_event("pre_process", pre_process_with_side_effect) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + pre_process_context = contexts[0] + + # We should run the callback with the task context + pre_process_context['task_name'].should.equal('index_incrementer') + pre_process_context['args'].should.equal([]) + pre_process_context['kwargs'].should.equal({'message': 'Test message'}) + pre_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + pre_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) + pre_process_context['timeout'].should.equal(30) + + assert 'fetch_time' in pre_process_context + assert 'receipt_handle' in pre_process_context + assert 'status' not in pre_process_context + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_post_process_callback_success(): + """ + Test simple worker runs registered callbacks when + processing a message and it succeeds + """ + + queue_url, packed_message = _create_packed_message( + 'tests.tasks.index_incrementer' + ) + + # Declare this so it can be checked as a side effect + # to post_process_with_side_effect + contexts = [] + + def post_process_with_side_effect(context): + contexts.append(context) + + # When we have a registered post_process callback + register_event("post_process", post_process_with_side_effect) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + post_process_context = contexts[0] + + # We should run the callback with the task context + post_process_context['task_name'].should.equal('index_incrementer') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('success') + + assert 'fetch_time' in post_process_context + assert 'receipt_handle' in post_process_context + assert 'exception' not in post_process_context + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_post_process_callback_exception(): + """ + Test simple worker runs registered callbacks when processing + a message and it fails + """ + + queue_url, packed_message = _create_packed_message( + 'tests.tasks.exception_task' + ) + + # Declare this so it can be checked as a side effect + # to post_process_with_side_effect + contexts = [] + + def post_process_with_side_effect(context): + contexts.append(context) + + # When we have a registered post_process callback + register_event("post_process", post_process_with_side_effect) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + post_process_context = contexts[0] + + # We should run the callback with the task context + post_process_context['task_name'].should.equal('exception_task') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal( + 'tests.tasks.exception_task' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('exception') + + assert 'fetch_time' in post_process_context + assert 'receipt_handle' in post_process_context + assert 'exception' in post_process_context + + +@clear_events_registry +@mock_sqs +def test_worker_processes_tasks_with_pre_and_post_process(): + """ + Test worker runs registered callbacks when processing a message + """ + + queue_url, packed_message = _create_packed_message( + 'tests.tasks.index_incrementer' + ) + + # Declare these so they can be checked as a side effect to the callbacks + contexts = [] + + def pre_process_with_side_effect(context): + contexts.append(context) + + def post_process_with_side_effect(context): + contexts.append(context) + + # When we have a registered pre_process and post_process callback + register_event("pre_process", pre_process_with_side_effect) + register_event("post_process", post_process_with_side_effect) + + worker = SimpleProcessWorker(queue_url, INTERVAL, BATCHSIZE, parent_id=1) + worker.process_message(packed_message) + + pre_process_context = contexts[0] + + # We should run the callbacks with the right task contexts + pre_process_context['task_name'].should.equal('index_incrementer') + pre_process_context['args'].should.equal([]) + pre_process_context['kwargs'].should.equal({'message': 'Test message'}) + pre_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + pre_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) + pre_process_context['timeout'].should.equal(30) + + assert 'fetch_time' in pre_process_context + assert 'receipt_handle' in pre_process_context + assert 'status' not in pre_process_context + + post_process_context = contexts[1] + + post_process_context['task_name'].should.equal('index_incrementer') + post_process_context['args'].should.equal([]) + post_process_context['kwargs'].should.equal({'message': 'Test message'}) + post_process_context['full_task_path'].should.equal( + 'tests.tasks.index_incrementer' + ) + post_process_context['queue_url'].should.equal( + 'https://queue.amazonaws.com/123456789012/tester' + ) + post_process_context['timeout'].should.equal(30) + post_process_context['status'].should.equal('success') + + assert 'fetch_time' in post_process_context + assert 'receipt_handle' in post_process_context + assert 'exception' not in post_process_context From b8d63eefd8f8833d82fef67fb29d8298009858aa Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Thu, 13 May 2021 11:46:14 -0400 Subject: [PATCH 36/49] Revert test test_master_shuts_down_busy_process_workers to original --- tests/test_manager_worker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index b7b909a..253d613 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -439,12 +439,12 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.worker_children[0].pid,)) + target=sleep_and_kill, args=(manager.reader_children[0].pid,)) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, args=(manager.worker_children[0].pid,), + target=sleep_and_kill, args=(manager.reader_children[0].pid,), daemon=True, ) @@ -453,10 +453,10 @@ def sleep_and_kill(pid): # Stop the Master Process manager.stop() - # Check if we had to kill the Process Worker or it exited gracefully + # Check if we had to kill the Reader Worker or it exited gracefully return_value = thread.join() if not return_value: - raise Exception("Process Worker failed to quit!") + raise Exception("Reader Worker failed to quit!") @mock_sqs From bcfdcc1aee4c3e5e6f187bd662103e58fa7828d8 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Fri, 14 May 2021 14:15:32 -0400 Subject: [PATCH 37/49] Don't run python 2 travis workflow --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 67e8055..18963b2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ language: python python: - - "2.7" - "3.6" - "3.7" - "3.8" From 01f43cd4520c05b1085a20848452ed3f7f637b99 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Fri, 14 May 2021 15:20:02 -0500 Subject: [PATCH 38/49] 1.0.0 --- CHANGELOG.rst | 5 +++++ pyqs/__init__.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4523348..b64e4f3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,11 @@ Changelog --------- +1.0.0 +~~~~~ +- Drop Py2 support +- Add new SimpleProcessWorker (https://github.com/spulec/PyQS/pull/76) + 0.1.6 ~~~~~ diff --git a/pyqs/__init__.py b/pyqs/__init__.py index 4f424be..5f5d767 100644 --- a/pyqs/__init__.py +++ b/pyqs/__init__.py @@ -1,4 +1,4 @@ from .decorator import task # noqa __title__ = 'pyqs' -__version__ = '0.1.6' +__version__ = '1.0.0' From e8a564ee34dd664c2d638973a5ef418f4b15dda9 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Mon, 17 May 2021 15:01:12 -0400 Subject: [PATCH 39/49] Update README for simple process worker --- README.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.rst b/README.rst index 6f176ac..6441200 100644 --- a/README.rst +++ b/README.rst @@ -100,6 +100,23 @@ messages. $ pyqs send_email --concurrency 10 +Simple Process Worker +~~~~~~~~~~~~~~~~~~~~~ + +To use a simpler version of PyQS that deals with some of the edge cases in the original implementation, pass the ``simple-worker`` flag. + +.. code:: bash + + $ pyqs send_email --simple-worker + +The Simple Process Worker differs in the following way from the original implementation. + +* Does not use an internal queue and removes support for the ``prefetch-multiplier`` flag. This helps simply the mental model required, as messages are not on both the SQS queue and an internal queue. +* When the ``simple-worker`` flag is passed, the default ``batchsize`` is 1 instead of 10. This is configurable. +* Does not check the visibility timeout when reading or processing a message from SQS. + * Allowing the worker to process the message even past its visibility timeout means we solve the problem of never processing a message if ``max_receives=1`` and we incorrectly set a shorter visibility timeout and exceed the visibility timeout. Previously, this message would have ended up in the DLQ, if one was configured, and never actually processed. + * It increases the probability that we process a message more than once, especially if ``batchsize > 1``, but this can be solved by the developer checking if the message has already been processed. + Hooks ~~~~~ From 229ebd209bca3098f5c6bca45ec002fb8b33c33b Mon Sep 17 00:00:00 2001 From: Janusz Mordarski Date: Mon, 30 Aug 2021 18:07:26 +0200 Subject: [PATCH 40/49] Make worker_children class configurable in manager subclasses --- pyqs/worker.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pyqs/worker.py b/pyqs/worker.py index 16c228f..4fc9426 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -439,6 +439,7 @@ def replace_workers(self): class SimpleManagerWorker(BaseManager): + WORKER_CHILDREN_CLASS = SimpleProcessWorker def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, region=None, access_key_id=None, secret_access_key=None): @@ -455,7 +456,7 @@ def _initialize_worker_children(self, number): for queue_url in self.queue_urls: for index in range(number): self.worker_children.append( - SimpleProcessWorker( + self.WORKER_CHILDREN_CLASS( queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, @@ -468,7 +469,7 @@ def check_for_new_queues(self): new_queue_urls = set(queue_urls) - set(self.queue_urls) for new_queue_url in new_queue_urls: logger.info("Found new queue\t{}".format(new_queue_url)) - worker = SimpleProcessWorker( + worker = self.WORKER_CHILDREN_CLASS( new_queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, @@ -497,7 +498,7 @@ def replace_workers(self): "Worker Process {} is no longer responding, " "spawning a new worker.".format(worker.pid)) self.worker_children.pop(index) - worker = SimpleProcessWorker( + worker = self.WORKER_CHILDREN_CLASS( worker.queue_url, self.interval, self.batchsize, connection_args=self.connection_args, parent_id=self._pid, @@ -507,6 +508,7 @@ def replace_workers(self): class ManagerWorker(BaseManager): + WORKER_CHILDREN_CLASS = ProcessWorker def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, prefetch_multiplier=2, region=None, access_key_id=None, @@ -538,7 +540,7 @@ def _initialize_reader_children(self): def _initialize_worker_children(self, number): for index in range(number): self.worker_children.append( - ProcessWorker( + self.WORKER_CHILDREN_CLASS( self.internal_queue, self.interval, connection_args=self.connection_args, parent_id=self._pid, @@ -613,7 +615,7 @@ def _replace_worker_children(self): "Worker Process {} is no longer responding, " "spawning a new worker.".format(worker.pid)) self.worker_children.pop(index) - worker = ProcessWorker( + worker = self.WORKER_CHILDREN_CLASS( self.internal_queue, self.interval, connection_args=self.connection_args, parent_id=self._pid, From b8d5ec1193c7a41d9fb5741f1409fba98ee901ff Mon Sep 17 00:00:00 2001 From: Janusz Mordarski Date: Mon, 30 Aug 2021 18:44:14 +0200 Subject: [PATCH 41/49] Add Github Action for linter and tests --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..63f18e8 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,24 @@ +name: CI + +on: [push] + +jobs: + build_and_test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r development.txt + - name: Lint + run: | + pip install flake8 + python -m flake8 pyqs tests + - name: Test + run: | + make test From d0f0dacc41ec32b5d8e4019f46721a69d84f5e23 Mon Sep 17 00:00:00 2001 From: Janusz Mordarski Date: Mon, 30 Aug 2021 19:49:05 +0200 Subject: [PATCH 42/49] Properly ignore flake8 with noqa --- tests/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index 387d5c9..41342ef 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1 +1 @@ -import sure # flake8: noqa +import sure # noqa: F401 From 958f9d6fa771e445e5473a032d48bad9bf3c2b2d Mon Sep 17 00:00:00 2001 From: Janusz Mordarski Date: Mon, 30 Aug 2021 20:21:31 +0200 Subject: [PATCH 43/49] Ignore VIRTUAL_ENV check in CI --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63f18e8..4cc32cd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,8 +3,10 @@ name: CI on: [push] jobs: - build_and_test: + test: runs-on: ubuntu-latest + env: + VIRTUAL_ENV: ignore steps: - uses: actions/checkout@v2 - name: Set up Python From 85a711f6709b6af71e437a25b08117a5fdf61331 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Tue, 5 Oct 2021 17:51:50 -0400 Subject: [PATCH 44/49] Add message_id to pre_process_context --- pyqs/worker.py | 1 + tests/test_simple_worker.py | 2 ++ tests/test_worker.py | 7 +++++++ 3 files changed, 10 insertions(+) diff --git a/pyqs/worker.py b/pyqs/worker.py index 4fc9426..bed100d 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -163,6 +163,7 @@ def _create_pre_process_context(self, packed_message): full_task_path = message_body['task'] pre_process_context = { + "message_id": message['MessageId'], "task_name": full_task_path.split(".")[-1], "args": message_body['args'], "kwargs": message_body['kwargs'], diff --git a/tests/test_simple_worker.py b/tests/test_simple_worker.py index be7f8f2..1090b76 100644 --- a/tests/test_simple_worker.py +++ b/tests/test_simple_worker.py @@ -33,6 +33,7 @@ def _create_packed_message(task_name): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } packed_message = { @@ -200,6 +201,7 @@ def test_worker_processes_tasks_and_logs_warning_correctly(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } packed_message = { diff --git a/tests/test_worker.py b/tests/test_worker.py index ef4bae3..459a835 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -40,6 +40,7 @@ def _add_message_to_internal_queue(task_name): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to queue internal_queue = Queue() @@ -184,6 +185,7 @@ def test_worker_processes_tasks_from_internal_queue(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to queue @@ -276,6 +278,7 @@ def test_worker_processes_tasks_and_logs_correctly(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to internal queue @@ -326,6 +329,7 @@ def test_worker_processes_tasks_and_logs_warning_correctly(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to internal queue @@ -547,6 +551,7 @@ def test_worker_processes_shuts_down_after_processing_its_max_number_of_msgs( }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to internal queue @@ -612,6 +617,7 @@ def test_worker_processes_discard_tasks_that_exceed_their_visibility_timeout(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to internal queue with timeout of 0 that started long ago @@ -660,6 +666,7 @@ def test_worker_processes_only_incr_processed_counter_if_a_msg_was_processed(): }, }), "ReceiptHandle": "receipt-1234", + "MessageId": "message-id-1", } # Add message to internal queue From 1aeef3b756c3499a34cafc5a58d1fb12d3cf4d22 Mon Sep 17 00:00:00 2001 From: Steve Pulec Date: Thu, 7 Oct 2021 19:29:18 -0500 Subject: [PATCH 45/49] 1.0.1 --- CHANGELOG.rst | 4 ++++ pyqs/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b64e4f3..5850e26 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,10 @@ Changelog --------- +1.0.1 +~~~~~ +- Add MessageId for tracking task executions + 1.0.0 ~~~~~ - Drop Py2 support diff --git a/pyqs/__init__.py b/pyqs/__init__.py index 5f5d767..757f9cd 100644 --- a/pyqs/__init__.py +++ b/pyqs/__init__.py @@ -1,4 +1,4 @@ from .decorator import task # noqa __title__ = 'pyqs' -__version__ = '1.0.0' +__version__ = '1.0.1' From 05c74877410475d2e90d20747f040c6831bc3b14 Mon Sep 17 00:00:00 2001 From: tkhr Date: Mon, 29 Nov 2021 19:48:07 +0900 Subject: [PATCH 46/49] add --endpint-url params add --endpint-url params --- pyqs/main.py | 16 ++++++++++++++-- pyqs/worker.py | 33 +++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/pyqs/main.py b/pyqs/main.py index 7dec331..f2fd0c6 100644 --- a/pyqs/main.py +++ b/pyqs/main.py @@ -94,6 +94,15 @@ def main(): action="store", ) + parser.add_argument( + "--endpoint-url", + dest="endpoint_url", + type=str, + default=None, + help="AWS SQS endpoint url", + action="store", + ) + parser.add_argument( "--interval", dest="interval", @@ -143,7 +152,8 @@ def main(): interval=args.interval, batchsize=_set_batchsize(args), prefetch_multiplier=args.prefetch_multiplier, - simple_worker=args.simple_worker + simple_worker=args.simple_worker, + endpoint_url=args.endpoint_url, ) @@ -156,7 +166,7 @@ def _add_cwd_to_path(): def _main(queue_prefixes, concurrency=5, logging_level="WARN", region=None, access_key_id=None, secret_access_key=None, interval=1, batchsize=DEFAULT_BATCH_SIZE, prefetch_multiplier=2, - simple_worker=False): + simple_worker=False, endpoint_url=None): logging.basicConfig( format="[%(levelname)s]: %(message)s", level=getattr(logging, logging_level), @@ -168,12 +178,14 @@ def _main(queue_prefixes, concurrency=5, logging_level="WARN", queue_prefixes, concurrency, interval, batchsize, region=region, access_key_id=access_key_id, secret_access_key=secret_access_key, + endpoint_url=endpoint_url, ) else: manager = ManagerWorker( queue_prefixes, concurrency, interval, batchsize, prefetch_multiplier=prefetch_multiplier, region=region, access_key_id=access_key_id, secret_access_key=secret_access_key, + endpoint_url=endpoint_url, ) _add_cwd_to_path() diff --git a/pyqs/worker.py b/pyqs/worker.py index bed100d..4e655ed 100644 --- a/pyqs/worker.py +++ b/pyqs/worker.py @@ -27,14 +27,23 @@ logger = logging.getLogger("pyqs") -def get_conn(region=None, access_key_id=None, secret_access_key=None): - if not region: - region = get_aws_region_name() +def get_conn( + region=None, access_key_id=None, secret_access_key=None, endpoint_url=None +): + kwargs = { + "aws_access_key_id": access_key_id, + "aws_secret_access_key": secret_access_key, + "region_name": region, + } + + if endpoint_url: + kwargs["endpoint_url"] = endpoint_url + if not kwargs["region_name"]: + kwargs["region_name"] = get_aws_region_name() return boto3.client( "sqs", - aws_access_key_id=access_key_id, - aws_secret_access_key=secret_access_key, region_name=region, + **kwargs, ) @@ -359,11 +368,12 @@ class BaseManager(object): def __init__(self, queue_prefixes, interval, batchsize, region=None, access_key_id=None, - secret_access_key=None): + secret_access_key=None, endpoint_url=None): self.connection_args = { "region": region, "access_key_id": access_key_id, "secret_access_key": secret_access_key, + "endpoint_url": endpoint_url, } self.interval = interval self.batchsize = batchsize @@ -443,12 +453,14 @@ class SimpleManagerWorker(BaseManager): WORKER_CHILDREN_CLASS = SimpleProcessWorker def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, - region=None, access_key_id=None, secret_access_key=None): + region=None, access_key_id=None, secret_access_key=None, + endpoint_url=None): super(SimpleManagerWorker, self).__init__(queue_prefixes, interval, batchsize, region, access_key_id, - secret_access_key) + secret_access_key, + endpoint_url) self.worker_children = [] self._initialize_worker_children(worker_concurrency) @@ -513,12 +525,13 @@ class ManagerWorker(BaseManager): def __init__(self, queue_prefixes, worker_concurrency, interval, batchsize, prefetch_multiplier=2, region=None, access_key_id=None, - secret_access_key=None): + secret_access_key=None, endpoint_url=None): super(ManagerWorker, self).__init__(queue_prefixes, interval, batchsize, region, access_key_id, - secret_access_key) + secret_access_key, + endpoint_url) self.prefetch_multiplier = prefetch_multiplier self.worker_children = [] From 3f2b2f0f49a4f0cc53cf061f7f5692fce484ea12 Mon Sep 17 00:00:00 2001 From: Filipe Goncalves Date: Mon, 20 Dec 2021 10:58:45 +0000 Subject: [PATCH 47/49] Fix typos in README file --- README.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 6441200..94e954f 100644 --- a/README.rst +++ b/README.rst @@ -78,7 +78,7 @@ To read tasks we need to run PyQS. If the task is already in your $ pyqs email.tasks.send_email -If we want want to run all tasks with a certain prefix. This is based on +If we want to run all tasks with a certain prefix. This is based on Python's `fnmatch `__. .. code:: bash @@ -130,7 +130,7 @@ PyQS has an event registry which can be used to run a function before or after e print({"pre_process": context}) def print_post_process(context): - print({"pre_process": context}) + print({"post_process": context}) events.register_event("pre_process", print_pre_process) events.register_event("post_process", print_post_process) From 8852017620d019540637937b4e423c9006ff77ae Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Tue, 12 Jul 2022 16:58:36 -0400 Subject: [PATCH 48/49] As a starting point fix tests that were broken by PR #81 for endpoint_url --- tests/test_manager_worker.py | 196 +++++++++++++++++++--------- tests/test_simple_manager_worker.py | 154 +++++++++++++++------- 2 files changed, 242 insertions(+), 108 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index 253d613..392b2b8 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -11,7 +11,9 @@ from pyqs.main import main, _main from pyqs.worker import ManagerWorker from tests.utils import ( - MockLoggingHandler, ThreadWithReturnValue2, ThreadWithReturnValue3, + MockLoggingHandler, + ThreadWithReturnValue2, + ThreadWithReturnValue3, ) @@ -20,11 +22,13 @@ def test_manager_worker_create_proper_children_workers(): """ Test managing process creates multiple child workers """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email") manager = ManagerWorker( - queue_prefixes=['email'], worker_concurrency=3, interval=2, + queue_prefixes=["email"], + worker_concurrency=3, + interval=2, batchsize=10, ) @@ -37,12 +41,14 @@ def test_manager_worker_with_queue_prefix(): """ Test managing process can find queues by prefix """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email.foobar") conn.create_queue(QueueName="email.baz") manager = ManagerWorker( - queue_prefixes=['email.*'], worker_concurrency=1, interval=1, + queue_prefixes=["email.*"], + worker_concurrency=1, + interval=1, batchsize=10, ) @@ -52,9 +58,11 @@ def test_manager_worker_with_queue_prefix(): sorted_children = sorted(children, key=lambda child: child.queue_url) sorted_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.baz") + "https://queue.amazonaws.com/123456789012/email.baz" + ) sorted_children[1].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.foobar") + "https://queue.amazonaws.com/123456789012/email.foobar" + ) @mock_sqs @@ -62,11 +70,13 @@ def test_manager_start_and_stop(): """ Test managing process can start and stop child processes """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email") manager = ManagerWorker( - queue_prefixes=['email'], worker_concurrency=2, interval=1, + queue_prefixes=["email"], + worker_concurrency=2, + interval=1, batchsize=10, ) @@ -95,8 +105,15 @@ def test_main_method(ManagerWorker): _main(["email1", "email2"], concurrency=2) ManagerWorker.assert_called_once_with( - ['email1', 'email2'], 2, 1, 10, prefetch_multiplier=2, - region=None, secret_access_key=None, access_key_id=None, + ["email1", "email2"], + 2, + 1, + 10, + prefetch_multiplier=2, + region=None, + secret_access_key=None, + access_key_id=None, + endpoint_url=None, ) ManagerWorker.return_value.start.assert_called_once_with() @@ -109,16 +126,32 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=5, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=False, + concurrency=3, + queues=["email1"], + interval=1, + batchsize=5, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=False, + endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=False, + queue_prefixes=["email1"], + concurrency=3, + interval=1, + batchsize=5, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=False, + endpoint_url=None, ) @@ -130,16 +163,32 @@ def test_real_main_method_default_batchsize(ArgumentParser, _main): Test parsing of arguments from main method batch default """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=None, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=False, + concurrency=3, + queues=["email1"], + interval=1, + batchsize=None, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=False, + endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=10, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=False, + queue_prefixes=["email1"], + concurrency=3, + interval=1, + batchsize=10, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=False, + endpoint_url=None, ) @@ -150,7 +199,7 @@ def test_master_spawns_worker_processes(): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager @@ -176,12 +225,14 @@ def test_master_replaces_reader_processes(): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.start() @@ -213,7 +264,7 @@ def test_master_counts_processes(): logger.handlers.append(MockLoggingHandler()) # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager @@ -228,11 +279,9 @@ def test_master_counts_processes(): # Check messages msg1 = "Reader Processes: 1" - logger.handlers[0].messages['debug'][-2].lower().should.contain( - msg1.lower()) + logger.handlers[0].messages["debug"][-2].lower().should.contain(msg1.lower()) msg2 = "Worker Processes: 2" - logger.handlers[0].messages['debug'][-1].lower().should.contain( - msg2.lower()) + logger.handlers[0].messages["debug"][-1].lower().should.contain(msg2.lower()) @mock_sqs @@ -241,12 +290,14 @@ def test_master_replaces_worker_processes(): Test managing process replaces worker processes """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.start() @@ -274,7 +325,7 @@ def test_master_handles_signals(sys): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Mock out sys.exit @@ -286,7 +337,9 @@ def process_counts(): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.process_counts = process_counts @@ -308,23 +361,26 @@ def test_master_shuts_down_busy_read_workers(): """ # For debugging test import sys + logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') - queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + conn = boto3.client("sqs", region_name="us-east-1") + queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] # Add Slow tasks - message = json.dumps({ - 'task': 'tests.tasks.sleeper', - 'args': [], - 'kwargs': { - 'message': 5, - }, - }) + message = json.dumps( + { + "task": "tests.tasks.sleeper", + "args": [], + "kwargs": { + "message": 5, + }, + } + ) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -335,6 +391,7 @@ def sleep_and_kill(pid): import os import signal import time + # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -348,7 +405,9 @@ def sleep_and_kill(pid): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=0, interval=0.0, + queue_prefixes=["tester"], + worker_concurrency=0, + interval=0.0, batchsize=1, ) manager.start() @@ -360,12 +419,14 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.reader_children[0].pid,)) + target=sleep_and_kill, args=(manager.reader_children[0].pid,) + ) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, args=(manager.reader_children[0].pid,), + target=sleep_and_kill, + args=(manager.reader_children[0].pid,), daemon=True, ) @@ -387,23 +448,26 @@ def test_master_shuts_down_busy_process_workers(): """ # For debugging test import sys + logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') - queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + conn = boto3.client("sqs", region_name="us-east-1") + queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] # Add Slow tasks - message = json.dumps({ - 'task': 'tests.tasks.sleeper', - 'args': [], - 'kwargs': { - 'message': 5, - }, - }) + message = json.dumps( + { + "task": "tests.tasks.sleeper", + "args": [], + "kwargs": { + "message": 5, + }, + } + ) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -414,6 +478,7 @@ def sleep_and_kill(pid): import os import signal import time + # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -427,7 +492,9 @@ def sleep_and_kill(pid): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=0.0, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=0.0, batchsize=1, ) manager.start() @@ -439,12 +506,14 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.reader_children[0].pid,)) + target=sleep_and_kill, args=(manager.reader_children[0].pid,) + ) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, args=(manager.reader_children[0].pid,), + target=sleep_and_kill, + args=(manager.reader_children[0].pid,), daemon=True, ) @@ -466,11 +535,13 @@ def test_manager_picks_up_new_queues(): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.start() @@ -485,7 +556,8 @@ def test_manager_picks_up_new_queues(): # The manager should have seen the new queue was created and add a reader len(manager.reader_children).should.equal(1) manager.reader_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/tester") + "https://queue.amazonaws.com/123456789012/tester" + ) # Cleanup manager.stop() diff --git a/tests/test_simple_manager_worker.py b/tests/test_simple_manager_worker.py index 6d8210d..02fec23 100644 --- a/tests/test_simple_manager_worker.py +++ b/tests/test_simple_manager_worker.py @@ -11,7 +11,9 @@ from pyqs.main import main, _main from pyqs.worker import SimpleManagerWorker from tests.utils import ( - MockLoggingHandler, ThreadWithReturnValue2, ThreadWithReturnValue3, + MockLoggingHandler, + ThreadWithReturnValue2, + ThreadWithReturnValue3, ) @@ -20,11 +22,13 @@ def test_simple_manager_worker_create_proper_children_workers(): """ Test simple managing process creates multiple child workers """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email") manager = SimpleManagerWorker( - queue_prefixes=['email'], worker_concurrency=3, interval=2, + queue_prefixes=["email"], + worker_concurrency=3, + interval=2, batchsize=10, ) @@ -36,12 +40,14 @@ def test_simple_manager_worker_with_queue_prefix(): """ Test simple managing process can find queues by prefix """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email.foobar") conn.create_queue(QueueName="email.baz") manager = SimpleManagerWorker( - queue_prefixes=['email.*'], worker_concurrency=1, interval=1, + queue_prefixes=["email.*"], + worker_concurrency=1, + interval=1, batchsize=10, ) @@ -51,9 +57,11 @@ def test_simple_manager_worker_with_queue_prefix(): sorted_children = sorted(children, key=lambda child: child.queue_url) sorted_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.baz") + "https://queue.amazonaws.com/123456789012/email.baz" + ) sorted_children[1].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.foobar") + "https://queue.amazonaws.com/123456789012/email.foobar" + ) @mock_sqs @@ -61,11 +69,13 @@ def test_simple_manager_start_and_stop(): """ Test simple managing process can start and stop child processes """ - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="email") manager = SimpleManagerWorker( - queue_prefixes=['email'], worker_concurrency=2, interval=1, + queue_prefixes=["email"], + worker_concurrency=2, + interval=1, batchsize=10, ) @@ -94,8 +104,14 @@ def test_main_method(SimpleManagerWorker): _main(["email1", "email2"], concurrency=2, simple_worker=True) SimpleManagerWorker.assert_called_once_with( - ['email1', 'email2'], 2, 1, 10, - region=None, secret_access_key=None, access_key_id=None, + ["email1", "email2"], + 2, + 1, + 10, + region=None, + secret_access_key=None, + access_key_id=None, + endpoint_url=None, ) SimpleManagerWorker.return_value.start.assert_called_once_with() @@ -108,16 +124,32 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=5, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=True, + concurrency=3, + queues=["email1"], + interval=1, + batchsize=5, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=True, + endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=True, + queue_prefixes=["email1"], + concurrency=3, + interval=1, + batchsize=5, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=True, + endpoint_url=None, ) @@ -129,16 +161,32 @@ def test_real_main_method_default_batchsize(ArgumentParser, _main): Test parsing of arguments from main method batch default """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, queues=["email1"], interval=1, batchsize=None, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=True, + concurrency=3, + queues=["email1"], + interval=1, + batchsize=None, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=True, + endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=1, - logging_level="WARN", region='us-east-1', prefetch_multiplier=2, - access_key_id=None, secret_access_key=None, simple_worker=True, + queue_prefixes=["email1"], + concurrency=3, + interval=1, + batchsize=1, + logging_level="WARN", + region="us-east-1", + prefetch_multiplier=2, + access_key_id=None, + secret_access_key=None, + simple_worker=True, + endpoint_url=None, ) @@ -149,7 +197,7 @@ def test_master_spawns_worker_processes(): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager @@ -177,7 +225,7 @@ def test_master_counts_processes(): logger.handlers.append(MockLoggingHandler()) # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager @@ -192,8 +240,7 @@ def test_master_counts_processes(): # Check messages msg2 = "Worker Processes: 2" - logger.handlers[0].messages['debug'][-1].lower().should.contain( - msg2.lower()) + logger.handlers[0].messages["debug"][-1].lower().should.contain(msg2.lower()) @mock_sqs @@ -202,12 +249,14 @@ def test_master_replaces_worker_processes(): Test simple managing process replaces worker processes """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.start() @@ -235,7 +284,7 @@ def test_master_handles_signals(sys): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") conn.create_queue(QueueName="tester") # Mock out sys.exit @@ -247,7 +296,9 @@ def process_counts(): # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.process_counts = process_counts @@ -268,23 +319,26 @@ def test_master_shuts_down_busy_process_workers(): """ # For debugging test import sys + logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') - queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] + conn = boto3.client("sqs", region_name="us-east-1") + queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] # Add Slow tasks - message = json.dumps({ - 'task': 'tests.tasks.sleeper', - 'args': [], - 'kwargs': { - 'message': 5, - }, - }) + message = json.dumps( + { + "task": "tests.tasks.sleeper", + "args": [], + "kwargs": { + "message": 5, + }, + } + ) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -295,6 +349,7 @@ def sleep_and_kill(pid): import os import signal import time + # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -308,7 +363,9 @@ def sleep_and_kill(pid): # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=0.0, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=0.0, batchsize=1, ) manager.start() @@ -320,12 +377,14 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.worker_children[0].pid,)) + target=sleep_and_kill, args=(manager.worker_children[0].pid,) + ) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, args=(manager.worker_children[0].pid,), + target=sleep_and_kill, + args=(manager.worker_children[0].pid,), daemon=True, ) @@ -347,11 +406,13 @@ def test_manager_picks_up_new_queues(): """ # Setup SQS Queue - conn = boto3.client('sqs', region_name='us-east-1') + conn = boto3.client("sqs", region_name="us-east-1") # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], worker_concurrency=1, interval=1, + queue_prefixes=["tester"], + worker_concurrency=1, + interval=1, batchsize=10, ) manager.start() @@ -366,7 +427,8 @@ def test_manager_picks_up_new_queues(): # The manager should have seen the new queue was created and add a reader len(manager.worker_children).should.equal(1) manager.worker_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/tester") + "https://queue.amazonaws.com/123456789012/tester" + ) # Cleanup manager.stop() From 272fd18de049a3fdd123db727f0bc4902d784de2 Mon Sep 17 00:00:00 2001 From: Priyam Sarma Date: Tue, 12 Jul 2022 17:24:42 -0400 Subject: [PATCH 49/49] Removed introduced formatting --- tests/test_manager_worker.py | 191 +++++++++------------------- tests/test_simple_manager_worker.py | 149 +++++++--------------- 2 files changed, 108 insertions(+), 232 deletions(-) diff --git a/tests/test_manager_worker.py b/tests/test_manager_worker.py index 392b2b8..d5a3289 100644 --- a/tests/test_manager_worker.py +++ b/tests/test_manager_worker.py @@ -11,9 +11,7 @@ from pyqs.main import main, _main from pyqs.worker import ManagerWorker from tests.utils import ( - MockLoggingHandler, - ThreadWithReturnValue2, - ThreadWithReturnValue3, + MockLoggingHandler, ThreadWithReturnValue2, ThreadWithReturnValue3, ) @@ -22,13 +20,11 @@ def test_manager_worker_create_proper_children_workers(): """ Test managing process creates multiple child workers """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email") manager = ManagerWorker( - queue_prefixes=["email"], - worker_concurrency=3, - interval=2, + queue_prefixes=['email'], worker_concurrency=3, interval=2, batchsize=10, ) @@ -41,14 +37,12 @@ def test_manager_worker_with_queue_prefix(): """ Test managing process can find queues by prefix """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email.foobar") conn.create_queue(QueueName="email.baz") manager = ManagerWorker( - queue_prefixes=["email.*"], - worker_concurrency=1, - interval=1, + queue_prefixes=['email.*'], worker_concurrency=1, interval=1, batchsize=10, ) @@ -58,11 +52,9 @@ def test_manager_worker_with_queue_prefix(): sorted_children = sorted(children, key=lambda child: child.queue_url) sorted_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.baz" - ) + "https://queue.amazonaws.com/123456789012/email.baz") sorted_children[1].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.foobar" - ) + "https://queue.amazonaws.com/123456789012/email.foobar") @mock_sqs @@ -70,13 +62,11 @@ def test_manager_start_and_stop(): """ Test managing process can start and stop child processes """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email") manager = ManagerWorker( - queue_prefixes=["email"], - worker_concurrency=2, - interval=1, + queue_prefixes=['email'], worker_concurrency=2, interval=1, batchsize=10, ) @@ -105,14 +95,8 @@ def test_main_method(ManagerWorker): _main(["email1", "email2"], concurrency=2) ManagerWorker.assert_called_once_with( - ["email1", "email2"], - 2, - 1, - 10, - prefetch_multiplier=2, - region=None, - secret_access_key=None, - access_key_id=None, + ['email1', 'email2'], 2, 1, 10, prefetch_multiplier=2, + region=None, secret_access_key=None, access_key_id=None, endpoint_url=None, ) ManagerWorker.return_value.start.assert_called_once_with() @@ -126,31 +110,17 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, - queues=["email1"], - interval=1, - batchsize=5, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=False, + concurrency=3, queues=["email1"], interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=["email1"], - concurrency=3, - interval=1, - batchsize=5, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=False, + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, endpoint_url=None, ) @@ -163,31 +133,17 @@ def test_real_main_method_default_batchsize(ArgumentParser, _main): Test parsing of arguments from main method batch default """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, - queues=["email1"], - interval=1, - batchsize=None, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=False, + concurrency=3, queues=["email1"], interval=1, batchsize=None, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=["email1"], - concurrency=3, - interval=1, - batchsize=10, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=False, + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=10, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=False, endpoint_url=None, ) @@ -199,7 +155,7 @@ def test_master_spawns_worker_processes(): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager @@ -225,14 +181,12 @@ def test_master_replaces_reader_processes(): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.start() @@ -264,7 +218,7 @@ def test_master_counts_processes(): logger.handlers.append(MockLoggingHandler()) # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager @@ -279,9 +233,11 @@ def test_master_counts_processes(): # Check messages msg1 = "Reader Processes: 1" - logger.handlers[0].messages["debug"][-2].lower().should.contain(msg1.lower()) + logger.handlers[0].messages['debug'][-2].lower().should.contain( + msg1.lower()) msg2 = "Worker Processes: 2" - logger.handlers[0].messages["debug"][-1].lower().should.contain(msg2.lower()) + logger.handlers[0].messages['debug'][-1].lower().should.contain( + msg2.lower()) @mock_sqs @@ -290,14 +246,12 @@ def test_master_replaces_worker_processes(): Test managing process replaces worker processes """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.start() @@ -325,7 +279,7 @@ def test_master_handles_signals(sys): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Mock out sys.exit @@ -337,9 +291,7 @@ def process_counts(): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.process_counts = process_counts @@ -361,26 +313,23 @@ def test_master_shuts_down_busy_read_workers(): """ # For debugging test import sys - logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") - queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] # Add Slow tasks - message = json.dumps( - { - "task": "tests.tasks.sleeper", - "args": [], - "kwargs": { - "message": 5, - }, - } - ) + message = json.dumps({ + 'task': 'tests.tasks.sleeper', + 'args': [], + 'kwargs': { + 'message': 5, + }, + }) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -391,7 +340,6 @@ def sleep_and_kill(pid): import os import signal import time - # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -405,9 +353,7 @@ def sleep_and_kill(pid): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=0, - interval=0.0, + queue_prefixes=["tester"], worker_concurrency=0, interval=0.0, batchsize=1, ) manager.start() @@ -419,14 +365,12 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.reader_children[0].pid,) - ) + target=sleep_and_kill, args=(manager.reader_children[0].pid,)) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, - args=(manager.reader_children[0].pid,), + target=sleep_and_kill, args=(manager.reader_children[0].pid,), daemon=True, ) @@ -448,26 +392,23 @@ def test_master_shuts_down_busy_process_workers(): """ # For debugging test import sys - logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") - queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] # Add Slow tasks - message = json.dumps( - { - "task": "tests.tasks.sleeper", - "args": [], - "kwargs": { - "message": 5, - }, - } - ) + message = json.dumps({ + 'task': 'tests.tasks.sleeper', + 'args': [], + 'kwargs': { + 'message': 5, + }, + }) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -478,7 +419,6 @@ def sleep_and_kill(pid): import os import signal import time - # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -492,9 +432,7 @@ def sleep_and_kill(pid): # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=0.0, + queue_prefixes=["tester"], worker_concurrency=1, interval=0.0, batchsize=1, ) manager.start() @@ -506,14 +444,12 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.reader_children[0].pid,) - ) + target=sleep_and_kill, args=(manager.reader_children[0].pid,)) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, - args=(manager.reader_children[0].pid,), + target=sleep_and_kill, args=(manager.reader_children[0].pid,), daemon=True, ) @@ -535,13 +471,11 @@ def test_manager_picks_up_new_queues(): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') # Setup Manager manager = ManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.start() @@ -556,8 +490,7 @@ def test_manager_picks_up_new_queues(): # The manager should have seen the new queue was created and add a reader len(manager.reader_children).should.equal(1) manager.reader_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/tester" - ) + "https://queue.amazonaws.com/123456789012/tester") # Cleanup manager.stop() diff --git a/tests/test_simple_manager_worker.py b/tests/test_simple_manager_worker.py index 02fec23..012faec 100644 --- a/tests/test_simple_manager_worker.py +++ b/tests/test_simple_manager_worker.py @@ -11,9 +11,7 @@ from pyqs.main import main, _main from pyqs.worker import SimpleManagerWorker from tests.utils import ( - MockLoggingHandler, - ThreadWithReturnValue2, - ThreadWithReturnValue3, + MockLoggingHandler, ThreadWithReturnValue2, ThreadWithReturnValue3, ) @@ -22,13 +20,11 @@ def test_simple_manager_worker_create_proper_children_workers(): """ Test simple managing process creates multiple child workers """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email") manager = SimpleManagerWorker( - queue_prefixes=["email"], - worker_concurrency=3, - interval=2, + queue_prefixes=['email'], worker_concurrency=3, interval=2, batchsize=10, ) @@ -40,14 +36,12 @@ def test_simple_manager_worker_with_queue_prefix(): """ Test simple managing process can find queues by prefix """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email.foobar") conn.create_queue(QueueName="email.baz") manager = SimpleManagerWorker( - queue_prefixes=["email.*"], - worker_concurrency=1, - interval=1, + queue_prefixes=['email.*'], worker_concurrency=1, interval=1, batchsize=10, ) @@ -57,11 +51,9 @@ def test_simple_manager_worker_with_queue_prefix(): sorted_children = sorted(children, key=lambda child: child.queue_url) sorted_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.baz" - ) + "https://queue.amazonaws.com/123456789012/email.baz") sorted_children[1].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/email.foobar" - ) + "https://queue.amazonaws.com/123456789012/email.foobar") @mock_sqs @@ -69,13 +61,11 @@ def test_simple_manager_start_and_stop(): """ Test simple managing process can start and stop child processes """ - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="email") manager = SimpleManagerWorker( - queue_prefixes=["email"], - worker_concurrency=2, - interval=1, + queue_prefixes=['email'], worker_concurrency=2, interval=1, batchsize=10, ) @@ -104,13 +94,8 @@ def test_main_method(SimpleManagerWorker): _main(["email1", "email2"], concurrency=2, simple_worker=True) SimpleManagerWorker.assert_called_once_with( - ["email1", "email2"], - 2, - 1, - 10, - region=None, - secret_access_key=None, - access_key_id=None, + ['email1', 'email2'], 2, 1, 10, + region=None, secret_access_key=None, access_key_id=None, endpoint_url=None, ) SimpleManagerWorker.return_value.start.assert_called_once_with() @@ -124,31 +109,17 @@ def test_real_main_method(ArgumentParser, _main): Test parsing of arguments from main method """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, - queues=["email1"], - interval=1, - batchsize=5, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=True, + concurrency=3, queues=["email1"], interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=["email1"], - concurrency=3, - interval=1, - batchsize=5, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=True, + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=5, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, endpoint_url=None, ) @@ -161,31 +132,17 @@ def test_real_main_method_default_batchsize(ArgumentParser, _main): Test parsing of arguments from main method batch default """ ArgumentParser.return_value.parse_args.return_value = Mock( - concurrency=3, - queues=["email1"], - interval=1, - batchsize=None, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=True, + concurrency=3, queues=["email1"], interval=1, batchsize=None, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, endpoint_url=None, ) main() _main.assert_called_once_with( - queue_prefixes=["email1"], - concurrency=3, - interval=1, - batchsize=1, - logging_level="WARN", - region="us-east-1", - prefetch_multiplier=2, - access_key_id=None, - secret_access_key=None, - simple_worker=True, + queue_prefixes=['email1'], concurrency=3, interval=1, batchsize=1, + logging_level="WARN", region='us-east-1', prefetch_multiplier=2, + access_key_id=None, secret_access_key=None, simple_worker=True, endpoint_url=None, ) @@ -197,7 +154,7 @@ def test_master_spawns_worker_processes(): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager @@ -225,7 +182,7 @@ def test_master_counts_processes(): logger.handlers.append(MockLoggingHandler()) # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager @@ -240,7 +197,8 @@ def test_master_counts_processes(): # Check messages msg2 = "Worker Processes: 2" - logger.handlers[0].messages["debug"][-1].lower().should.contain(msg2.lower()) + logger.handlers[0].messages['debug'][-1].lower().should.contain( + msg2.lower()) @mock_sqs @@ -249,14 +207,12 @@ def test_master_replaces_worker_processes(): Test simple managing process replaces worker processes """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.start() @@ -284,7 +240,7 @@ def test_master_handles_signals(sys): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') conn.create_queue(QueueName="tester") # Mock out sys.exit @@ -296,9 +252,7 @@ def process_counts(): # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.process_counts = process_counts @@ -319,26 +273,23 @@ def test_master_shuts_down_busy_process_workers(): """ # For debugging test import sys - logger = logging.getLogger("pyqs") logger.setLevel(logging.DEBUG) stdout_handler = logging.StreamHandler(sys.stdout) logger.addHandler(stdout_handler) # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") - queue_url = conn.create_queue(QueueName="tester")["QueueUrl"] + conn = boto3.client('sqs', region_name='us-east-1') + queue_url = conn.create_queue(QueueName="tester")['QueueUrl'] # Add Slow tasks - message = json.dumps( - { - "task": "tests.tasks.sleeper", - "args": [], - "kwargs": { - "message": 5, - }, - } - ) + message = json.dumps({ + 'task': 'tests.tasks.sleeper', + 'args': [], + 'kwargs': { + 'message': 5, + }, + }) # Fill the queue (we need a lot of messages to trigger the bug) for _ in range(20): @@ -349,7 +300,6 @@ def sleep_and_kill(pid): import os import signal import time - # This sleep time is long enoug for 100 messages in queue time.sleep(5) try: @@ -363,9 +313,7 @@ def sleep_and_kill(pid): # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=0.0, + queue_prefixes=["tester"], worker_concurrency=1, interval=0.0, batchsize=1, ) manager.start() @@ -377,14 +325,12 @@ def sleep_and_kill(pid): try: # Try Python 2 Style thread = ThreadWithReturnValue2( - target=sleep_and_kill, args=(manager.worker_children[0].pid,) - ) + target=sleep_and_kill, args=(manager.worker_children[0].pid,)) thread.daemon = True except TypeError: # Use Python 3 Style thread = ThreadWithReturnValue3( - target=sleep_and_kill, - args=(manager.worker_children[0].pid,), + target=sleep_and_kill, args=(manager.worker_children[0].pid,), daemon=True, ) @@ -406,13 +352,11 @@ def test_manager_picks_up_new_queues(): """ # Setup SQS Queue - conn = boto3.client("sqs", region_name="us-east-1") + conn = boto3.client('sqs', region_name='us-east-1') # Setup Manager manager = SimpleManagerWorker( - queue_prefixes=["tester"], - worker_concurrency=1, - interval=1, + queue_prefixes=["tester"], worker_concurrency=1, interval=1, batchsize=10, ) manager.start() @@ -427,8 +371,7 @@ def test_manager_picks_up_new_queues(): # The manager should have seen the new queue was created and add a reader len(manager.worker_children).should.equal(1) manager.worker_children[0].queue_url.should.equal( - "https://queue.amazonaws.com/123456789012/tester" - ) + "https://queue.amazonaws.com/123456789012/tester") # Cleanup manager.stop()