diff --git a/pym/bob/scm/url.py b/pym/bob/scm/url.py index d1d9111a..d95b254f 100644 --- a/pym/bob/scm/url.py +++ b/pym/bob/scm/url.py @@ -12,6 +12,7 @@ from http.client import HTTPException from abc import abstractmethod import asyncio +import errno import concurrent.futures.process import contextlib import hashlib @@ -512,7 +513,7 @@ def _download(self, url, destination, mode): if "content-length" in rsp.info(): expected = int(rsp.info()["Content-Length"]) if expected > read: - return False, "Response too short: {} < {} (bytes)".format(read, expected) + return False, "Response too short: {} < {} (bytes)".format(read, expected), False # Atomically move file to destination. Set mode to 0600 to # retain Bob 0.15 behaviour if no explicit mode was configured. @@ -522,12 +523,14 @@ def _download(self, url, destination, mode): except urllib.error.HTTPError as e: if e.code != 304: - return False, "HTTP error {}: {}".format(e.code, e.reason) + return False, "HTTP error {}: {}".format(e.code, e.reason), False else: # HTTP 304 Not modifed -> local file up-to-date - return False, None + return False, None, False except HTTPException as e: - return False, "HTTP error: " + str(e) + return False, "HTTP error: " + str(e), False + except OSError as e: + return False, "Failed to download: " + str(e), e.errno == errno.ENOSPC finally: if tmpFileName is not None: os.remove(tmpFileName) @@ -535,7 +538,7 @@ def _download(self, url, destination, mode): # to prevent ugly backtraces when user presses ctrl+c. signal.signal(signal.SIGINT, signal.SIG_DFL) - return True, None + return True, None, False async def _fetch(self, invoker, url, destination, mode): if url.scheme in ['', 'file']: @@ -565,9 +568,11 @@ async def _fetch(self, invoker, url, destination, mode): while True: invoker.trace("", url.geturl(), ">", destination, "retires:", retries) try: - updated, err = await invoker.runInExecutor(UrlScm._download, self, url, destination, mode) + updated, err, fatal = await invoker.runInExecutor(UrlScm._download, self, url, destination, mode) if err: - if retries == 0: + if fatal: + invoker.fail(err) + elif retries == 0: return False, err else: invoker.warn(err) diff --git a/test/unit/test_input_urlscm.py b/test/unit/test_input_urlscm.py index a2957c00..475eece2 100644 --- a/test/unit/test_input_urlscm.py +++ b/test/unit/test_input_urlscm.py @@ -7,6 +7,9 @@ from unittest import TestCase, skipIf from unittest.mock import MagicMock, patch import asyncio +import contextlib +import errno +import concurrent.futures import os, stat import subprocess import shutil @@ -54,8 +57,9 @@ def __exit__(self, exception_type, exception_value, exception_traceback): class UrlScmExecutor: - def invokeScm(self, workspace, scm, switch=False, oldScm=None, workspaceCreated=False): - executor = getProcessPoolExecutor() + def invokeScm(self, workspace, scm, switch=False, oldScm=None, workspaceCreated=False, executor=None): + if executor is None: + executor = getProcessPoolExecutor() try: spec = MagicMock(workspaceWorkspacePath=workspace, envWhiteList=set()) invoker = Invoker(spec, False, True, True, True, True, False, @@ -341,6 +345,27 @@ def testNoResponse(self): with self.assertRaises(InvocationError): self.invokeScm(workspace, scm) + @patch('signal.signal') + def testDownloadDiskFull(self, signalMock): + """Full disk directly terminates downloads""" + tmp = MagicMock() + tmp.name = None + tmp.write.side_effect = OSError(errno.ENOSPC, "Disk full") + ntf = MagicMock(return_value=contextlib.nullcontext(enter_result=tmp)) + + with (HttpServerMock(self.dir) as srv, + TemporaryWorkspace() as workspace, + patch('tempfile.NamedTemporaryFile', ntf)): + scm = self.createUrlScm({ + "url" : "http://localhost:{}/test.txt".format(srv.port), + "retries" : 3 + }) + + with self.assertRaises(InvocationError): + self.invokeScm(workspace, scm, executor=concurrent.futures.ThreadPoolExecutor()) + + self.assertEqual(srv.getRequests, 1, "URL SCM does not retry on disk full errors") + class TestExtraction(TestCase): @classmethod