From 0589dd171a9d1bf711369bbb794ae54c8c2b117a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20M=C3=BCller?= Date: Thu, 21 Nov 2024 00:00:22 +0100 Subject: [PATCH] fix: downloads tab became unresponsive for > 100 downloads --- CHANGELOG | 2 + dcoraid/download/job.py | 64 ++++++++++++++----------- dcoraid/gui/download/widget_download.py | 51 ++++++++++++-------- 3 files changed, 69 insertions(+), 48 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 41138ed..129f2c8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ +0.15.5 + - fix: downloads tab became unresponsive for > 100 downloads 0.15.4 - fix: check for already uploaded resources before attempting to compress - fix: timeout parameter not passed to `CKANAPI.get` diff --git a/dcoraid/download/job.py b/dcoraid/download/job.py index caa1b99..1bbf1f8 100644 --- a/dcoraid/download/job.py +++ b/dcoraid/download/job.py @@ -12,7 +12,7 @@ import requests from ..api import errors as api_errors -from ..common import etagsum, sha256sum, weak_lru_cache +from ..common import etagsum, sha256sum logger = logging.getLogger(__name__) @@ -53,6 +53,9 @@ def __init__(self, api, resource_id, download_path, condensed=False): condensed: bool Whether to download the condensed dataset """ + self._resource_dict = None + self._dataset_dict = None + self._download_path = None self.api = api.copy() # create a copy of the API self.resource_id = resource_id self.job_id = resource_id + ("_cond" if condensed else "") @@ -117,43 +120,46 @@ def file_size(self): def id(self): return self.resource_id - @weak_lru_cache(maxsize=100) def get_resource_dict(self): """Return resource dictionary""" - return self.api.get("resource_show", id=self.resource_id) + if self._resource_dict is None: + self._resource_dict = self.api.get("resource_show", + id=self.resource_id) + return self._resource_dict - @weak_lru_cache(maxsize=100) def get_dataset_dict(self): - res_dict = self.get_resource_dict() - ds_dict = self.api.get("package_show", id=res_dict["package_id"]) - return ds_dict + if self._dataset_dict is None: + res_dict = self.get_resource_dict() + self._dataset_dict = self.api.get("package_show", + id=res_dict["package_id"]) + return self._dataset_dict - @weak_lru_cache() def get_download_path(self): """Return the final location to which the file is downloaded""" - if self._user_path.is_dir(): - # Compute the resource path from the dataset dictionary - rsdict = self.get_resource_dict() - ds_name = self.get_dataset_dict()["name"] - # append dataset name to user-specified download directory - ds_dir = self._user_path / ds_name - ds_dir.mkdir(parents=True, exist_ok=True) - if self.condensed and rsdict["mimetype"] == "RT-DC": - stem, suffix = rsdict["name"].rsplit(".", 1) - res_name = stem + "_condensed." + suffix + if self._download_path is None: + if self._user_path.is_dir(): + # Compute the resource path from the dataset dictionary + rsdict = self.get_resource_dict() + ds_name = self.get_dataset_dict()["name"] + # append dataset name to user-specified download directory + ds_dir = self._user_path / ds_name + ds_dir.mkdir(parents=True, exist_ok=True) + if self.condensed and rsdict["mimetype"] == "RT-DC": + stem, suffix = rsdict["name"].rsplit(".", 1) + res_name = stem + "_condensed." + suffix + else: + res_name = rsdict["name"] + self._download_path = ds_dir / res_name + elif self._user_path.parent.is_dir(): + # user specified an actual file + self._download_path = self._user_path else: - res_name = rsdict["name"] - dl_path = ds_dir / res_name - elif self._user_path.parent.is_dir(): - # user specified an actual file - dl_path = self._user_path - else: - raise ValueError( - f"The `download_path` passed in __init__ is invalid. " - f"Please make sure the target directory for {self._user_path} " - f"exists.") + raise ValueError( + f"The `download_path` passed in __init__ is invalid. " + f"Please make sure the target directory for " + f"{self._user_path} exists.") - return dl_path + return self._download_path def get_resource_url(self): """Return a link to the resource on DCOR""" diff --git a/dcoraid/gui/download/widget_download.py b/dcoraid/gui/download/widget_download.py index 06e10b6..4a98f56 100644 --- a/dcoraid/gui/download/widget_download.py +++ b/dcoraid/gui/download/widget_download.py @@ -17,6 +17,9 @@ from ..tools import show_wait_cursor +logger = logging.getLogger(__name__) + + class DownloadWidget(QtWidgets.QWidget): download_finished = QtCore.pyqtSignal() @@ -81,8 +84,8 @@ class DownloadTableWidget(QtWidgets.QTableWidget): download_finished = QtCore.pyqtSignal() def __init__(self, *args, **kwargs): + self._busy_updating_widgets = False super(DownloadTableWidget, self).__init__(*args, **kwargs) - self.logger = logging.getLogger(__name__) self.jobs = [] # Will become DownloadQueue with self.set_job_list settings = QtCore.QSettings() @@ -127,40 +130,50 @@ def update_job_status(self): """Update UI with information from self.jobs (DownloadJobList)""" if not self.parent().parent().isVisible(): return - # disable updates - self.setUpdatesEnabled(False) + if self._busy_updating_widgets: + return + self._busy_updating_widgets = True + # make sure the length of the table is long enough self.setRowCount(len(self.jobs)) self.setColumnCount(6) for row, job in enumerate(self.jobs): - status = job.get_status() - self.set_label_item(row, 0, job.job_id[:5]) - try: - title = get_download_title(job) - except BaseException: - self.logger.error(traceback.format_exc()) - # Probably a connection error - title = "-- error getting dataset title --" - self.set_label_item(row, 1, title) - self.set_label_item(row, 2, status["state"]) - self.set_label_item(row, 3, job.get_progress_string()) - self.set_label_item(row, 4, job.get_rate_string()) - if status["state"] == "done": - self.on_download_finished(job.job_id) + if job.job_id in self._finished_downloads: + # Widgets of finished downloads have already been drawn. + continue try: + status = job.get_status() + self.set_label_item(row, 0, job.job_id[:5]) + try: + title = get_download_title(job) + except BaseException: + logger.error(traceback.format_exc()) + # Probably a connection error + title = "-- error getting dataset title --" + self.set_label_item(row, 1, title) + self.set_label_item(row, 2, status["state"]) + self.set_label_item(row, 3, job.get_progress_string()) + self.set_label_item(row, 4, job.get_rate_string()) + if status["state"] == "done": + logger.info(f"Download {job.job_id} finished") + self.on_download_finished(job.job_id) self.set_actions_item(row, 5, job) except BaseException: job.set_state("error") job.traceback = traceback.format_exc() + QtWidgets.QApplication.processEvents( + QtCore.QEventLoop.AllEvents, + 300) + # spacing (did not work in __init__) header = self.horizontalHeader() header.setSectionResizeMode( 0, QtWidgets.QHeaderView.ResizeToContents) header.setSectionResizeMode(1, QtWidgets.QHeaderView.Stretch) - # enable updates again - self.setUpdatesEnabled(True) + + self._busy_updating_widgets = False def set_label_item(self, row, col, label): """Get/Create a Qlabel at the specified position