Skip to content

Add certifi SSL fallback for urlopen - #3363

Closed
C-Achard wants to merge 2 commits into
mainfrom
cy/fix-win-ssl-ci-failure
Closed

Add certifi SSL fallback for urlopen#3363
C-Achard wants to merge 2 commits into
mainfrom
cy/fix-win-ssl-ci-failure

Conversation

@C-Achard

Copy link
Copy Markdown
Collaborator

Motivation

The latest Ci runner on Windows seems to be having issues with the default SSL context, currently it fails while loading certificates from the Windows certificate store with

ssl.SSLError: [ASN1: NOT_ENOUGH_DATA] not enough data

Fix

Import ssl and add _urlopen_with_certifi_fallback in tests/conftest.py to handle ssl.SSLError by falling back to an SSLContext using certifi's CA bundle. Replace direct urllib.request.urlopen call in unzip_from_url with the helper so test data downloads succeed on systems with incomplete SSL trust stores; re-raise the original error if certifi is not available.

### Motivation

The latest Ci runner on Windows seems to be having issues with the default SSL context, currently it fails while loading certificates from the Windows certificate store with

```python
ssl.SSLError: [ASN1: NOT_ENOUGH_DATA] not enough data
```

### Fix

Import ssl and add _urlopen_with_certifi_fallback in tests/conftest.py to handle ssl.SSLError by falling back to an SSLContext using certifi's CA bundle. Replace direct urllib.request.urlopen call in unzip_from_url with the helper so test data downloads succeed on systems with incomplete SSL trust stores; re-raise the original error if certifi is not available.
@C-Achard
C-Achard requested a review from deruyter92 June 10, 2026 19:16
@C-Achard C-Achard self-assigned this Jun 10, 2026
@C-Achard C-Achard added bug fix! fix for a real buggy one... CI Related to CI/CD jobs and automated testing labels Jun 10, 2026
@C-Achard

Copy link
Copy Markdown
Collaborator Author

This requires broader patching as in c494945, since the failure affects everything including torch hub downloads. We might want to revert this patch once a fix is released for Win runners,

@deruyter92

Copy link
Copy Markdown
Collaborator

Good fix for now! Let's merge as soon as possible so we can use CI

@C-Achard

Copy link
Copy Markdown
Collaborator Author

@deruyter92 I think this is fixed now since CI works in other PRs?

@C-Achard C-Achard added invalid This doesn't seem right and removed high-priority labels Jun 12, 2026
@C-Achard C-Achard closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix! fix for a real buggy one... CI Related to CI/CD jobs and automated testing invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants