-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/add pre and post process hooks #71
Feature/add pre and post process hooks #71
Conversation
d710205
to
750fb45
Compare
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. |
I like this! If we can add some tests, I'll be happy to merge. |
779ddcd
to
f557af1
Compare
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. |
95d540b
to
6c4204d
Compare
Bumping this @spulec — if you're not sure about merging yet, is there a suggested way to test a beta version from a branch? |
6c4204d
to
d2d19a7
Compare
eaef60f
to
0d456c0
Compare
Thanks @jimjshields ! |
This is a proof of concept for #70
I haven't figured out how best to test this, it might require a more end-to-end test.
Works like: