Each queue should be able to have its custom_exception or None. - #215
Each queue should be able to have its custom_exception or None.#215fchevitarese wants to merge 6 commits into
Conversation
|
How can i run tests in my machine? |
… of a test. I cannot send a failed job to failed
|
I agree that this is a good idea. However, I'd like to keep it backwards compatible if we can. Could you please change your PR to make |
|
Will do ;) |
|
i'm sorry, i was really busy those days ;) Take a look and see if it fits ^^ |
| """ | ||
| return [import_attribute(path) for path in EXCEPTION_HANDLERS] | ||
| try: | ||
| from django.settings import RQ_EXCEPTION_HANDLERS |
There was a problem hiding this comment.
RQ_EXCEPTION_HANDLERS should only be a fallback. So we only try to import it if exception_handlers is None.
There was a problem hiding this comment.
Ok, i will rewrite asap ;)
|
Done, could you take a look if it's ok? Cheers! |
| Custom exception handlers could be defined in settings.py: | ||
| RQ = { | ||
| 'EXCEPTION_HANDLERS': ['path.to.handler'], | ||
| def get_exception_handlers(exception_handlers): |
There was a problem hiding this comment.
I think it's better to change the function signature to get_exception_handlers(queue_name)
| def __init__(self, *args, **kwargs): | ||
| autocommit = kwargs.pop('autocommit', None) | ||
| self._autocommit = get_commit_mode() if autocommit is None else autocommit | ||
| self.exception_handlers = kwargs.pop('exception_handlers', None) |
There was a problem hiding this comment.
There's no need to store exception_handlers as property in DjangoRQ
There was a problem hiding this comment.
Ok. About the failing tests, idk how to fix it because, once the exception handler comes in, i couldn't find how to put them in failed again if they fail.
That's the problem with the test. The failed job never go to failed after a exception_handlers takes in. :(
I would like some help about it , if possible.
Thanks! ^^
There was a problem hiding this comment.
You can simply write a test for get_queue('default') and ensure that queue.exception_handlers contains the exception handler you specified in settings.
There was a problem hiding this comment.
Sorry, the test should be for get_worker() because only workers have exception_handlers. Queue objects have no exception_handlers in RQ.
|
@fchevitarese could you also fix the failing tests? |
…s by checking if have a custom exception, otherwise, it start a Worker without the parameter.
|
@selwin , the only way i found to fix the testing was to check if exception_handler exists, and if not, start a Worker without the parameter. I'm not sure why, and i couldn't find a way to fix this problem in other way. About the property "self.exception_handlers = kwargs.pop('exception_handlers', None)" i removed, but started to face a lot of errors, so i put it back. Hope it's all good! |
| connection=queues[0].connection, | ||
| name=options['name'], | ||
| exception_handlers=get_exception_handlers() or None, | ||
| exception_handlers=get_exception_handlers( |
There was a problem hiding this comment.
I don't think this is right. The function is defined as get_exception_handlers(queue_name) but you're passing in queue.exception_handlers here.
| connection=get_connection(name), async=async, | ||
| autocommit=autocommit) | ||
| autocommit=autocommit, | ||
| exception_handlers=exception_handlers) |
There was a problem hiding this comment.
You should also import exception_handlers before passing it into __init__
There was a problem hiding this comment.
@fchevitarese reminder to update this PR when you're free :)
|
Hi! I'm sorry I've been very busy. Will do when I can
Em 22/07/2017 21:54, "Selwin Ong" <notifications@github.com> escreveu:
… ***@***.**** commented on this pull request.
------------------------------
In django_rq/queues.py
<#215 (comment)>:
> return queue_class(name, default_timeout=default_timeout,
connection=get_connection(name), async=async,
- autocommit=autocommit)
+ autocommit=autocommit,
+ exception_handlers=exception_handlers)
@fchevitarese <https://github.com/fchevitarese> reminder to update this
PR when you're free :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#215 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AACAx9KbgEC1fsa7LejrT_heZOToLfa0ks5sQpmogaJpZM4LxhAD>
.
|
2 participants
Each queue should be able to have its custom_exception or None.
Make a global custom_exception may lead us to a non desired behavior on other queues.