Skip to content

Improve download error failures #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ehennestad wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improve download error failures #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb4354c
add error handling for failed download of versions.json
ehennestad Apr 23, 2026
40db2dd
Create .gitignore
ehennestad Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
versions.json
19 changes: 18 additions & 1 deletion openMINDS_validation/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import re
import shutil
import json
import ssl
import urllib.request
import urllib.error

Expand All @@ -19,6 +20,10 @@

_remote_schema_cache = {}


class DownloadError(RuntimeError):
pass
Comment on lines +24 to +25

@Raphael-Gazzotti Raphael-Gazzotti May 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could improve this by adding an init method that accepts url and path as part of the exception, instead of passing and handling them separately throughout the code.


class VocabManager:
def __init__(self, path_vocab_types, path_vocab_properties):
# TODO handle dev
Expand Down Expand Up @@ -50,11 +55,23 @@ def load_json(file_path):
json_file = json.load(f)
return json_file

def _format_download_error(url, path, error):
if isinstance(error, urllib.error.URLError) and isinstance(error.reason, ssl.SSLCertVerificationError):
reason = "SSL certificate verification failed. Check your local Python certificate store."
else:
reason = str(error)
return f'Failed to download "{url}" to "{path}": {reason}'
Comment on lines +58 to +63

@Raphael-Gazzotti Raphael-Gazzotti May 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could move "_format_download_error" into DownloadError as a @classmethod, and rename it to something like from_exception, since its responsibility is focused only on producing a clear error message.


def download_file(url, path):
try:
urllib.request.urlretrieve(url, path)
except (urllib.error.URLError, IOError) as e:
logging.error(e)
has_local_fallback = os.path.exists(path) and os.path.getsize(path) > 0
message = _format_download_error(url, path, e)
if has_local_fallback:
logging.warning(f"{message} Using existing local file instead.")
return
raise DownloadError(message) from e

def clone_central(refetch:bool=False):
if refetch and os.path.exists("sources"):
Expand Down