spulec / PyQS

Python task-queues for Amazon SQS
MIT License
174 stars 36 forks source link

Feature/add pre and post process hooks #71

Closed jimjshields closed 4 years ago

jimjshields commented 4 years ago

This is a proof of concept for https://github.com/spulec/PyQS/issues/70

I haven't figured out how best to test this, it might require a more end-to-end test.

Works like:

from pyqs import task

def print_pre_process(context):
    print({"pre_process": context})

def print_post_process(context):
    print({"pre_process": context})

task.set_hooks({"pre_process": print_pre_process, "post_process": print_post_process})

@task(queue="my_queue")
def send_email(subject):
    pass
pyqs email.tasks.send_email
# {"pre_process": {"task_name": "send_email", ...}}
# {"post_process": {"task_name": "send_email", "status": "success", ...}}
coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.2%) to 97.872% when pulling 0d456c0f0128dc50f86af17e820a2ea4227dfb53 on jimjshields:feature/add-pre-and-post-process-hooks into 2f1b02d42260ea358236ac134923aa4facd7021f on spulec:master.

jimjshields commented 4 years ago

Changed the suggested API after discussing with @spulec (inspiration from botocore's event registry):

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
pyqs email.tasks.send_email
# {"pre_process": {"task_name": "send_email", ...}}
# {"post_process": {"task_name": "send_email", "status": "success", ...}}

Still no tests, but what do you think about this API @spulec? If it's good, I can work on tests.

spulec commented 4 years ago

I like this!

If we can add some tests, I'll be happy to merge.

jimjshields commented 4 years ago

Refactored & renamed a few things, and added unit / functional tests.

@spulec do you think there are uncovered cases? I know there aren't any end-to-end tests, but I don't think it's necessary to come up with a way to do those for this change.

jimjshields commented 4 years ago

Bumping this @spulec — if you're not sure about merging yet, is there a suggested way to test a beta version from a branch?

spulec commented 4 years ago

Thanks @jimjshields !