From 96cd87ebb1fd9cff9c1ca1dd471c430daf27f03b Mon Sep 17 00:00:00 2001 From: Damien Goutte-Gattat Date: Sat, 23 May 2026 16:16:07 +0100 Subject: [PATCH 1/4] Do not error out if the first download attempt fails for any reason. This commit reorders the exception handling logic within the `odk-helper download` command. We catch download errors _inside_ the loop on download attempts, rather than _outside_, so that a failed download attempt (for any reason at all) does not automatically cause the command to error out without even trying the remaining attempts. Only when the _last_ attempt fails do we actually error out. This fixes INCATools/ontology-development-kit#1349. --- src/incatools/odk/download.py | 3 --- src/incatools/odk/helper.py | 13 ++++++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/incatools/odk/download.py b/src/incatools/odk/download.py index 020ea9b..c11d2e8 100644 --- a/src/incatools/odk/download.py +++ b/src/incatools/odk/download.py @@ -161,9 +161,6 @@ def download_file( elif response.status_code == 304: logging.info(f"{output.name}: Not modified at {url}") return 304 - elif response.status_code == 404: - logging.warning(f"{output.name}: Not found at {url}") - return 404 elif response.status_code in RETRIABLE_HTTP_ERRORS and n_try < max_retry: n_try += 1 logging.warning( diff --git a/src/incatools/odk/helper.py b/src/incatools/odk/helper.py index 83c7aa9..d50c3da 100644 --- a/src/incatools/odk/helper.py +++ b/src/incatools/odk/helper.py @@ -204,15 +204,14 @@ def download(url, output, reference, cache_info, max_retry, try_gzip) -> None: if reference != output and output.exists(): output.unlink() - try: - for u, c in attempts: - status = download_file(u, output, info, max_retry, c) + for i, attempt in enumerate(attempts): + try: + status = download_file(attempt[0], output, info, max_retry, attempt[1]) if status == 200: info.to_file(cache_info) return elif status == 304: return - if status == 404: # Last attempt failed - raise click.ClickException(f"Cannot download {url}: 404 Not Found") - except DownloadError as e: - raise click.ClickException(f"Cannot download {url}: {e}") + except DownloadError as e: + if i == len(attempt) - 1: + raise click.ClickException(f"Cannot download {url}: {e}") From 9f3b0a9d58720f0134242a9ddbb77f373ef51d5b Mon Sep 17 00:00:00 2001 From: Damien Goutte-Gattat Date: Tue, 26 May 2026 11:39:44 +0100 Subject: [PATCH 2/4] Typo fix. --- src/incatools/odk/helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/incatools/odk/helper.py b/src/incatools/odk/helper.py index d50c3da..f08be68 100644 --- a/src/incatools/odk/helper.py +++ b/src/incatools/odk/helper.py @@ -213,5 +213,5 @@ def download(url, output, reference, cache_info, max_retry, try_gzip) -> None: elif status == 304: return except DownloadError as e: - if i == len(attempt) - 1: + if i == len(attempts) - 1: raise click.ClickException(f"Cannot download {url}: {e}") From 4d205d17bc927db5c35085aee5e89f167c73b2aa Mon Sep 17 00:00:00 2001 From: Damien Goutte-Gattat Date: Tue, 26 May 2026 12:17:19 +0100 Subject: [PATCH 3/4] Emit log messages when downloading. Update the `odk-helper download` command to emit: * one INFO message indicating the URL that we are currently trying to download from; * one WARNING message if we fail to download from one particular URL, unless that URL was the last URL to try (in which case we error out instead of logging a warning). --- src/incatools/odk/helper.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/incatools/odk/helper.py b/src/incatools/odk/helper.py index f08be68..754a0f5 100644 --- a/src/incatools/odk/helper.py +++ b/src/incatools/odk/helper.py @@ -206,6 +206,7 @@ def download(url, output, reference, cache_info, max_retry, try_gzip) -> None: for i, attempt in enumerate(attempts): try: + logging.info(f"{output.name}: Trying download from <{attempt[0]}>") status = download_file(attempt[0], output, info, max_retry, attempt[1]) if status == 200: info.to_file(cache_info) @@ -215,3 +216,7 @@ def download(url, output, reference, cache_info, max_retry, try_gzip) -> None: except DownloadError as e: if i == len(attempts) - 1: raise click.ClickException(f"Cannot download {url}: {e}") + else: + logging.warning( + f"{output.name}: Download failed from <{attempt[0]}>: {e}" + ) From d72c8d8dd20b612b8c3a30ad04434862065aa542 Mon Sep 17 00:00:00 2001 From: Damien Goutte-Gattat Date: Tue, 26 May 2026 14:17:02 +0100 Subject: [PATCH 4/4] Include the HTTP status code when reporting an error. --- src/incatools/odk/download.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/incatools/odk/download.py b/src/incatools/odk/download.py index c11d2e8..2db3fa2 100644 --- a/src/incatools/odk/download.py +++ b/src/incatools/odk/download.py @@ -181,8 +181,10 @@ def download_file( raise DownloadError(f"Timeout when connecting to {hostname}") except requests.exceptions.ConnectionError: raise DownloadError(f"Cannot connect to {hostname}") - except requests.exceptions.HTTPError: - raise DownloadError(f"HTTP error when downloading {url}") + except requests.exceptions.HTTPError as e: + raise DownloadError( + f"HTTP {e.response.status_code} error when downloading {url}" + ) except requests.exceptions.ReadTimeout: raise DownloadError(f"Timeout when downloading {url}")