Skip to content

Make _http_exists send the ranged GET its docstring describes - #216

Open
arpitjain099 wants to merge 1 commit into
aboutcode-org:masterfrom
arpitjain099:fix/http-exists-ranged-get
Open

Make _http_exists send the ranged GET its docstring describes#216
arpitjain099 wants to merge 1 commit into
aboutcode-org:masterfrom
arpitjain099:fix/http-exists-ranged-get

Conversation

@arpitjain099

Copy link
Copy Markdown

The docstring states the requirement exactly, and the body does the one thing it rules out:

def _http_exists(url: str) -> bool:
    """
    Lightweight existence check using a ranged GET so CDNs/servers that ignore HEAD still work.
    """
    try:
        resp = make_head_request(url, headers={"Range": "bytes=0-0"})

make_head_request is requests.head. Two things follow. The Range header is meaningless on a HEAD, so the 206 this function tests for is never observed on that path; and requests.head defaults to allow_redirects=False, so a 302 also reads as "missing".

The combination takes out the HuggingFace lane. huggingface.py accepts exactly one extension, ALLOWED_EXECUTABLE_EXTS = (".bin",), every real .bin weight file on HuggingFace is Git-LFS backed, and HF serves those from a CDN behind a 302. So the one file class the handler will return is exactly the class this check rejects.

Against a local server with three routes, positive control first so a later failure is not ambiguous:

                                          before    after
plain 200 on HEAD (positive control)      True      True
server answers 405 to HEAD, 200 to GET    False     True
server 302-redirects to a real file       False     True
genuinely missing (negative control)      False     False

The last row matters as much as the middle two: the fix does not simply make everything return True.

Confirmed against the live API too, where a non-LFS file (config.json) already returned True and the LFS weights (model.safetensors, pytorch_model.bin) returned False while a ranged GET on the same URLs answered 206.

The change uses requests.get with the range, redirects followed, and stream=True so the body is not pulled down, closing the response rather than leaving it open. I left make_head_request alone since other callers may want a genuine HEAD.

Note on testing: I could not run the suite here, pytest tests/ fails collection with ModuleNotFoundError: No module named 'pip' in the vendored fetchcode.vcs.pip tests, before and after the change alike, so it is unrelated to this. The verification above is a standalone script driving the real _http_exists.

The docstring states the requirement exactly: a ranged GET, so that servers
which ignore HEAD still work. The body calls make_head_request, which is
requests.head.

Two consequences. The Range header is meaningless on a HEAD, so the 206 the
function tests for is never observed on that path. And requests.head defaults
to allow_redirects=False, so a 302 also reads as missing.

That combination takes out the HuggingFace lane entirely: huggingface.py
accepts only .bin, every real .bin weight is Git-LFS backed, and HF serves
those from a CDN via a 302.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant