Skip to content

Commit

Permalink
Meaningful error messages for filesystem backend (celery#5289)
Browse files Browse the repository at this point in the history
* Update filesystem.py

* refactor error messages into constants

* add tests to confirm meaningful error messages on improperly configured paths
  • Loading branch information
larsrinn authored and auvipy committed Feb 21, 2019
1 parent 8bec9eb commit 9bde585
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
12 changes: 9 additions & 3 deletions celery/backends/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

default_encoding = locale.getpreferredencoding(False)

E_NO_PATH_SET = 'You need to configure a path for the file-system backend'
E_PATH_NON_CONFORMING_SCHEME = (
'A path for the file-system backend should conform to the file URI scheme'
)
E_PATH_INVALID = """\
The configured path for the file-system backend does not
work correctly, please make sure that it exists and has
Expand Down Expand Up @@ -56,10 +60,12 @@ def __init__(self, url=None, open=open, unlink=os.unlink, sep=os.sep,

def _find_path(self, url):
if not url:
raise ImproperlyConfigured(
'You need to configure a path for the File-system backend')
if url is not None and url.startswith('file:///'):
raise ImproperlyConfigured(E_NO_PATH_SET)
if url.startswith('file:///'):
return url[7:]
if url.startswith('file://localhost/'):
return url[16:]
raise ImproperlyConfigured(E_PATH_NON_CONFORMING_SCHEME)

def _do_directory_test(self, key):
try:
Expand Down
24 changes: 21 additions & 3 deletions t/unit/backends/test_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from case import skip

from celery import states, uuid
from celery.backends import filesystem
from celery.backends.filesystem import FilesystemBackend
from celery.exceptions import ImproperlyConfigured

Expand All @@ -28,9 +29,26 @@ def test_a_path_in_url(self):
tb = FilesystemBackend(app=self.app, url=self.url)
assert tb.path == self.path

def test_path_is_incorrect(self):
with pytest.raises(ImproperlyConfigured):
FilesystemBackend(app=self.app, url=self.url + '-incorrect')
@pytest.mark.parametrize("url,expected_error_message", [
('file:///non-existing', filesystem.E_PATH_INVALID),
('url://non-conforming', filesystem.E_PATH_NON_CONFORMING_SCHEME),
(None, filesystem.E_NO_PATH_SET)
])
def test_raises_meaningful_errors_for_invalid_urls(
self,
url,
expected_error_message
):
with pytest.raises(
ImproperlyConfigured,
match=expected_error_message
):
FilesystemBackend(app=self.app, url=url)

def test_localhost_is_removed_from_url(self):
url = 'file://localhost' + self.directory
tb = FilesystemBackend(app=self.app, url=url)
assert tb.path == self.path

def test_missing_task_is_PENDING(self):
tb = FilesystemBackend(app=self.app, url=self.url)
Expand Down

0 comments on commit 9bde585

Please sign in to comment.