Feature/download progress#209
Conversation
| if exc_value is not None: | ||
| formatted_exception = u''.join(traceback.format_exception(exc_type, exc_value, exc_tb)) | ||
| failed_message = u'{0}<br/><pre>{1}</pre>'\ | ||
| failed_message = u'{0}<br/><pre>{1}</pre>' \ |
| engine.execute(ids) | ||
| except: | ||
| caught_exception = sys.exc_info()[0] | ||
| log.error("An error has occurred during execute", exception=str(caught_exception)) |
There was a problem hiding this comment.
This doesn't provide stacktrace info, may be log.error already support it?
There was a problem hiding this comment.
Well, it doesn't, as sys.exc_info()[0] returns exception type only (ref to https://docs.python.org/2/library/sys.html#sys.exc_info)
Also, it's worth to mention, that structlog has it's own event and render for handling exceptions (take a look at http://structlog.readthedocs.io/en/stable/api.html#structlog.stdlib.BoundLogger.exception)
Btw, may I ask (as not that experienced python developer) - why sys.exec_info instead of traceback.print_exception ? It's not that safe to assign it to a local variable (from docs and SO discussions).
There was a problem hiding this comment.
We store it for next usage, not for printing right now.
And from docs:
Process event and call
logging.Logger.error()with the result, after settingexc_infotoTrue.
That was what I'm asked for, thx
| list(self.clients.values())[0] if len(self.clients) > 0 else None) | ||
|
|
||
| def set_default(self, name): | ||
|
|
| if torrent_settings.download_dir is not None: | ||
| torrent_settings_dict['download_dir'] = torrent_settings.download_dir | ||
| client.add_torrent(base64.b64encode(torrent).decode('utf-8'), **torrent_settings_dict) | ||
| return True |
There was a problem hiding this comment.
I think we can remove such return True because right now if there are no exception, than it is ok :)
|
|
||
| torrent_hash = 'SomeRandomHashMockString' | ||
| self.assertFalse(plugin.find_torrent(torrent_hash)) | ||
| assert plugin.find_torrent(torrent_hash) is False |
There was a problem hiding this comment.
I didn't know it is supported syntax 👍
| @@ -112,53 +103,40 @@ def add_torrent(self, torrent, torrent_settings): | |||
| client = self.check_connection() | |||
| if not client: | |||
| return False | |||
There was a problem hiding this comment.
We should throw exception instead in check connection directly
# Conflicts: # monitorrent/plugins/clients/deluge.py # monitorrent/plugins/clients/qbittorrent.py # monitorrent/plugins/clients/transmission.py # monitorrent/plugins/clients/utorrent.py # monitorrent/rest/notifiers.py # tests/plugins/clients/test_qbittorrent.py # tests/plugins/clients/test_transmission_plugin.py # tests/plugins/clients/test_utorrent_plugin.py # tests/rest/test_api_clients.py # tests/rest/test_api_execute.py # tests/rest/test_api_notifiers.py
Codecov Report
@@ Coverage Diff @@
## develop #209 +/- ##
=======================================
Coverage 100% 100%
=======================================
Files 55 55
Lines 4468 4631 +163
=======================================
+ Hits 4468 4631 +163
Continue to review full report at Codecov.
|
1 similar comment
|
Updated the pull request |
aa9da35 to
67a7ebc
Compare
ab4c80e to
e42a621
Compare
Also added logs