Skip to content
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

Each queue should be able to have its custom_exception or None. #215

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Installation
'DB': 0,
'PASSWORD': 'some-password',
'DEFAULT_TIMEOUT': 360,
'EXCEPTION_HANDLERS': ['path.to.my.handler'], # If you need custom exception handlers in your queue. If not just delete this.
},
'high': {
'URL': os.getenv('REDISTOGO_URL', 'redis://localhost:6379/0'), # If you're on Heroku
Expand All @@ -58,7 +59,6 @@ Installation
}
}

RQ_EXCEPTION_HANDLERS = ['path.to.my.handler'] # If you need custom exception handlers

* Include ``django_rq.urls`` in your ``urls.py``:

Expand Down
3 changes: 2 additions & 1 deletion django_rq/management/commands/rqworker.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ def handle(self, *args, **options):
queues,
connection=queues[0].connection,
name=options['name'],
exception_handlers=get_exception_handlers() or None,
exception_handlers=get_exception_handlers(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

queues[0].exception_handlers) or None,
default_worker_ttl=options['worker_ttl']
)

Expand Down
7 changes: 6 additions & 1 deletion django_rq/queues.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class DjangoRQ(Queue):
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to store exception_handlers as property in DjangoRQ

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! ^^

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simply write a test for get_queue('default') and ensure that queue.exception_handlers contains the exception handler you specified in settings.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the test should be for get_worker() because only workers have exception_handlers. Queue objects have no exception_handlers in RQ.


super(DjangoRQ, self).__init__(*args, **kwargs)

Expand Down Expand Up @@ -138,9 +139,13 @@ def get_queue(name='default', default_timeout=None, async=None,
default_timeout = QUEUES[name].get('DEFAULT_TIMEOUT')
if queue_class is None:
queue_class = get_queue_class(QUEUES[name])

exception_handlers = QUEUES[name].get('EXCEPTION_HANDLERS', None)

return queue_class(name, default_timeout=default_timeout,
connection=get_connection(name), async=async,
autocommit=autocommit)
autocommit=autocommit,
exception_handlers=exception_handlers)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also import exception_handlers before passing it into __init__

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fchevitarese reminder to update this PR when you're free :)



def get_queue_by_index(index):
Expand Down
3 changes: 3 additions & 0 deletions django_rq/test_exception_handler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

def my_custom_exception(job, exc_type, exc_value, traceback):
return False
2 changes: 1 addition & 1 deletion django_rq/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
'HOST': REDIS_HOST,
'PORT': 6379,
'DB': 0,
'DEFAULT_TIMEOUT': 500
'DEFAULT_TIMEOUT': 500,
},
'test': {
'HOST': REDIS_HOST,
Expand Down
35 changes: 25 additions & 10 deletions django_rq/workers.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,40 @@
from rq import Worker
from rq.utils import import_attribute

from .queues import get_queues
from .settings import EXCEPTION_HANDLERS
from .queues import get_queues


def get_exception_handlers():
"""
Custom exception handlers could be defined in settings.py:
RQ = {
'EXCEPTION_HANDLERS': ['path.to.handler'],
def get_exception_handlers(exception_handlers):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to change the function signature to get_exception_handlers(queue_name)

"""Custom exception handler defined in QUEUE settings:
RQ_QUEUES = {
'default': {
'HOST': 'localhost',
'PORT': 6379,
'DB': 0,
'PASSWORD': '',
'DEFAULT_TIMEOUT': 360,
'EXCEPTION_HANDLERS': [
'test_exception_handler.my_custom_exception'
],
}
}
"""
return [import_attribute(path) for path in EXCEPTION_HANDLERS]

if exception_handlers:
return [import_attribute(exception_handler) for exception_handler in
exception_handlers]
else:
return [import_attribute(path) for path in EXCEPTION_HANDLERS]


def get_worker(*queue_names):
"""
Returns a RQ worker for all queues or specified ones.
"""
queues = get_queues(*queue_names)
return Worker(queues,
connection=queues[0].connection,
exception_handlers=get_exception_handlers() or None)
return Worker(
queues,
connection=queues[0].connection,
exception_handlers=get_exception_handlers(queues[0].exception_handlers)
)