Skip to content

Commit 7ef32f1

Browse files
committed
fix: handle resource downloads whose target directory got deleted
1 parent 3653ab9 commit 7ef32f1

5 files changed

Lines changed: 84 additions & 17 deletions

File tree

CHANGELOG

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
1.0.1
2+
- fix: handle resource downloads whose target directory got deleted
13
1.0.0
24
- fix: do not attempt to updates non-existent eternal jobs on exit
35
- enh: allow users to specify circle IDs as well as names during upload

dcoraid/download/job.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
]
3131

3232

33+
class InvalidDownloadDirectoryError(BaseException):
34+
"""Used for signaling a non-existent download directory"""
35+
36+
3337
class DownloadJob:
3438
def __init__(self, api, resource_id, download_path, condensed=False):
3539
"""Wrapper for resource downloads
@@ -155,7 +159,13 @@ def get_dataset_dict(self):
155159
return self._dataset_dict
156160

157161
def get_download_path(self):
158-
"""Return the final location to which the file is downloaded"""
162+
"""Return the final location to which the file is downloaded
163+
164+
Raises
165+
------
166+
InvalidDownloadDirectoryError
167+
When the target directory does not exist
168+
"""
159169
if self._download_path is None:
160170
if self._user_path.is_dir():
161171
# Compute the resource path from the dataset dictionary
@@ -178,7 +188,7 @@ def get_download_path(self):
178188
# user specified an actual file
179189
self._download_path = self._user_path
180190
else:
181-
raise ValueError(
191+
raise InvalidDownloadDirectoryError(
182192
f"The `download_path` passed in __init__ is invalid. "
183193
f"Please make sure the target directory for "
184194
f"{self._user_path} exists.")
@@ -309,7 +319,7 @@ def get_status(self):
309319
data["bytes local"] = size_temp
310320
else:
311321
data["bytes local"] = 0
312-
except api_errors.APINotFoundError:
322+
except (api_errors.APINotFoundError, InvalidDownloadDirectoryError):
313323
# The user likely tried to download the file from a different
314324
# host or an evil admin deleted a file.
315325
self.traceback = traceback.format_exc()
@@ -326,7 +336,11 @@ def get_status(self):
326336
def retry_download(self):
327337
"""Retry downloading resources when an error occured"""
328338
if self.state in ["abort", "error"]:
339+
# Create the target download directory
340+
if self._user_path is not None:
341+
self._user_path.parent.mkdir(parents=True, exist_ok=True)
329342
self.set_state("init")
343+
self.traceback = None
330344
else:
331345
raise ValueError("Can only retry download in error state!")
332346

dcoraid/gui/panel_downloads/widget_actions_download.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def on_error(self):
5151
+ "<a href='https://github.com/DCOR-dev/DCOR-Aid/issues'>"
5252
+ "create an issue online</a>. Click on the button below "
5353
"to see details required for fixing the problem.")
54-
msg.setWindowTitle(f"Job {self.job.dataset_id[:5]} error")
54+
msg.setWindowTitle(f"Job {self.job.job_id[:5]} error")
5555
msg.setDetailedText(self.job.traceback)
5656
msg.exec()
5757

dcoraid/gui/panel_downloads/widget_download.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -231,23 +231,32 @@ def update_job_status(self):
231231
for row, job in enumerate(self.jobs):
232232
try:
233233
status = job.get_status()
234+
state = status["state"]
234235
self.set_label_item(row, 0, job.job_id[:5])
235-
try:
236-
title = get_download_title(job)
237-
except BaseException:
238-
logger.error(traceback.format_exc())
239-
# Probably a connection error
240-
title = "-- error getting dataset title --"
241-
self.set_label_item(row, 1, title)
242-
self.set_label_item(row, 2, status["state"])
243-
self.set_label_item(row, 3, job.get_progress_string())
244-
self.set_label_item(row, 4, job.get_rate_string())
245-
self.set_actions_item(row, 5, job)
246-
if status["state"] == "done":
247-
self.on_download_finished(job.job_id)
248236
except BaseException:
237+
logger.error(traceback.format_exc())
249238
job.set_state("error")
250239
job.traceback = traceback.format_exc()
240+
state = "error"
241+
242+
try:
243+
title = get_download_title(job)
244+
progress = job.get_progress_string()
245+
rate = job.get_rate_string()
246+
except BaseException:
247+
logger.error(traceback.format_exc())
248+
# Probably a connection error
249+
title = "-- error getting dataset title --"
250+
progress = "unknown"
251+
rate = "unknown"
252+
253+
self.set_label_item(row, 1, title)
254+
self.set_label_item(row, 2, state)
255+
self.set_label_item(row, 3, progress)
256+
self.set_label_item(row, 4, rate)
257+
self.set_actions_item(row, 5, job)
258+
if state == "done":
259+
self.on_download_finished(job.job_id)
251260

252261
QtWidgets.QApplication.processEvents(
253262
QtCore.QEventLoop.ProcessEventsFlag.AllEvents, 300)

tests/test_download_job.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import shutil
12
import tempfile
23
from unittest import mock
34
import uuid
@@ -21,6 +22,47 @@ def test_initialize():
2122
assert dj.state == "init"
2223

2324

25+
def test_download_deleted_directory():
26+
api = common.get_api()
27+
td = tempfile.mkdtemp(prefix="test-download")
28+
ds_dict = common.make_dataset_for_download()
29+
dj = job.DownloadJob(api=api,
30+
resource_id=ds_dict["resources"][0]["id"],
31+
download_path=td)
32+
# sanity check
33+
assert dj.get_status()["bytes total"] == 899298
34+
dj_state = dj.__getstate__()
35+
36+
# delete the job and its directory
37+
path_dir = dj.path_dir
38+
del dj
39+
shutil.rmtree(path_dir)
40+
41+
# create a new job
42+
dj2 = job.DownloadJob(api=api, **dj_state)
43+
44+
with pytest.raises(job.InvalidDownloadDirectoryError):
45+
dj2.get_download_path()
46+
47+
assert np.isnan(dj2.get_status()["bytes total"])
48+
assert dj2.traceback
49+
50+
# retry
51+
dj2.retry_download()
52+
assert dj2.get_status()["bytes total"] == 899298
53+
54+
# now attempt a resume-download
55+
# create a truncated temporary file
56+
dj2.task_download_resource()
57+
dj2.task_verify_resource()
58+
assert not dj2.traceback
59+
assert dj2.start_time is not None
60+
assert dj2.end_time is not None
61+
assert dj2.path.exists()
62+
assert dj2.file_bytes_downloaded == 899298
63+
assert dj2.file_size == 899298
64+
65+
2466
def test_download_resume():
2567
api = common.get_api()
2668
td = tempfile.mkdtemp(prefix="test-download")

0 commit comments

Comments
 (0)