From f98d0157b830d183a63a95981662590f084871da Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Tue, 1 Feb 2022 21:51:08 -0500 Subject: [PATCH 01/15] Add maybe_coroutine decorator to allow async/await syntax for twisted --- deluge/decorators.py | 54 +++++++ deluge/tests/test_maybe_coroutine.py | 213 +++++++++++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 deluge/tests/test_maybe_coroutine.py diff --git a/deluge/decorators.py b/deluge/decorators.py index 8ca8b805d0..b5a249de31 100644 --- a/deluge/decorators.py +++ b/deluge/decorators.py @@ -10,6 +10,9 @@ import re import warnings from functools import wraps +from typing import Any, Callable, Coroutine, TypeVar + +from twisted.internet import defer def proxy(proxy_func): @@ -159,3 +162,54 @@ def depr_func(*args, **kwargs): return func(*args, **kwargs) return depr_func + + +class CoroutineDeferred(defer.Deferred): + """Wraps a coroutine in a Deferred. + It will dynamically pass through the underlying coroutine without wrapping where apporpriate.""" + + def __init__(self, coro: Coroutine): + # Delay this import to make sure a reactor was installed first + from twisted.internet import reactor + + super().__init__() + self.coro = coro + self.awaited = None + self.activate_deferred = reactor.callLater(0, self.activate) + + def __await__(self): + if self.awaited in [None, True]: + self.awaited = True + return self.coro.__await__() + # Already in deferred mode + return super().__await__() + + def activate(self): + """If the result wasn't awaited before the next context switch, we turn it into a deferred.""" + if self.awaited is None: + self.awaited = False + d = defer.Deferred.fromCoroutine(self.coro) + d.chainDeferred(self) + + def addCallbacks(self, *args, **kwargs): # noqa: N802 + assert not self.awaited, 'Cannot add callbacks to an already awaited coroutine.' + self.activate() + return super().addCallbacks(*args, **kwargs) + + +_RetT = TypeVar('_RetT') + + +def maybe_coroutine( + f: Callable[..., Coroutine[Any, Any, _RetT]] +) -> Callable[..., defer.Deferred[_RetT]]: + """Wraps a coroutine function to make it usable as a normal function that returns a Deferred.""" + + @wraps(f) + def wrapper(*args, **kwargs): + # Uncomment for quick testing to make sure CoroutineDeferred magic isn't at fault + # from twisted.internet.defer import ensureDeferred + # return ensureDeferred(f(*args, **kwargs)) + return CoroutineDeferred(f(*args, **kwargs)) + + return wrapper diff --git a/deluge/tests/test_maybe_coroutine.py b/deluge/tests/test_maybe_coroutine.py new file mode 100644 index 0000000000..2717e78bb6 --- /dev/null +++ b/deluge/tests/test_maybe_coroutine.py @@ -0,0 +1,213 @@ +# +# This file is part of Deluge and is licensed under GNU General Public License 3.0, or later, with +# the additional special exception to link portions of this program with the OpenSSL library. +# See LICENSE for more details. +# +import pytest +import pytest_twisted +import twisted.python.failure +from twisted.internet import defer, reactor, task +from twisted.internet.defer import maybeDeferred + +from deluge.decorators import maybe_coroutine + + +@defer.inlineCallbacks +def inline_func(): + result = yield task.deferLater(reactor, 0, lambda: 'function_result') + return result + + +@defer.inlineCallbacks +def inline_error(): + raise Exception('function_error') + yield + + +@maybe_coroutine +async def coro_func(): + result = await task.deferLater(reactor, 0, lambda: 'function_result') + return result + + +@maybe_coroutine +async def coro_error(): + raise Exception('function_error') + + +@defer.inlineCallbacks +def coro_func_from_inline(): + result = yield coro_func() + return result + + +@defer.inlineCallbacks +def coro_error_from_inline(): + result = yield coro_error() + return result + + +@maybe_coroutine +async def coro_func_from_coro(): + return await coro_func() + + +@maybe_coroutine +async def coro_error_from_coro(): + return await coro_error() + + +@maybe_coroutine +async def inline_func_from_coro(): + return await inline_func() + + +@maybe_coroutine +async def inline_error_from_coro(): + return await inline_error() + + +@pytest_twisted.inlineCallbacks +def test_standard_twisted(): + """Sanity check that twisted tests work how we expect. + + Not really testing deluge code at all. + """ + result = yield inline_func() + assert result == 'function_result' + + with pytest.raises(Exception, match='function_error'): + yield inline_error() + + +@pytest.mark.parametrize( + 'function', + [ + inline_func, + coro_func, + coro_func_from_coro, + coro_func_from_inline, + inline_func_from_coro, + ], +) +@pytest_twisted.inlineCallbacks +def test_from_inline(function): + """Test our coroutines wrapped with maybe_coroutine as if they returned plain twisted deferreds.""" + result = yield function() + assert result == 'function_result' + + def cb(result): + assert result == 'function_result' + + d = function() + d.addCallback(cb) + yield d + + +@pytest.mark.parametrize( + 'function', + [ + inline_error, + coro_error, + coro_error_from_coro, + coro_error_from_inline, + inline_error_from_coro, + ], +) +@pytest_twisted.inlineCallbacks +def test_error_from_inline(function): + """Test our coroutines wrapped with maybe_coroutine as if they returned plain twisted deferreds that raise.""" + with pytest.raises(Exception, match='function_error'): + yield function() + + def eb(result): + assert isinstance(result, twisted.python.failure.Failure) + assert result.getErrorMessage() == 'function_error' + + d = function() + d.addErrback(eb) + yield d + + +@pytest.mark.parametrize( + 'function', + [ + inline_func, + coro_func, + coro_func_from_coro, + coro_func_from_inline, + inline_func_from_coro, + ], +) +@pytest_twisted.ensureDeferred +async def test_from_coro(function): + """Test our coroutines wrapped with maybe_coroutine work from another coroutine.""" + result = await function() + assert result == 'function_result' + + +@pytest.mark.parametrize( + 'function', + [ + inline_error, + coro_error, + coro_error_from_coro, + coro_error_from_inline, + inline_error_from_coro, + ], +) +@pytest_twisted.ensureDeferred +async def test_error_from_coro(function): + """Test our coroutines wrapped with maybe_coroutine work from another coroutine with errors.""" + with pytest.raises(Exception, match='function_error'): + await function() + + +@pytest_twisted.ensureDeferred +async def test_tracebacks_preserved(): + with pytest.raises(Exception) as exc: + await coro_error_from_coro() + traceback_lines = [ + 'await coro_error_from_coro()', + 'return await coro_error()', + "raise Exception('function_error')", + ] + # If each coroutine got wrapped with ensureDeferred, the traceback will be mangled + # verify the coroutines passed through by checking the traceback. + for expected, actual in zip(traceback_lines, exc.traceback): + assert expected in str(actual) + + +@pytest_twisted.ensureDeferred +async def test_maybe_deferred_coroutine(): + result = await maybeDeferred(coro_func) + assert result == 'function_result' + + +@pytest_twisted.ensureDeferred +async def test_callback_before_await(): + def cb(res): + assert res == 'function_result' + return res + + d = coro_func() + d.addCallback(cb) + result = await d + assert result == 'function_result' + + +@pytest_twisted.ensureDeferred +async def test_callback_after_await(): + """If it has already been used as a coroutine, can't be retroactively turned into a Deferred. + This limitation could be fixed, but the extra complication doesn't feel worth it. + """ + + def cb(res): + pass + + d = coro_func() + await d + with pytest.raises( + Exception, match='Cannot add callbacks to an already awaited coroutine' + ): + d.addCallback(cb) From 06118c43f5f9a86b134fc9407f11904b51794101 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Wed, 2 Feb 2022 00:21:54 -0500 Subject: [PATCH 02/15] [Core] Convert inlineCallbacks to maybe_coroutine --- deluge/core/core.py | 14 +++++----- deluge/core/torrentmanager.py | 9 ++++--- deluge/log.py | 49 ++++++++++++++++++----------------- 3 files changed, 37 insertions(+), 35 deletions(-) diff --git a/deluge/core/core.py b/deluge/core/core.py index 1090b0f2a3..398e3584aa 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -38,7 +38,7 @@ from deluge.core.preferencesmanager import PreferencesManager from deluge.core.rpcserver import export from deluge.core.torrentmanager import TorrentManager -from deluge.decorators import deprecated +from deluge.decorators import deprecated, maybe_coroutine from deluge.error import ( AddTorrentError, DelugeError, @@ -498,13 +498,13 @@ def add_torrent_files(self, torrent_files): """ - @defer.inlineCallbacks - def add_torrents(): + @maybe_coroutine + async def add_torrents(): errors = [] last_index = len(torrent_files) - 1 for idx, torrent in enumerate(torrent_files): try: - yield self.add_torrent_file_async( + await self.add_torrent_file_async( torrent[0], torrent[1], torrent[2], save_state=idx == last_index ) except AddTorrentError as ex: @@ -769,14 +769,14 @@ def get_torrent_status(self, torrent_id, keys, diff=False): ) @export - @defer.inlineCallbacks - def get_torrents_status(self, filter_dict, keys, diff=False): + @maybe_coroutine + async def get_torrents_status(self, filter_dict, keys, diff=False): """ returns all torrents , optionally filtered by filter_dict. """ all_keys = not keys torrent_ids = self.filtermanager.filter_torrent_ids(filter_dict) - status_dict, plugin_keys = yield self.torrentmanager.torrents_status_update( + status_dict, plugin_keys = await self.torrentmanager.torrents_status_update( torrent_ids, keys, diff=diff ) # Ask the plugin manager to fill in the plugin keys diff --git a/deluge/core/torrentmanager.py b/deluge/core/torrentmanager.py index 1bc05365c4..ef1cabe6be 100644 --- a/deluge/core/torrentmanager.py +++ b/deluge/core/torrentmanager.py @@ -33,6 +33,7 @@ from deluge.configmanager import ConfigManager, get_config_dir from deluge.core.authmanager import AUTH_LEVEL_ADMIN from deluge.core.torrent import Torrent, TorrentOptions, sanitize_filepath +from deluge.decorators import maybe_coroutine from deluge.error import AddTorrentError, InvalidTorrentError from deluge.event import ( ExternalIPEvent, @@ -247,8 +248,8 @@ def start(self): self.save_resume_data_timer.start(190, False) self.prev_status_cleanup_loop.start(10) - @defer.inlineCallbacks - def stop(self): + @maybe_coroutine + async def stop(self): # Stop timers if self.save_state_timer.running: self.save_state_timer.stop() @@ -260,11 +261,11 @@ def stop(self): self.prev_status_cleanup_loop.stop() # Save state on shutdown - yield self.save_state() + await self.save_state() self.session.pause() - result = yield self.save_resume_data(flush_disk_cache=True) + result = await self.save_resume_data(flush_disk_cache=True) # Remove the temp_file to signify successfully saved state if result and os.path.isfile(self.temp_file): os.remove(self.temp_file) diff --git a/deluge/log.py b/deluge/log.py index 6ce6c2df69..70151e6342 100644 --- a/deluge/log.py +++ b/deluge/log.py @@ -20,6 +20,7 @@ from twisted.python.log import PythonLoggingObserver from deluge import common +from deluge.decorators import maybe_coroutine __all__ = ('setup_logger', 'set_logger_level', 'get_plugin_logger', 'LOG') @@ -51,39 +52,39 @@ def __init__(self, logger_name): ) ) - @defer.inlineCallbacks - def garbage(self, msg, *args, **kwargs): - yield LoggingLoggerClass.log(self, 1, msg, *args, **kwargs) + @maybe_coroutine + async def garbage(self, msg, *args, **kwargs): + LoggingLoggerClass.log(self, 1, msg, *args, **kwargs) - @defer.inlineCallbacks - def trace(self, msg, *args, **kwargs): - yield LoggingLoggerClass.log(self, 5, msg, *args, **kwargs) + @maybe_coroutine + async def trace(self, msg, *args, **kwargs): + LoggingLoggerClass.log(self, 5, msg, *args, **kwargs) - @defer.inlineCallbacks - def debug(self, msg, *args, **kwargs): - yield LoggingLoggerClass.debug(self, msg, *args, **kwargs) + @maybe_coroutine + async def debug(self, msg, *args, **kwargs): + LoggingLoggerClass.debug(self, msg, *args, **kwargs) - @defer.inlineCallbacks - def info(self, msg, *args, **kwargs): - yield LoggingLoggerClass.info(self, msg, *args, **kwargs) + @maybe_coroutine + async def info(self, msg, *args, **kwargs): + LoggingLoggerClass.info(self, msg, *args, **kwargs) - @defer.inlineCallbacks - def warning(self, msg, *args, **kwargs): - yield LoggingLoggerClass.warning(self, msg, *args, **kwargs) + @maybe_coroutine + async def warning(self, msg, *args, **kwargs): + LoggingLoggerClass.warning(self, msg, *args, **kwargs) warn = warning - @defer.inlineCallbacks - def error(self, msg, *args, **kwargs): - yield LoggingLoggerClass.error(self, msg, *args, **kwargs) + @maybe_coroutine + async def error(self, msg, *args, **kwargs): + LoggingLoggerClass.error(self, msg, *args, **kwargs) - @defer.inlineCallbacks - def critical(self, msg, *args, **kwargs): - yield LoggingLoggerClass.critical(self, msg, *args, **kwargs) + @maybe_coroutine + async def critical(self, msg, *args, **kwargs): + LoggingLoggerClass.critical(self, msg, *args, **kwargs) - @defer.inlineCallbacks - def exception(self, msg, *args, **kwargs): - yield LoggingLoggerClass.exception(self, msg, *args, **kwargs) + @maybe_coroutine + async def exception(self, msg, *args, **kwargs): + LoggingLoggerClass.exception(self, msg, *args, **kwargs) def findCaller(self, *args, **kwargs): # NOQA: N802 f = logging.currentframe().f_back From 25c6ef99e17d72693a5bd23add0a300c58d76039 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Wed, 2 Feb 2022 19:50:12 -0500 Subject: [PATCH 03/15] [Core] Document all exported core methods with type hints --- deluge/core/core.py | 137 ++++++++++++++++++++-------------- deluge/path_chooser_common.py | 2 +- 2 files changed, 82 insertions(+), 57 deletions(-) diff --git a/deluge/core/core.py b/deluge/core/core.py index 1090b0f2a3..52479796c9 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -14,6 +14,7 @@ import tempfile import threading from base64 import b64decode, b64encode +from typing import Any, Dict, List, Optional, Tuple, Union from urllib.request import URLError, urlopen from twisted.internet import defer, reactor, task @@ -401,7 +402,9 @@ def check_new_release(self): # Exported Methods @export - def add_torrent_file_async(self, filename, filedump, options, save_state=True): + def add_torrent_file_async( + self, filename: str, filedump: str, options: dict, save_state: bool = True + ) -> defer.Deferred[Optional[str]]: """Adds a torrent file to the session asynchronously. Args: @@ -433,7 +436,9 @@ def add_torrent_file_async(self, filename, filedump, options, save_state=True): return d @export - def prefetch_magnet_metadata(self, magnet, timeout=30): + def prefetch_magnet_metadata( + self, magnet: str, timeout: int = 30 + ) -> defer.Deferred[Tuple[str, bytes]]: """Download magnet metadata without adding to Deluge session. Used by UIs to get magnet files for selection before adding to session. @@ -461,7 +466,9 @@ def on_metadata(result, result_d): return result_d @export - def add_torrent_file(self, filename, filedump, options): + def add_torrent_file( + self, filename: str, filedump: Union[str, bytes], options: dict + ) -> Optional[str]: """Adds a torrent file to the session. Args: @@ -486,7 +493,9 @@ def add_torrent_file(self, filename, filedump, options): raise @export - def add_torrent_files(self, torrent_files): + def add_torrent_files( + self, torrent_files: List[Tuple[str, Union[str, bytes], dict]] + ) -> defer.Deferred[List[AddTorrentError]]: """Adds multiple torrent files to the session asynchronously. Args: @@ -515,7 +524,9 @@ def add_torrents(): return task.deferLater(reactor, 0, add_torrents) @export - def add_torrent_url(self, url, options, headers=None): + def add_torrent_url( + self, url: str, options: dict, headers: dict = None + ) -> defer.Deferred[Optional[str]]: """ Adds a torrent from a URL. Deluge will attempt to fetch the torrent from the URL prior to adding it to the session. @@ -553,7 +564,7 @@ def on_download_fail(failure): return d @export - def add_torrent_magnet(self, uri, options): + def add_torrent_magnet(self, uri: str, options: dict) -> str: """ Adds a torrent from a magnet link. @@ -571,7 +582,7 @@ def add_torrent_magnet(self, uri, options): return self.torrentmanager.add(magnet=uri, options=options) @export - def remove_torrent(self, torrent_id, remove_data): + def remove_torrent(self, torrent_id: str, remove_data: bool) -> bool: """Removes a single torrent from the session. Args: @@ -589,7 +600,9 @@ def remove_torrent(self, torrent_id, remove_data): return self.torrentmanager.remove(torrent_id, remove_data) @export - def remove_torrents(self, torrent_ids, remove_data): + def remove_torrents( + self, torrent_ids: List[str], remove_data: bool + ) -> defer.Deferred[List[Tuple[str, str]]]: """Remove multiple torrents from the session. Args: @@ -625,7 +638,7 @@ def do_remove_torrents(): return task.deferLater(reactor, 0, do_remove_torrents) @export - def get_session_status(self, keys): + def get_session_status(self, keys: List[str]) -> Dict[str, Union[int, float]]: """Gets the session status values for 'keys', these keys are taking from libtorrent's session status. @@ -656,13 +669,13 @@ def get_session_status(self, keys): return status @export - def force_reannounce(self, torrent_ids): + def force_reannounce(self, torrent_ids: List[str]) -> None: log.debug('Forcing reannouncment to: %s', torrent_ids) for torrent_id in torrent_ids: self.torrentmanager[torrent_id].force_reannounce() @export - def pause_torrent(self, torrent_id): + def pause_torrent(self, torrent_id: str) -> None: """Pauses a torrent""" log.debug('Pausing: %s', torrent_id) if not isinstance(torrent_id, str): @@ -671,7 +684,7 @@ def pause_torrent(self, torrent_id): self.torrentmanager[torrent_id].pause() @export - def pause_torrents(self, torrent_ids=None): + def pause_torrents(self, torrent_ids: List[str] = None) -> None: """Pauses a list of torrents""" if not torrent_ids: torrent_ids = self.torrentmanager.get_torrent_list() @@ -679,27 +692,27 @@ def pause_torrents(self, torrent_ids=None): self.pause_torrent(torrent_id) @export - def connect_peer(self, torrent_id, ip, port): + def connect_peer(self, torrent_id: str, ip: str, port: int): log.debug('adding peer %s to %s', ip, torrent_id) if not self.torrentmanager[torrent_id].connect_peer(ip, port): log.warning('Error adding peer %s:%s to %s', ip, port, torrent_id) @export - def move_storage(self, torrent_ids, dest): + def move_storage(self, torrent_ids: List[str], dest: str): log.debug('Moving storage %s to %s', torrent_ids, dest) for torrent_id in torrent_ids: if not self.torrentmanager[torrent_id].move_storage(dest): log.warning('Error moving torrent %s to %s', torrent_id, dest) @export - def pause_session(self): + def pause_session(self) -> None: """Pause the entire session""" if not self.session.is_paused(): self.session.pause() component.get('EventManager').emit(SessionPausedEvent()) @export - def resume_session(self): + def resume_session(self) -> None: """Resume the entire session""" if self.session.is_paused(): self.session.resume() @@ -708,12 +721,12 @@ def resume_session(self): component.get('EventManager').emit(SessionResumedEvent()) @export - def is_session_paused(self): + def is_session_paused(self) -> bool: """Returns the activity of the session""" return self.session.is_paused() @export - def resume_torrent(self, torrent_id): + def resume_torrent(self, torrent_id: str) -> None: """Resumes a torrent""" log.debug('Resuming: %s', torrent_id) if not isinstance(torrent_id, str): @@ -722,7 +735,7 @@ def resume_torrent(self, torrent_id): self.torrentmanager[torrent_id].resume() @export - def resume_torrents(self, torrent_ids=None): + def resume_torrents(self, torrent_ids: List[str] = None) -> None: """Resumes a list of torrents""" if not torrent_ids: torrent_ids = self.torrentmanager.get_torrent_list() @@ -755,7 +768,9 @@ def create_torrent_status( return status @export - def get_torrent_status(self, torrent_id, keys, diff=False): + def get_torrent_status( + self, torrent_id: str, keys: List[str], diff: bool = False + ) -> dict: torrent_keys, plugin_keys = self.torrentmanager.separate_keys( keys, [torrent_id] ) @@ -770,7 +785,9 @@ def get_torrent_status(self, torrent_id, keys, diff=False): @export @defer.inlineCallbacks - def get_torrents_status(self, filter_dict, keys, diff=False): + def get_torrents_status( + self, filter_dict: dict, keys: List[str], diff: bool = False + ) -> dict: """ returns all torrents , optionally filtered by filter_dict. """ @@ -786,7 +803,7 @@ def get_torrents_status(self, filter_dict, keys, diff=False): return status_dict @export - def get_filter_tree(self, show_zero_hits=True, hide_cat=None): + def get_filter_tree(self, show_zero_hits: bool = True, hide_cat: List[str] = None): """ returns {field: [(value,count)] } for use in sidebar(s) @@ -794,28 +811,28 @@ def get_filter_tree(self, show_zero_hits=True, hide_cat=None): return self.filtermanager.get_filter_tree(show_zero_hits, hide_cat) @export - def get_session_state(self): + def get_session_state(self) -> List[str]: """Returns a list of torrent_ids in the session.""" # Get the torrent list from the TorrentManager return self.torrentmanager.get_torrent_list() @export - def get_config(self): + def get_config(self) -> dict: """Get all the preferences as a dictionary""" return self.config.config @export - def get_config_value(self, key): + def get_config_value(self, key: str) -> Any: """Get the config value for key""" return self.config.get(key) @export - def get_config_values(self, keys): + def get_config_values(self, keys: List[str]) -> Dict[str, Any]: """Get the config values for the entered keys""" return {key: self.config.get(key) for key in keys} @export - def set_config(self, config): + def set_config(self, config: Dict[str, Any]): """Set the config with values from dictionary""" # Load all the values into the configuration for key in config: @@ -824,12 +841,12 @@ def set_config(self, config): self.config[key] = config[key] @export - def get_listen_port(self): + def get_listen_port(self) -> int: """Returns the active listen port""" return self.session.listen_port() @export - def get_proxy(self): + def get_proxy(self) -> Dict[str, Any]: """Returns the proxy settings Returns: @@ -861,31 +878,33 @@ def get_proxy(self): return proxy_dict @export - def get_available_plugins(self): + def get_available_plugins(self) -> List[str]: """Returns a list of plugins available in the core""" return self.pluginmanager.get_available_plugins() @export - def get_enabled_plugins(self): + def get_enabled_plugins(self) -> List[str]: """Returns a list of enabled plugins in the core""" return self.pluginmanager.get_enabled_plugins() @export - def enable_plugin(self, plugin): + def enable_plugin(self, plugin: str) -> defer.Deferred[bool]: return self.pluginmanager.enable_plugin(plugin) @export - def disable_plugin(self, plugin): + def disable_plugin(self, plugin: str) -> defer.Deferred[bool]: return self.pluginmanager.disable_plugin(plugin) @export - def force_recheck(self, torrent_ids): + def force_recheck(self, torrent_ids: List[str]) -> None: """Forces a data recheck on torrent_ids""" for torrent_id in torrent_ids: self.torrentmanager[torrent_id].force_recheck() @export - def set_torrent_options(self, torrent_ids, options): + def set_torrent_options( + self, torrent_ids: List[str], options: Dict[str, Any] + ) -> None: """Sets the torrent options for torrent_ids Args: @@ -903,12 +922,14 @@ def set_torrent_options(self, torrent_ids, options): self.torrentmanager[torrent_id].set_options(options) @export - def set_torrent_trackers(self, torrent_id, trackers): + def set_torrent_trackers( + self, torrent_id: str, trackers: List[Dict[str, Any]] + ) -> None: """Sets a torrents tracker list. trackers will be ``[{"url", "tier"}]``""" return self.torrentmanager[torrent_id].set_trackers(trackers) @export - def get_magnet_uri(self, torrent_id): + def get_magnet_uri(self, torrent_id: str) -> str: return self.torrentmanager[torrent_id].get_magnet_uri() @deprecated @@ -1056,7 +1077,7 @@ def _create_torrent_thread( self.add_torrent_file(os.path.split(target)[1], filedump, options) @export - def upload_plugin(self, filename, filedump): + def upload_plugin(self, filename: str, filedump: Union[str, bytes]) -> None: """This method is used to upload new plugins to the daemon. It is used when connecting to the daemon remotely and installing a new plugin on the client side. ``plugin_data`` is a ``xmlrpc.Binary`` object of the file data, @@ -1074,14 +1095,16 @@ def upload_plugin(self, filename, filedump): component.get('CorePluginManager').scan_for_plugins() @export - def rescan_plugins(self): + def rescan_plugins(self) -> None: """ Re-scans the plugin folders for new plugins """ component.get('CorePluginManager').scan_for_plugins() @export - def rename_files(self, torrent_id, filenames): + def rename_files( + self, torrent_id: str, filenames: List[Tuple[int, str]] + ) -> defer.Deferred: """ Rename files in ``torrent_id``. Since this is an asynchronous operation by libtorrent, watch for the TorrentFileRenamedEvent to know when the @@ -1104,7 +1127,9 @@ def rename(): return task.deferLater(reactor, 0, rename) @export - def rename_folder(self, torrent_id, folder, new_folder): + def rename_folder( + self, torrent_id: str, folder: str, new_folder: str + ) -> defer.Deferred: """ Renames the 'folder' to 'new_folder' in 'torrent_id'. Watch for the TorrentFolderRenamedEvent which is emitted when the folder has been @@ -1126,7 +1151,7 @@ def rename_folder(self, torrent_id, folder, new_folder): return self.torrentmanager[torrent_id].rename_folder(folder, new_folder) @export - def queue_top(self, torrent_ids): + def queue_top(self, torrent_ids: List[str]) -> None: log.debug('Attempting to queue %s to top', torrent_ids) # torrent_ids must be sorted in reverse before moving to preserve order for torrent_id in sorted( @@ -1140,7 +1165,7 @@ def queue_top(self, torrent_ids): log.warning('torrent_id: %s does not exist in the queue', torrent_id) @export - def queue_up(self, torrent_ids): + def queue_up(self, torrent_ids: List[str]) -> None: log.debug('Attempting to queue %s to up', torrent_ids) torrents = ( (self.torrentmanager.get_queue_position(torrent_id), torrent_id) @@ -1165,7 +1190,7 @@ def queue_up(self, torrent_ids): prev_queue_position = queue_position @export - def queue_down(self, torrent_ids): + def queue_down(self, torrent_ids: List[str]) -> None: log.debug('Attempting to queue %s to down', torrent_ids) torrents = ( (self.torrentmanager.get_queue_position(torrent_id), torrent_id) @@ -1190,7 +1215,7 @@ def queue_down(self, torrent_ids): prev_queue_position = queue_position @export - def queue_bottom(self, torrent_ids): + def queue_bottom(self, torrent_ids: List[str]) -> None: log.debug('Attempting to queue %s to bottom', torrent_ids) # torrent_ids must be sorted before moving to preserve order for torrent_id in sorted( @@ -1204,11 +1229,11 @@ def queue_bottom(self, torrent_ids): log.warning('torrent_id: %s does not exist in the queue', torrent_id) @export - def glob(self, path): + def glob(self, path: str) -> List[str]: return glob.glob(path) @export - def test_listen_port(self): + def test_listen_port(self) -> defer.Deferred[Optional[bool]]: """ Checks if the active port is open @@ -1233,7 +1258,7 @@ def on_error(failure): return d @export - def get_free_space(self, path=None): + def get_free_space(self, path: str = None) -> int: """ Returns the number of free bytes at path @@ -1257,14 +1282,14 @@ def _on_external_ip_event(self, external_ip): self.external_ip = external_ip @export - def get_external_ip(self): + def get_external_ip(self) -> str: """ Returns the external IP address received from libtorrent. """ return self.external_ip @export - def get_libtorrent_version(self): + def get_libtorrent_version(self) -> str: """ Returns the libtorrent version. @@ -1275,28 +1300,28 @@ def get_libtorrent_version(self): return LT_VERSION @export - def get_completion_paths(self, args): + def get_completion_paths(self, args: Dict[str, Any]) -> Dict[str, Any]: """ Returns the available path completions for the input value. """ return path_chooser_common.get_completion_paths(args) @export(AUTH_LEVEL_ADMIN) - def get_known_accounts(self): + def get_known_accounts(self) -> List[Dict[str, Any]]: return self.authmanager.get_known_accounts() @export(AUTH_LEVEL_NONE) - def get_auth_levels_mappings(self): + def get_auth_levels_mappings(self) -> Tuple[Dict[str, int], Dict[int, str]]: return (AUTH_LEVELS_MAPPING, AUTH_LEVELS_MAPPING_REVERSE) @export(AUTH_LEVEL_ADMIN) - def create_account(self, username, password, authlevel): + def create_account(self, username: str, password: str, authlevel: str) -> bool: return self.authmanager.create_account(username, password, authlevel) @export(AUTH_LEVEL_ADMIN) - def update_account(self, username, password, authlevel): + def update_account(self, username: str, password: str, authlevel: str) -> bool: return self.authmanager.update_account(username, password, authlevel) @export(AUTH_LEVEL_ADMIN) - def remove_account(self, username): + def remove_account(self, username: str) -> bool: return self.authmanager.remove_account(username) diff --git a/deluge/path_chooser_common.py b/deluge/path_chooser_common.py index 858f7c2da0..0ea92341cf 100644 --- a/deluge/path_chooser_common.py +++ b/deluge/path_chooser_common.py @@ -40,7 +40,7 @@ def get_completion_paths(args): :param args: options :type args: dict :returns: the args argument containing the available completions for the completion_text - :rtype: list + :rtype: dict """ args['paths'] = [] From b62832a1bfc8a68214b70665ac4ff7e50e289d7d Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 3 Feb 2022 22:48:13 -0500 Subject: [PATCH 04/15] Standardize docstrings in core.py to google standard. Remove type hints in docstrings in favor of the ones in method signatures. --- deluge/core/core.py | 197 +++++++++++++++++++------------------------- 1 file changed, 83 insertions(+), 114 deletions(-) diff --git a/deluge/core/core.py b/deluge/core/core.py index 52479796c9..1e5774ff16 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -241,13 +241,12 @@ def apply_session_settings(self, settings): """Apply libtorrent session settings. Args: - settings (dict): A dict of lt session settings to apply. - + settings: A dict of lt session settings to apply. """ self.session.apply_settings(settings) @staticmethod - def _create_peer_id(version): + def _create_peer_id(version: str) -> str: """Create a peer_id fingerprint. This creates the peer_id and modifies the release char to identify @@ -262,11 +261,10 @@ def _create_peer_id(version): ``--DE201b--`` (beta pre-release of v2.0.1) Args: - version (str): The version string in PEP440 dotted notation. + version: The version string in PEP440 dotted notation. Returns: - str: The formatted peer_id with Deluge prefix e.g. '--DE200s--' - + The formatted peer_id with Deluge prefix e.g. '--DE200s--' """ split = deluge.common.VersionSplit(version) # Fill list with zeros to length of 4 and use lt to create fingerprint. @@ -315,12 +313,11 @@ def _save_session_state(self): log.info('Restoring backup of %s from: %s', filename, filepath_bak) shutil.move(filepath_bak, filepath) - def _load_session_state(self): + def _load_session_state(self) -> dict: """Loads the libtorrent session state Returns: - dict: A libtorrent sesion state, empty dict if unable to load it. - + A libtorrent sesion state, empty dict if unable to load it. """ filename = 'session.state' filepath = get_config_dir(filename) @@ -408,14 +405,13 @@ def add_torrent_file_async( """Adds a torrent file to the session asynchronously. Args: - filename (str): The filename of the torrent. - filedump (str): A base64 encoded string of torrent file contents. - options (dict): The options to apply to the torrent upon adding. - save_state (bool): If the state should be saved after adding the file. + filename: The filename of the torrent. + filedump: A base64 encoded string of torrent file contents. + options: The options to apply to the torrent upon adding. + save_state: If the state should be saved after adding the file. Returns: - Deferred: The torrent ID or None. - + The torrent ID or None. """ try: filedump = b64decode(filedump) @@ -446,12 +442,11 @@ def prefetch_magnet_metadata( The metadata is bencoded and for transfer base64 encoded. Args: - magnet (str): The magnet URI. - timeout (int): Number of seconds to wait before canceling request. + magnet: The magnet URI. + timeout: Number of seconds to wait before canceling request. Returns: - Deferred: A tuple of (torrent_id (str), metadata (str)) for the magnet. - + A tuple of (torrent_id (str), metadata (str)) for the magnet. """ def on_metadata(result, result_d): @@ -472,12 +467,12 @@ def add_torrent_file( """Adds a torrent file to the session. Args: - filename (str): The filename of the torrent. - filedump (str): A base64 encoded string of the torrent file contents. - options (dict): The options to apply to the torrent upon adding. + filename: The filename of the torrent. + filedump: A base64 encoded string of the torrent file contents. + options: The options to apply to the torrent upon adding. Returns: - str: The torrent_id or None. + The torrent_id or None. """ try: filedump = b64decode(filedump) @@ -499,12 +494,11 @@ def add_torrent_files( """Adds multiple torrent files to the session asynchronously. Args: - torrent_files (list of tuples): Torrent files as tuple of - ``(filename, filedump, options)``. + torrent_files: Torrent files as tuple of + ``(filename, filedump, options)``. Returns: - Deferred - + A list of errors (if there were any) """ @defer.inlineCallbacks @@ -527,18 +521,16 @@ def add_torrents(): def add_torrent_url( self, url: str, options: dict, headers: dict = None ) -> defer.Deferred[Optional[str]]: - """ - Adds a torrent from a URL. Deluge will attempt to fetch the torrent + """Adds a torrent from a URL. Deluge will attempt to fetch the torrent from the URL prior to adding it to the session. - :param url: the URL pointing to the torrent file - :type url: string - :param options: the options to apply to the torrent on add - :type options: dict - :param headers: any optional headers to send - :type headers: dict + Args: + url: the URL pointing to the torrent file + options: the options to apply to the torrent on add + headers: any optional headers to send - :returns: a Deferred which returns the torrent_id as a str or None + Returns: + a Deferred which returns the torrent_id as a str or None """ log.info('Attempting to add URL %s', url) @@ -565,17 +557,14 @@ def on_download_fail(failure): @export def add_torrent_magnet(self, uri: str, options: dict) -> str: - """ - Adds a torrent from a magnet link. - - :param uri: the magnet link - :type uri: string - :param options: the options to apply to the torrent on add - :type options: dict + """Adds a torrent from a magnet link. - :returns: the torrent_id - :rtype: string + Args: + uri: the magnet link + options: the options to apply to the torrent on add + Returns: + the torrent_id """ log.debug('Attempting to add by magnet URI: %s', uri) @@ -586,15 +575,14 @@ def remove_torrent(self, torrent_id: str, remove_data: bool) -> bool: """Removes a single torrent from the session. Args: - torrent_id (str): The torrent ID to remove. - remove_data (bool): If True, also remove the downloaded data. + torrent_id: The torrent ID to remove. + remove_data: If True, also remove the downloaded data. Returns: - bool: True if removed successfully. + True if removed successfully. Raises: InvalidTorrentError: If the torrent ID does not exist in the session. - """ log.debug('Removing torrent %s from the core.', torrent_id) return self.torrentmanager.remove(torrent_id, remove_data) @@ -606,15 +594,14 @@ def remove_torrents( """Remove multiple torrents from the session. Args: - torrent_ids (list): The torrent IDs to remove. - remove_data (bool): If True, also remove the downloaded data. + torrent_ids: The torrent IDs to remove. + remove_data: If True, also remove the downloaded data. Returns: - list: An empty list if no errors occurred otherwise the list contains - tuples of strings, a torrent ID and an error message. For example: - - [('', 'Error removing torrent')] + An empty list if no errors occurred otherwise the list contains + tuples of strings, a torrent ID and an error message. For example: + [('', 'Error removing torrent')] """ log.info('Removing %d torrents from core.', len(torrent_ids)) @@ -644,11 +631,11 @@ def get_session_status(self, keys: List[str]) -> Dict[str, Union[int, float]]: See: http://www.rasterbar.com/products/libtorrent/manual.html#status - :param keys: the keys for which we want values - :type keys: list - :returns: a dictionary of {key: value, ...} - :rtype: dict + Args: + keys: the keys for which we want values + Returns: + a dictionary of {key: value, ...} """ if not keys: return self.session_status @@ -788,9 +775,7 @@ def get_torrent_status( def get_torrents_status( self, filter_dict: dict, keys: List[str], diff: bool = False ) -> dict: - """ - returns all torrents , optionally filtered by filter_dict. - """ + """returns all torrents , optionally filtered by filter_dict.""" all_keys = not keys torrent_ids = self.filtermanager.filter_torrent_ids(filter_dict) status_dict, plugin_keys = yield self.torrentmanager.torrents_status_update( @@ -803,9 +788,10 @@ def get_torrents_status( return status_dict @export - def get_filter_tree(self, show_zero_hits: bool = True, hide_cat: List[str] = None): - """ - returns {field: [(value,count)] } + def get_filter_tree( + self, show_zero_hits: bool = True, hide_cat: List[str] = None + ) -> Dict: + """returns {field: [(value,count)] } for use in sidebar(s) """ return self.filtermanager.get_filter_tree(show_zero_hits, hide_cat) @@ -850,12 +836,11 @@ def get_proxy(self) -> Dict[str, Any]: """Returns the proxy settings Returns: - dict: Contains proxy settings. + Proxy settings. Notes: Proxy type names: 0: None, 1: Socks4, 2: Socks5, 3: Socks5 w Auth, 4: HTTP, 5: HTTP w Auth, 6: I2P - """ settings = self.session.get_settings() @@ -908,8 +893,8 @@ def set_torrent_options( """Sets the torrent options for torrent_ids Args: - torrent_ids (list): A list of torrent_ids to set the options for. - options (dict): A dict of torrent options to set. See + torrent_ids: A list of torrent_ids to set the options for. + options: A dict of torrent options to set. See ``torrent.TorrentOptions`` class for valid keys. """ if 'owner' in options and not self.authmanager.has_account(options['owner']): @@ -1096,27 +1081,23 @@ def upload_plugin(self, filename: str, filedump: Union[str, bytes]) -> None: @export def rescan_plugins(self) -> None: - """ - Re-scans the plugin folders for new plugins - """ + """Re-scans the plugin folders for new plugins""" component.get('CorePluginManager').scan_for_plugins() @export def rename_files( self, torrent_id: str, filenames: List[Tuple[int, str]] ) -> defer.Deferred: - """ - Rename files in ``torrent_id``. Since this is an asynchronous operation by + """Rename files in ``torrent_id``. Since this is an asynchronous operation by libtorrent, watch for the TorrentFileRenamedEvent to know when the files have been renamed. - :param torrent_id: the torrent_id to rename files - :type torrent_id: string - :param filenames: a list of index, filename pairs - :type filenames: ((index, filename), ...) - - :raises InvalidTorrentError: if torrent_id is invalid + Args: + torrent_id: the torrent_id to rename files + filenames: a list of index, filename pairs + Raises: + InvalidTorrentError: if torrent_id is invalid """ if torrent_id not in self.torrentmanager.torrents: raise InvalidTorrentError('torrent_id is not in session') @@ -1130,20 +1111,17 @@ def rename(): def rename_folder( self, torrent_id: str, folder: str, new_folder: str ) -> defer.Deferred: - """ - Renames the 'folder' to 'new_folder' in 'torrent_id'. Watch for the + """Renames the 'folder' to 'new_folder' in 'torrent_id'. Watch for the TorrentFolderRenamedEvent which is emitted when the folder has been renamed successfully. - :param torrent_id: the torrent to rename folder in - :type torrent_id: string - :param folder: the folder to rename - :type folder: string - :param new_folder: the new folder name - :type new_folder: string - - :raises InvalidTorrentError: if the torrent_id is invalid + Args: + torrent_id: the torrent to rename folder in + folder: the folder to rename + new_folder: the new folder name + Raises: + InvalidTorrentError: if the torrent_id is invalid """ if torrent_id not in self.torrentmanager.torrents: raise InvalidTorrentError('torrent_id is not in session') @@ -1234,12 +1212,10 @@ def glob(self, path: str) -> List[str]: @export def test_listen_port(self) -> defer.Deferred[Optional[bool]]: - """ - Checks if the active port is open - - :returns: True if the port is open, False if not - :rtype: bool + """Checks if the active port is open + Returns: + True if the port is open, False if not """ port = self.get_listen_port() url = 'https://deluge-torrent.org/test_port.php?port=%s' % port @@ -1259,17 +1235,16 @@ def on_error(failure): @export def get_free_space(self, path: str = None) -> int: - """ - Returns the number of free bytes at path + """Returns the number of free bytes at path - :param path: the path to check free space at, if None, use the default download location - :type path: string - - :returns: the number of free bytes at path - :rtype: int + Args: + path: the path to check free space at, if None, use the default download location - :raises InvalidPathError: if the path is invalid + Returns: + the number of free bytes at path + Raises: + InvalidPathError: if the path is invalid """ if not path: path = self.config['download_location'] @@ -1283,27 +1258,21 @@ def _on_external_ip_event(self, external_ip): @export def get_external_ip(self) -> str: - """ - Returns the external IP address received from libtorrent. - """ + """Returns the external IP address received from libtorrent.""" return self.external_ip @export def get_libtorrent_version(self) -> str: - """ - Returns the libtorrent version. - - :returns: the version - :rtype: string + """Returns the libtorrent version. + Returns: + the version """ return LT_VERSION @export def get_completion_paths(self, args: Dict[str, Any]) -> Dict[str, Any]: - """ - Returns the available path completions for the input value. - """ + """Returns the available path completions for the input value.""" return path_chooser_common.get_completion_paths(args) @export(AUTH_LEVEL_ADMIN) From 9ebbaf317da7986f53c724ccefebacf13fd00b06 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 3 Feb 2022 22:48:52 -0500 Subject: [PATCH 05/15] Use function signature type hints in autodoc generated docs. --- docs/requirements.txt | 1 + docs/source/conf.py | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/requirements.txt b/docs/requirements.txt index 18ef4fe47f..70739640a4 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -2,3 +2,4 @@ sphinx==4.* myst-parser sphinx_rtd_theme sphinxcontrib-spelling==7.3.0 +sphinx-autodoc-typehints diff --git a/docs/source/conf.py b/docs/source/conf.py index f04653e031..9c5853c739 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -48,6 +48,7 @@ 'sphinx.ext.coverage', 'sphinxcontrib.spelling', 'myst_parser', + 'sphinx_autodoc_typehints', ] napoleon_include_init_with_doc = True From e0770bec0899a80631a3b8569e647ee3397dccd4 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 10:42:28 -0500 Subject: [PATCH 06/15] Fix up core.py for mypy usage Add a mypy config to pyproject.toml --- deluge/core/core.py | 36 +++++++++++++++++++++--------------- pyproject.toml | 6 ++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/deluge/core/core.py b/deluge/core/core.py index 1e5774ff16..87330ffdcf 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -13,11 +13,15 @@ import shutil import tempfile import threading +import typing from base64 import b64decode, b64encode from typing import Any, Dict, List, Optional, Tuple, Union -from urllib.request import URLError, urlopen +from urllib.error import URLError +from urllib.request import urlopen -from twisted.internet import defer, reactor, task +import twisted.internet.reactor +from twisted.internet import defer, task +from twisted.internet.base import ReactorBase from twisted.web.client import Agent, readBody import deluge.common @@ -54,6 +58,8 @@ ) from deluge.httpdownloader import download_file +reactor = typing.cast(ReactorBase, twisted.internet.reactor) + log = logging.getLogger(__name__) DEPR_SESSION_STATUS_KEYS = { @@ -313,12 +319,8 @@ def _save_session_state(self): log.info('Restoring backup of %s from: %s', filename, filepath_bak) shutil.move(filepath_bak, filepath) - def _load_session_state(self) -> dict: - """Loads the libtorrent session state - - Returns: - A libtorrent sesion state, empty dict if unable to load it. - """ + def _load_session_state(self): + """Loads the libtorrent session state""" filename = 'session.state' filepath = get_config_dir(filename) filepath_bak = filepath + '.bak' @@ -400,7 +402,11 @@ def check_new_release(self): # Exported Methods @export def add_torrent_file_async( - self, filename: str, filedump: str, options: dict, save_state: bool = True + self, + filename: str, + filedump: Union[str, bytes], + options: dict, + save_state: bool = True, ) -> defer.Deferred[Optional[str]]: """Adds a torrent file to the session asynchronously. @@ -456,7 +462,7 @@ def on_metadata(result, result_d): d = self.torrentmanager.prefetch_metadata(magnet, timeout) # Use a separate callback chain to handle existing prefetching magnet. - result_d = defer.Deferred() + result_d: defer.Deferred = defer.Deferred() d.addBoth(on_metadata, result_d) return result_d @@ -515,7 +521,7 @@ def add_torrents(): errors.append(ex) defer.returnValue(errors) - return task.deferLater(reactor, 0, add_torrents) + return task.deferLater(reactor, 0, add_torrents) # type: ignore @export def add_torrent_url( @@ -673,7 +679,7 @@ def pause_torrent(self, torrent_id: str) -> None: @export def pause_torrents(self, torrent_ids: List[str] = None) -> None: """Pauses a list of torrents""" - if not torrent_ids: + if torrent_ids is None: torrent_ids = self.torrentmanager.get_torrent_list() for torrent_id in torrent_ids: self.pause_torrent(torrent_id) @@ -724,7 +730,7 @@ def resume_torrent(self, torrent_id: str) -> None: @export def resume_torrents(self, torrent_ids: List[str] = None) -> None: """Resumes a list of torrents""" - if not torrent_ids: + if torrent_ids is None: torrent_ids = self.torrentmanager.get_torrent_list() for torrent_id in torrent_ids: self.resume_torrent(torrent_id) @@ -770,9 +776,9 @@ def get_torrent_status( all_keys=not keys, ) - @export + @export # type: ignore @defer.inlineCallbacks - def get_torrents_status( + def get_torrents_status( # type: ignore self, filter_dict: dict, keys: List[str], diff: bool = False ) -> dict: """returns all torrents , optionally filtered by filter_dict.""" diff --git a/pyproject.toml b/pyproject.toml index 67ebe0a0c9..275414f13b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,3 +9,9 @@ skip-string-normalization = true [tool.isort] profile = "black" + +[tool.mypy] +python_version = 3.6 +namespace_packages = true +plugins = ["mypy_zope:plugin"] +allow_redefinition = true From f539109f332c2ad675112d56e7a0af8c3b5ce8ea Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 11:16:46 -0500 Subject: [PATCH 07/15] Add project level workaround for mypy twisted.internet.reactor Adds a custom stubs directory that defines twisted.internet.reactor for mypy. This prevents needing to cast the module in every file we want mypy to check. --- deluge/core/core.py | 7 +------ pyproject.toml | 1 + stubs/twisted/__init__.pyi | 3 +++ stubs/twisted/internet/__init__.pyi | 4 ++++ 4 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 stubs/twisted/__init__.pyi create mode 100644 stubs/twisted/internet/__init__.pyi diff --git a/deluge/core/core.py b/deluge/core/core.py index 87330ffdcf..e62661ac8b 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -13,15 +13,12 @@ import shutil import tempfile import threading -import typing from base64 import b64decode, b64encode from typing import Any, Dict, List, Optional, Tuple, Union from urllib.error import URLError from urllib.request import urlopen -import twisted.internet.reactor -from twisted.internet import defer, task -from twisted.internet.base import ReactorBase +from twisted.internet import defer, reactor, task from twisted.web.client import Agent, readBody import deluge.common @@ -58,8 +55,6 @@ ) from deluge.httpdownloader import download_file -reactor = typing.cast(ReactorBase, twisted.internet.reactor) - log = logging.getLogger(__name__) DEPR_SESSION_STATUS_KEYS = { diff --git a/pyproject.toml b/pyproject.toml index 275414f13b..5254329b71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,3 +15,4 @@ python_version = 3.6 namespace_packages = true plugins = ["mypy_zope:plugin"] allow_redefinition = true +mypy_path = ["$MYPY_CONFIG_FILE_DIR/stubs"] diff --git a/stubs/twisted/__init__.pyi b/stubs/twisted/__init__.pyi new file mode 100644 index 0000000000..d9817e5de2 --- /dev/null +++ b/stubs/twisted/__init__.pyi @@ -0,0 +1,3 @@ +from . import __all__ + +__version__: str diff --git a/stubs/twisted/internet/__init__.pyi b/stubs/twisted/internet/__init__.pyi new file mode 100644 index 0000000000..88066105b8 --- /dev/null +++ b/stubs/twisted/internet/__init__.pyi @@ -0,0 +1,4 @@ +from .base import ReactorBase + + +reactor: ReactorBase From c31d7773068ef839c257f5612191f90df6f53de1 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 21:04:51 -0500 Subject: [PATCH 08/15] Add some ignored 3rd party modules without typing to mypy config --- pyproject.toml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 5254329b71..d822913169 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,3 +16,16 @@ namespace_packages = true plugins = ["mypy_zope:plugin"] allow_redefinition = true mypy_path = ["$MYPY_CONFIG_FILE_DIR/stubs"] + +[[tool.mypy.overrides]] +module = "win32con" +ignore_missing_imports = true +[[tool.mypy.overrides]] +module = "win32api" +ignore_missing_imports = true +[[tool.mypy.overrides]] +module = "GeoIP" +ignore_missing_imports = true +[[tool.mypy.overrides]] +module = "pygeoip" +ignore_missing_imports = true From e1fff989eabcf1d881d993a361e3d5f1930c1345 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 21:28:47 -0500 Subject: [PATCH 09/15] Fix some more mypy issues --- deluge/core/core.py | 50 ++++++++++++++----------------- deluge/core/preferencesmanager.py | 3 +- deluge/core/torrentmanager.py | 3 +- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/deluge/core/core.py b/deluge/core/core.py index 73f657434e..7823a44585 100644 --- a/deluge/core/core.py +++ b/deluge/core/core.py @@ -133,13 +133,13 @@ def __init__( self.session.add_extension('smart_ban') # Create the components - self.eventmanager = EventManager() - self.preferencesmanager = PreferencesManager() - self.alertmanager = AlertManager() - self.pluginmanager = PluginManager(self) - self.torrentmanager = TorrentManager() - self.filtermanager = FilterManager(self) - self.authmanager = AuthManager() + self.eventmanager: EventManager = EventManager() + self.preferencesmanager: PreferencesManager = PreferencesManager() + self.alertmanager: AlertManager = AlertManager() + self.pluginmanager: PluginManager = PluginManager(self) + self.torrentmanager: TorrentManager = TorrentManager() + self.filtermanager: FilterManager = FilterManager(self) + self.authmanager: AuthManager = AuthManager() # New release check information self.new_release = None @@ -489,9 +489,10 @@ def add_torrent_file( raise @export - def add_torrent_files( + @maybe_coroutine + async def add_torrent_files( self, torrent_files: List[Tuple[str, Union[str, bytes], dict]] - ) -> defer.Deferred[List[AddTorrentError]]: + ) -> List[AddTorrentError]: """Adds multiple torrent files to the session asynchronously. Args: @@ -501,22 +502,17 @@ def add_torrent_files( Returns: A list of errors (if there were any) """ - - @maybe_coroutine - async def add_torrents(): - errors = [] - last_index = len(torrent_files) - 1 - for idx, torrent in enumerate(torrent_files): - try: - await self.add_torrent_file_async( - torrent[0], torrent[1], torrent[2], save_state=idx == last_index - ) - except AddTorrentError as ex: - log.warning('Error when adding torrent: %s', ex) - errors.append(ex) - defer.returnValue(errors) - - return task.deferLater(reactor, 0, add_torrents) + errors = [] + last_index = len(torrent_files) - 1 + for idx, torrent in enumerate(torrent_files): + try: + await self.add_torrent_file_async( + torrent[0], torrent[1], torrent[2], save_state=idx == last_index + ) + except AddTorrentError as ex: + log.warning('Error when adding torrent: %s', ex) + errors.append(ex) + return errors @export def add_torrent_url( @@ -674,7 +670,7 @@ def pause_torrent(self, torrent_id: str) -> None: @export def pause_torrents(self, torrent_ids: List[str] = None) -> None: """Pauses a list of torrents""" - if torrent_ids is None: + if not torrent_ids: torrent_ids = self.torrentmanager.get_torrent_list() for torrent_id in torrent_ids: self.pause_torrent(torrent_id) @@ -725,7 +721,7 @@ def resume_torrent(self, torrent_id: str) -> None: @export def resume_torrents(self, torrent_ids: List[str] = None) -> None: """Resumes a list of torrents""" - if torrent_ids is None: + if not torrent_ids: torrent_ids = self.torrentmanager.get_torrent_list() for torrent_id in torrent_ids: self.resume_torrent(torrent_id) diff --git a/deluge/core/preferencesmanager.py b/deluge/core/preferencesmanager.py index 79e960252e..d921da0aab 100644 --- a/deluge/core/preferencesmanager.py +++ b/deluge/core/preferencesmanager.py @@ -23,14 +23,13 @@ from deluge._libtorrent import lt from deluge.event import ConfigValueChangedEvent -GeoIP = None try: from GeoIP import GeoIP except ImportError: try: from pygeoip import GeoIP except ImportError: - pass + GeoIP = None log = logging.getLogger(__name__) diff --git a/deluge/core/torrentmanager.py b/deluge/core/torrentmanager.py index ef1cabe6be..aa06e34e42 100644 --- a/deluge/core/torrentmanager.py +++ b/deluge/core/torrentmanager.py @@ -16,6 +16,7 @@ from base64 import b64encode from collections import namedtuple from tempfile import gettempdir +from typing import List from twisted.internet import defer, error, reactor, threads from twisted.internet.defer import Deferred, DeferredList @@ -306,7 +307,7 @@ def __getitem__(self, torrent_id): """ return self.torrents[torrent_id] - def get_torrent_list(self): + def get_torrent_list(self) -> List[str]: """Creates a list of torrent_ids, owned by current user and any marked shared. Returns: From 209e9bb20dbdf0c530b85c758d00315ace78d5b4 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 21:36:57 -0500 Subject: [PATCH 10/15] Keep typing stuff --- deluge/core/torrentmanager.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/deluge/core/torrentmanager.py b/deluge/core/torrentmanager.py index aa06e34e42..69c33714cf 100644 --- a/deluge/core/torrentmanager.py +++ b/deluge/core/torrentmanager.py @@ -16,7 +16,7 @@ from base64 import b64encode from collections import namedtuple from tempfile import gettempdir -from typing import List +from typing import Any, Dict, List, Optional from twisted.internet import defer, error, reactor, threads from twisted.internet.defer import Deferred, DeferredList @@ -295,7 +295,7 @@ def update(self): if not torrent.handle.status().paused: torrent.pause() - def __getitem__(self, torrent_id): + def __getitem__(self, torrent_id: str) -> Torrent: """Return the Torrent with torrent_id. Args: @@ -311,7 +311,7 @@ def get_torrent_list(self) -> List[str]: """Creates a list of torrent_ids, owned by current user and any marked shared. Returns: - list: A list of torrent_ids. + A list of torrent_ids. """ torrent_ids = list(self.torrents) @@ -325,11 +325,11 @@ def get_torrent_list(self) -> List[str]: torrent_ids.pop(torrent_ids.index(torrent_id)) return torrent_ids - def get_torrent_info_from_file(self, filepath): + def get_torrent_info_from_file(self, filepath: str) -> Optional[Dict[str, Any]]: """Retrieves torrent_info from the file specified. Args: - filepath (str): The filepath to extract torrent info from. + filepath: The filepath to extract torrent info from. Returns: lt.torrent_info: A libtorrent torrent_info dict or None if invalid file or data. @@ -342,15 +342,16 @@ def get_torrent_info_from_file(self, filepath): torrent_info = lt.torrent_info(filepath) except RuntimeError as ex: log.warning('Unable to open torrent file %s: %s', filepath, ex) + return None else: return torrent_info - def prefetch_metadata(self, magnet, timeout): + def prefetch_metadata(self, magnet: str, timeout: int) -> Deferred: """Download the metadata for a magnet URI. Args: - magnet (str): A magnet URI to download the metadata for. - timeout (int): Number of seconds to wait before canceling. + magnet: A magnet URI to download the metadata for. + timeout: Number of seconds to wait before canceling. Returns: Deferred: A tuple of (torrent_id (str), metadata (dict)) @@ -361,7 +362,7 @@ def prefetch_metadata(self, magnet, timeout): if torrent_id in self.prefetching_metadata: return self.prefetching_metadata[torrent_id].defer - add_torrent_params = {} + add_torrent_params: Dict[str, Any] = {} add_torrent_params['save_path'] = gettempdir() add_torrent_params['url'] = magnet.strip().encode('utf8') add_torrent_params['flags'] = ( @@ -376,9 +377,9 @@ def prefetch_metadata(self, magnet, timeout): torrent_handle = self.session.add_torrent(add_torrent_params) - d = Deferred() + d: Deferred = Deferred() # Cancel the defer if timeout reached. - defer_timeout = self.callLater(timeout, d.cancel) + defer_timeout = TorrentManager.callLater(timeout, d.cancel) d.addBoth(self.on_prefetch_metadata, torrent_id, defer_timeout) Prefetch = namedtuple('Prefetch', 'defer handle') self.prefetching_metadata[torrent_id] = Prefetch(defer=d, handle=torrent_handle) From 447e3fc39d01cd9d58daa74eb6ea768c06ca5bfa Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 22:45:31 -0500 Subject: [PATCH 11/15] Attempt to enable mypy on github actions --- .github/workflows/mypy.yml | 43 ++++++++++++++++++++++++++++++++++++++ requirements-tests.txt | 2 ++ 2 files changed, 45 insertions(+) create mode 100644 .github/workflows/mypy.yml diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml new file mode 100644 index 0000000000..990c4370fa --- /dev/null +++ b/.github/workflows/mypy.yml @@ -0,0 +1,43 @@ +name: mypy + +on: + push: + pull_request: + + # Allows you to run this workflow manually from the Actions tab + workflow_dispatch: + +jobs: + test-linux: + runs-on: ubuntu-20.04 + + steps: + # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: "3.10" + cache: "pip" + + # - name: Add libtorrent deb repository + # uses: myci-actions/add-deb-repo@8 + # with: + # repo: deb http://ppa.launchpad.net/libtorrent.org/1.2-daily/ubuntu focal main + # repo-name: libtorrent + # keys: 58E5430D9667FAEFFCA0B93F32309D6B9E009EDB + # key-server: keyserver.ubuntu.com + # install: python3-libtorrent-dbg + + - name: Install dependencies + run: | + pip install --upgrade pip wheel + pip install -r requirements.txt -r requirements-tests.txt + pip install -e . + + - name: Run mypy + run: | + mypy deluge/ diff --git a/requirements-tests.txt b/requirements-tests.txt index 705d967745..aba2cd7c33 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -2,6 +2,8 @@ pytest != 5.2.3, < 5.4 pytest-twisted pytest-cov mock +mypy +mypy-zope pre-commit flake8<=3.7.9 flake8-quotes From 2b1e7184b0f2e252be7ad988efff8ff715deb25d Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 23:00:20 -0500 Subject: [PATCH 12/15] Disable type checking for pytest Add pygobject-stubs --- pyproject.toml | 9 +++------ requirements-tests.txt | 6 ++++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d822913169..b8161a140f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,14 +18,11 @@ allow_redefinition = true mypy_path = ["$MYPY_CONFIG_FILE_DIR/stubs"] [[tool.mypy.overrides]] -module = "win32con" +module = ["win32con", "win32api"] ignore_missing_imports = true [[tool.mypy.overrides]] -module = "win32api" +module = ["GeoIP", "pygeoip"] ignore_missing_imports = true [[tool.mypy.overrides]] -module = "GeoIP" -ignore_missing_imports = true -[[tool.mypy.overrides]] -module = "pygeoip" +module = ["pytest", "pytest_twisted"] ignore_missing_imports = true diff --git a/requirements-tests.txt b/requirements-tests.txt index aba2cd7c33..c62267f35f 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -2,8 +2,6 @@ pytest != 5.2.3, < 5.4 pytest-twisted pytest-cov mock -mypy -mypy-zope pre-commit flake8<=3.7.9 flake8-quotes @@ -11,3 +9,7 @@ flake8-isort pep8-naming mccabe pylint +# Type checking stuff +mypy +mypy-zope +pygobject-stubs From 9cf57b14fbdce17a25b518c7e4376719e1096b1b Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 23:09:53 -0500 Subject: [PATCH 13/15] Try to get pygobject-stubs working --- .github/workflows/mypy.yml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 990c4370fa..b2178c2426 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -23,14 +23,10 @@ jobs: python-version: "3.10" cache: "pip" - # - name: Add libtorrent deb repository - # uses: myci-actions/add-deb-repo@8 - # with: - # repo: deb http://ppa.launchpad.net/libtorrent.org/1.2-daily/ubuntu focal main - # repo-name: libtorrent - # keys: 58E5430D9667FAEFFCA0B93F32309D6B9E009EDB - # key-server: keyserver.ubuntu.com - # install: python3-libtorrent-dbg + # This is needed for pygobject-stubs + - name: Install system packages + run: | + sudo apt install libgirepository1.0-dev - name: Install dependencies run: | From 15a63d8d14d8df5d8b5bf4ee20122789f517b6a5 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 23:18:58 -0500 Subject: [PATCH 14/15] Add some more type stubs --- requirements-tests.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/requirements-tests.txt b/requirements-tests.txt index c62267f35f..1805f30d33 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -13,3 +13,7 @@ pylint mypy mypy-zope pygobject-stubs +types-certifi +types-chardet +types-pyOpenSSL +types-setuptools From 6f522bd259d135c7d5fbc2ce98585f9875a7ac23 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 4 Feb 2022 23:19:21 -0500 Subject: [PATCH 15/15] Ignore win32 types --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index b8161a140f..e26d1075bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ allow_redefinition = true mypy_path = ["$MYPY_CONFIG_FILE_DIR/stubs"] [[tool.mypy.overrides]] -module = ["win32con", "win32api"] +module = ["win32con", "win32api", "win32file", "win32process", "win32event", "winerror"] ignore_missing_imports = true [[tool.mypy.overrides]] module = ["GeoIP", "pygeoip"]