refactor catch-all exceptions #545
Conversation
The flake8 checks were failing for py3.5 due to a couple of catch-all exceptions when importing tornado and gevent libraries. Refactored to catch the `ImportError`
|
I am actually of the opinion to remove catching the exceptions at all, since we're now essentially converting the |
| @@ -97,7 +97,7 @@ def run(self, port=None, server=None, debug=None, host=None, **options): # prag | |||
| import tornado.wsgi | |||
| import tornado.httpserver | |||
| import tornado.ioloop | |||
| except: | |||
| except ImportError: | |||
| raise Exception('tornado library not installed') | |||
There was a problem hiding this comment.
I guess for this purpose raising RuntimeError(...) would be more appropriate
There was a problem hiding this comment.
At least it is more specific than an Exception, I can change it.
However regardless of which exception is raised, the application terminates. Given that the server is a user provided parameter, isn't letting the ImportError bubble up to the cli the clearest way of informing the user that the library they want to use is not installed?
There was a problem hiding this comment.
Not always. ImportError may also indicate that PYTHONPATH is wrong. I think the biggest value here is the error description or the message the user sees and it's quite clear.
There was a problem hiding this comment.
Perhaps my wording was poor, but that is exactly what I meant, the library cannot be found.
>>> import tornado
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'tornado'
vs
>>> raise RuntimeError("Module 'tornado' cannot be found")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
RuntimeError: Module 'tornado' cannot be found
I think we're splitting hairs now 😄
There was a problem hiding this comment.
I just pointed to the fact the there are at least two reasons why module can't be found: it's not installed or it's installed but PYTONPATH is wrong and should we do pip install or check our environment variables. So it's kind of make sense to be precise here when it comes to debugging.
To me, giving the message that it's not installed (which is more probable) is better than just the module is not found, purely for debug reasons
|
👍 |
1 similar comment
|
👍 |
The flake8 checks were failing for py3.5 due to a couple of catch-all
exceptions when importing tornado and gevent libraries. Refactored to
catch the
ImportError.