From 40da20d8b5d71cbbb884f42cc68fc80b51480d54 Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Tue, 6 Sep 2022 10:56:57 +0200 Subject: [PATCH 1/6] Fix broken test --- src/tribler/gui/tests/test_core_manager.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/tribler/gui/tests/test_core_manager.py b/src/tribler/gui/tests/test_core_manager.py index 98ebfc337c5..9764a976ec2 100644 --- a/src/tribler/gui/tests/test_core_manager.py +++ b/src/tribler/gui/tests/test_core_manager.py @@ -71,13 +71,9 @@ def test_decode_raw_core_output(core_manager): def test_format_error_message(): - actual = CoreManager.format_error_message(exit_code=errno.ENOENT, exit_status=1, last_core_output='last\noutput') + actual = CoreManager.format_error_message(exit_code=errno.ENOENT, exit_status=1) expected = '''The Tribler core has unexpectedly finished with exit code 2 and status: 1. -Error message: No such file or directory - -Last core output: -> last -> output''' +Error message: No such file or directory''' assert actual == expected From cdbb9e842546e14905aaf9dbed1eba589e94c06a Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Wed, 31 Aug 2022 08:34:09 +0200 Subject: [PATCH 2/6] Increase EventRequestManager timeout to two minutes to be sure it can connect to the events endpoint if some component delays the start of REST API --- src/tribler/gui/event_request_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tribler/gui/event_request_manager.py b/src/tribler/gui/event_request_manager.py index 9e73f144bf2..e8c24a00e57 100644 --- a/src/tribler/gui/event_request_manager.py +++ b/src/tribler/gui/event_request_manager.py @@ -15,7 +15,7 @@ received_events = [] -CORE_CONNECTION_TIMEOUT = 60 +CORE_CONNECTION_TIMEOUT = 120 RECONNECT_INTERVAL_MS = 100 From 640d0fa5f0effd9c8e5fdfc47b3fa6eed4e10a1d Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Wed, 31 Aug 2022 08:32:15 +0200 Subject: [PATCH 3/6] Load libtorrent checkpoints in a background task to not delay initialization of REST API --- .../download_manager/download_manager.py | 21 ++++++++++++++++--- .../libtorrent/libtorrent_component.py | 11 +++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/tribler/core/components/libtorrent/download_manager/download_manager.py b/src/tribler/core/components/libtorrent/download_manager/download_manager.py index ced69f75ed3..9d604d4e1f0 100644 --- a/src/tribler/core/components/libtorrent/download_manager/download_manager.py +++ b/src/tribler/core/components/libtorrent/download_manager/download_manager.py @@ -32,6 +32,7 @@ HTTPS_SCHEME, HTTP_SCHEME, MAGNET_SCHEME, + path_to_url, scheme_from_url, url_to_path, ) @@ -72,6 +73,7 @@ def __init__(self, notifier: Notifier, peer_mid: bytes, config: LibtorrentSettings = None, + gui_test_mode: bool = False, download_defaults: DownloadDefaultsSettings = None, bootstrap_infohash=None, socks_listen_ports: Optional[List[int]] = None, @@ -92,6 +94,7 @@ def __init__(self, self.notifier = notifier self.peer_mid = peer_mid self.config = config or LibtorrentSettings() + self.gui_test_mode = gui_test_mode self.bootstrap_infohash = bootstrap_infohash self.download_defaults = download_defaults or DownloadDefaultsSettings() self._libtorrent_port = None @@ -158,10 +161,23 @@ def initialize(self): self.set_download_states_callback(self.sesscb_states_callback) + def start(self): + self.register_task("start", self._start) + + async def _start(self): + await self.load_checkpoints() + + if self.gui_test_mode: + from tribler.core.tests.tools.common import TORRENT_WITH_DIRS # pylint: disable=import-outside-toplevel + uri = path_to_url(TORRENT_WITH_DIRS) + await self.start_download_from_uri(uri) + def notify_shutdown_state(self, state): self.notifier[notifications.tribler_shutdown_state](state) async def shutdown(self, timeout=30): + self.cancel_pending_task("start") + self.cancel_pending_task("download_states_lc") if self.downloads: self.notify_shutdown_state("Checkpointing Downloads...") await gather(*[download.stop() for download in self.downloads.values()], return_exceptions=True) @@ -782,9 +798,6 @@ def set_download_states_callback(self, user_callback, interval=1.0): self._logger.debug("Starting the download state callback with interval %f", interval) self.replace_task("download_states_lc", self._invoke_states_cb, user_callback, interval=interval) - def stop_download_states_callback(self): - return self.cancel_pending_task("download_states_lc") - async def _invoke_states_cb(self, callback): """ Invoke the download states callback with a list of the download states. @@ -825,9 +838,11 @@ def get_last_download_states(self): return self._last_states_list async def load_checkpoints(self): + self._logger.info("Load checkpoints...") for filename in self.get_checkpoint_dir().glob('*.conf'): self.load_checkpoint(filename) await sleep(.01) + self._logger.info("Checkpoints are loaded") def load_checkpoint(self, filename): try: diff --git a/src/tribler/core/components/libtorrent/libtorrent_component.py b/src/tribler/core/components/libtorrent/libtorrent_component.py index b2619c63a82..ff586d1eb0d 100644 --- a/src/tribler/core/components/libtorrent/libtorrent_component.py +++ b/src/tribler/core/components/libtorrent/libtorrent_component.py @@ -2,7 +2,6 @@ from tribler.core.components.key.key_component import KeyComponent from tribler.core.components.libtorrent.download_manager.download_manager import DownloadManager from tribler.core.components.socks_servers.socks_servers_component import SocksServersComponent -from tribler.core.utilities.rest_utils import path_to_url class LibtorrentComponent(Component): @@ -21,6 +20,7 @@ async def run(self): self.download_manager = DownloadManager( config=config.libtorrent, + gui_test_mode=config.gui_test_mode, state_dir=config.state_dir, notifier=self.session.notifier, peer_mid=key_component.primary_key.key_to_hash(), @@ -30,15 +30,10 @@ async def run(self): dummy_mode=config.gui_test_mode) self.download_manager.initialize() - await self.download_manager.load_checkpoints() - - if config.gui_test_mode: - from tribler.core.tests.tools.common import TORRENT_WITH_DIRS # pylint: disable=import-outside-toplevel - uri = path_to_url(TORRENT_WITH_DIRS) - await self.download_manager.start_download_from_uri(uri) + # load checkpoints in a background task to not delay initialization of dependent components (e.g. RESTComponent) + self.download_manager.start() async def shutdown(self): await super().shutdown() if self.download_manager: - self.download_manager.stop_download_states_callback() await self.download_manager.shutdown() From ac08f67006f14e13112211b16737a45b2c259cd2 Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Tue, 6 Sep 2022 09:44:15 +0200 Subject: [PATCH 4/6] Delay showing the download list in the UI until all checkpoints are loaded; show the number of the loaded checkpoints instead --- .../download_manager/download_manager.py | 10 +++++++- .../libtorrent/restapi/downloads_endpoint.py | 18 +++++++++++-- .../restapi/tests/test_downloads_endpoint.py | 23 ++++++++++++++--- .../libtorrent/tests/test_download_manager.py | 16 ++++++++++++ src/tribler/core/conftest.py | 3 +++ src/tribler/gui/widgets/downloadspage.py | 25 ++++++++++++++++--- 6 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/tribler/core/components/libtorrent/download_manager/download_manager.py b/src/tribler/core/components/libtorrent/download_manager/download_manager.py index 9d604d4e1f0..fdbe56f4380 100644 --- a/src/tribler/core/components/libtorrent/download_manager/download_manager.py +++ b/src/tribler/core/components/libtorrent/download_manager/download_manager.py @@ -104,6 +104,10 @@ def __init__(self, self.downloads = {} + self.checkpoints_count = None + self.checkpoints_loaded = 0 + self.all_checkpoints_are_loaded = False + self.metadata_tmpdir = None # Dictionary that maps infohashes to download instances. These include only downloads that have # been made specifically for fetching metainfo, and will be removed afterwards. @@ -839,9 +843,13 @@ def get_last_download_states(self): async def load_checkpoints(self): self._logger.info("Load checkpoints...") - for filename in self.get_checkpoint_dir().glob('*.conf'): + checkpoint_filenames = list(self.get_checkpoint_dir().glob('*.conf')) + self.checkpoints_count = len(checkpoint_filenames) + for i, filename in enumerate(checkpoint_filenames, start=1): self.load_checkpoint(filename) + self.checkpoints_loaded = i await sleep(.01) + self.all_checkpoints_are_loaded = True self._logger.info("Checkpoints are loaded") def load_checkpoint(self, filename): diff --git a/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py b/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py index e5862a039da..c1170cd923e 100644 --- a/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py +++ b/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py @@ -84,7 +84,7 @@ class DownloadsEndpoint(RESTEndpoint): starting, pausing and stopping downloads. """ - def __init__(self, download_manager, metadata_store=None, tunnel_community=None): + def __init__(self, download_manager: DownloadManager, metadata_store=None, tunnel_community=None): super().__init__() self.download_manager = download_manager self.mds = metadata_store @@ -219,6 +219,11 @@ def get_files_info_json(download): 'vod_prebuffering_progress_consec': Float, 'error': String, 'time_added': Integer + }), + 'checkpoints': schema(Checkpoints={ + 'total': Integer, + 'loaded': Integer, + 'all_loaded': Boolean, }) }), } @@ -237,6 +242,15 @@ async def get_downloads(self, request): get_pieces = params.get('get_pieces', '0') == '1' get_files = params.get('get_files', '0') == '1' + checkpoints = { + "total": self.download_manager.checkpoints_count, + "loaded": self.download_manager.checkpoints_loaded, + "all_loaded": self.download_manager.all_checkpoints_are_loaded, + } + + if not self.download_manager.all_checkpoints_are_loaded: + return RESTResponse({"downloads": [], "checkpoints": checkpoints}) + downloads_json = [] downloads = self.download_manager.get_downloads() for download in downloads: @@ -332,7 +346,7 @@ async def get_downloads(self, request): download_json["files"] = self.get_files_info_json(download) downloads_json.append(download_json) - return RESTResponse({"downloads": downloads_json}) + return RESTResponse({"downloads": downloads_json, "checkpoints": checkpoints}) @docs( tags=["Libtorrent"], diff --git a/src/tribler/core/components/libtorrent/restapi/tests/test_downloads_endpoint.py b/src/tribler/core/components/libtorrent/restapi/tests/test_downloads_endpoint.py index 303652199c0..66098cb6143 100644 --- a/src/tribler/core/components/libtorrent/restapi/tests/test_downloads_endpoint.py +++ b/src/tribler/core/components/libtorrent/restapi/tests/test_downloads_endpoint.py @@ -149,13 +149,25 @@ def test_get_extended_status_circuits(mock_extended_status): assert mock_extended_status == DLSTATUS_CIRCUITS +async def test_get_downloads_if_checkpoints_are_not_loaded(mock_dlmgr, rest_api): + mock_dlmgr.checkpoints_count = 10 + mock_dlmgr.checkpoints_loaded = 5 + mock_dlmgr.all_checkpoints_are_loaded = False + + expected_json = {"downloads": [], "checkpoints": {"total": 10, "loaded": 5, "all_loaded": False}} + await do_request(rest_api, "downloads?get_peers=1&get_pieces=1", expected_code=200, expected_json=expected_json) + + async def test_get_downloads_no_downloads(mock_dlmgr, rest_api): """ Testing whether the API returns an empty list when downloads are fetched but no downloads are active """ - result = await do_request(rest_api, 'downloads?get_peers=1&get_pieces=1', - expected_code=200, expected_json={"downloads": []}) - assert result["downloads"] == [] + mock_dlmgr.checkpoints_count = 0 + mock_dlmgr.checkpoints_loaded = 0 + mock_dlmgr.all_checkpoints_are_loaded = True + + expected_json = {"downloads": [], "checkpoints": {"total": 0, "loaded": 0, "all_loaded": True}} + await do_request(rest_api, "downloads?get_peers=1&get_pieces=1", expected_code=200, expected_json=expected_json) async def test_get_downloads(mock_dlmgr, test_download, rest_api): @@ -163,8 +175,13 @@ async def test_get_downloads(mock_dlmgr, test_download, rest_api): Testing whether the API returns the right download when a download is added """ mock_dlmgr.get_downloads = lambda: [test_download] + mock_dlmgr.checkpoints_count = 1 + mock_dlmgr.checkpoints_loaded = 1 + mock_dlmgr.all_checkpoints_are_loaded = True + downloads = await do_request(rest_api, 'downloads?get_peers=1&get_pieces=1', expected_code=200) assert len(downloads["downloads"]) == 1 + assert downloads["checkpoints"] == {"total": 1, "loaded": 1, "all_loaded": True} async def test_start_download_no_uri(rest_api): diff --git a/src/tribler/core/components/libtorrent/tests/test_download_manager.py b/src/tribler/core/components/libtorrent/tests/test_download_manager.py index 20d8f7cd65c..c947e56fe90 100644 --- a/src/tribler/core/components/libtorrent/tests/test_download_manager.py +++ b/src/tribler/core/components/libtorrent/tests/test_download_manager.py @@ -1,3 +1,4 @@ +import asyncio from asyncio import Future, gather, get_event_loop, sleep from unittest.mock import MagicMock @@ -381,6 +382,13 @@ def mock_start_download(*_, **__): assert not good +@pytest.mark.asyncio +async def test_download_manager_start(fake_dlmgr): + fake_dlmgr.start() + await asyncio.sleep(0.01) + assert fake_dlmgr.all_checkpoints_are_loaded + + def test_load_empty_checkpoint(fake_dlmgr, tmpdir): """ Test whether download resumes with faulty pstate file. @@ -413,8 +421,16 @@ def mocked_load_checkpoint(filename): state_file.write(b"hi") fake_dlmgr.load_checkpoint = mocked_load_checkpoint + assert fake_dlmgr.all_checkpoints_are_loaded is False + assert fake_dlmgr.checkpoints_count is None + assert fake_dlmgr.checkpoints_loaded == 0 + await fake_dlmgr.load_checkpoints() + assert mocked_load_checkpoint.called + assert fake_dlmgr.all_checkpoints_are_loaded is True + assert fake_dlmgr.checkpoints_count == 1 + assert fake_dlmgr.checkpoints_loaded == 1 @pytest.mark.asyncio diff --git a/src/tribler/core/conftest.py b/src/tribler/core/conftest.py index 3ff8f0630ce..97545852fd3 100644 --- a/src/tribler/core/conftest.py +++ b/src/tribler/core/conftest.py @@ -96,6 +96,9 @@ def mock_dlmgr(state_dir): dlmgr.get_checkpoint_dir = lambda: checkpoints_dir dlmgr.state_dir = state_dir dlmgr.get_downloads = lambda: [] + dlmgr.checkpoints_count = 1 + dlmgr.checkpoints_loaded = 1 + dlmgr.all_checkpoints_are_loaded = True return dlmgr diff --git a/src/tribler/gui/widgets/downloadspage.py b/src/tribler/gui/widgets/downloadspage.py index 8da7591cdf5..0cb5a7efed8 100644 --- a/src/tribler/gui/widgets/downloadspage.py +++ b/src/tribler/gui/widgets/downloadspage.py @@ -1,5 +1,7 @@ +import logging import os import time +from typing import Optional from PyQt5.QtCore import QTimer, QUrl, Qt, pyqtSignal from PyQt5.QtGui import QDesktopServices @@ -53,6 +55,7 @@ class DownloadsPage(AddBreadcrumbOnShowMixin, QWidget): def __init__(self): QWidget.__init__(self) + self._logger = logging.getLogger(self.__class__.__name__) self.export_dir = None self.filter = DOWNLOADS_FILTER_ALL self.download_widgets = {} # key: infohash, value: QTreeWidgetItem @@ -62,7 +65,8 @@ def __init__(self): self.downloads_last_update = 0 self.selected_items = [] self.dialog = None - self.loading_message_widget = None + self.loading_message_widget: Optional[LoadingDownloadWidgetItem] = None + self.loading_list_item: Optional[LoadingListItem] = None self.total_download = 0 self.total_upload = 0 @@ -109,10 +113,9 @@ def on_filter_text_changed(self, text): def start_loading_downloads(self): self.window().downloads_list.setSelectionMode(QAbstractItemView.NoSelection) self.loading_message_widget = LoadingDownloadWidgetItem() + self.loading_list_item = LoadingListItem(self.window().downloads_list) self.window().downloads_list.addTopLevelItem(self.loading_message_widget) - self.window().downloads_list.setItemWidget( - self.loading_message_widget, 2, LoadingListItem(self.window().downloads_list) - ) + self.window().downloads_list.setItemWidget(self.loading_message_widget, 2, self.loading_list_item) self.schedule_downloads_timer(now=True) def schedule_downloads_timer(self, now=False): @@ -148,6 +151,20 @@ def load_downloads(self): def on_received_downloads(self, downloads): if not downloads or "downloads" not in downloads: return # This might happen when closing Tribler + + checkpoints = downloads.get('checkpoints', {}) + if checkpoints and self.loading_message_widget: + # If not all checkpoints are loaded, display the number of the loaded checkpoints + total = checkpoints['total'] + loaded = checkpoints['loaded'] + if not checkpoints['all_loaded']: + # The column is too narrow for a long message, probably we should redesign this UI element later + message = f'{loaded}/{total} checkpoints' + self._logger.info(f'Loading checkpoints: {message}') + self.loading_list_item.textlabel.setText(message) + self.schedule_downloads_timer() + return + loading_widget_index = self.window().downloads_list.indexOfTopLevelItem(self.loading_message_widget) if loading_widget_index > -1: self.window().downloads_list.takeTopLevelItem(loading_widget_index) From d10f724a626cd8a05cc77b1c90a717cbae53ab11 Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Wed, 7 Sep 2022 15:11:17 +0200 Subject: [PATCH 5/6] Add a constant for `start` task name --- .../libtorrent/download_manager/download_manager.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tribler/core/components/libtorrent/download_manager/download_manager.py b/src/tribler/core/components/libtorrent/download_manager/download_manager.py index fdbe56f4380..e57a6dc95f7 100644 --- a/src/tribler/core/components/libtorrent/download_manager/download_manager.py +++ b/src/tribler/core/components/libtorrent/download_manager/download_manager.py @@ -67,6 +67,7 @@ def encode_atp(atp): class DownloadManager(TaskManager): + START_TASK = "start" def __init__(self, state_dir, @@ -166,7 +167,7 @@ def initialize(self): self.set_download_states_callback(self.sesscb_states_callback) def start(self): - self.register_task("start", self._start) + self.register_task(self.START_TASK, self._start) async def _start(self): await self.load_checkpoints() @@ -180,7 +181,7 @@ def notify_shutdown_state(self, state): self.notifier[notifications.tribler_shutdown_state](state) async def shutdown(self, timeout=30): - self.cancel_pending_task("start") + self.cancel_pending_task(self.START_TASK) self.cancel_pending_task("download_states_lc") if self.downloads: self.notify_shutdown_state("Checkpointing Downloads...") From d5887854b2bd31998953f390ae24c54874e7a2ee Mon Sep 17 00:00:00 2001 From: Alexander Kozlovsky Date: Thu, 8 Sep 2022 17:55:29 +0200 Subject: [PATCH 6/6] Introduce constants for JSON keys as requested by @drew2a --- .../libtorrent/restapi/downloads_endpoint.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py b/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py index c1170cd923e..2759e1e8558 100644 --- a/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py +++ b/src/tribler/core/components/libtorrent/restapi/downloads_endpoint.py @@ -38,6 +38,11 @@ from tribler.core.utilities.utilities import froze_it +TOTAL = 'total' +LOADED = 'loaded' +ALL_LOADED = 'all_loaded' + + def _safe_extended_peer_info(ext_peer_info): """ Given a string describing peer info, return a json.dumps() safe representation. @@ -221,9 +226,9 @@ def get_files_info_json(download): 'time_added': Integer }), 'checkpoints': schema(Checkpoints={ - 'total': Integer, - 'loaded': Integer, - 'all_loaded': Boolean, + TOTAL: Integer, + LOADED: Integer, + ALL_LOADED: Boolean, }) }), } @@ -243,9 +248,9 @@ async def get_downloads(self, request): get_files = params.get('get_files', '0') == '1' checkpoints = { - "total": self.download_manager.checkpoints_count, - "loaded": self.download_manager.checkpoints_loaded, - "all_loaded": self.download_manager.all_checkpoints_are_loaded, + TOTAL: self.download_manager.checkpoints_count, + LOADED: self.download_manager.checkpoints_loaded, + ALL_LOADED: self.download_manager.all_checkpoints_are_loaded, } if not self.download_manager.all_checkpoints_are_loaded: