simple-repository / simple-repository-server

A tool for running a PEP-503 simple Python package repository, including features such as dist metadata (PEP-658) and JSON API (PEP-691)
MIT License
18 stars 0 forks source link

unusable 304 logged by uv (with fix) #6

Closed bewinsnw closed 6 days ago

bewinsnw commented 1 month ago

If you run an install twice with uv, like:

$ time uv pip install -v --force-reinstall --index-url=http://localhost:9191/simple -r requirements.txt

some proxied requests that 304 are repeated without the etag to get a 200 response; uv logs this:

DEBUG Found modified response for: http://localhost:9191/resources/nwpy-awskms/nwpy_awskms-1.2.6-py3-none-any.whl.metadata
WARN Server returned unusable 304 for: http://localhost:9191/resources/nwpy-awskms/nwpy_awskms-1.2.6-py3-none-any.whl.metadata

this relates to https://github.com/astral-sh/uv/issues/1754 where uv started making the second fetch because the 304 responses similarly were somehow "not valid".

Looking at one of these:

$ curl -v -H 'if-none-match: "c8b73eca5053"' http://localhost:9191/resources/nwpy-logging/nwpy_logging-2.19.6-py3-none-any.whl.metadata
*   Trying [::1]:9191...
* Connected to localhost (::1) port 9191
> GET /resources/nwpy-logging/nwpy_logging-2.19.6-py3-none-any.whl.metadata HTTP/1.1
> Host: localhost:9191
> User-Agent: curl/8.4.0
> Accept: */*
> if-none-match: "c8b73eca5053"
> 
< HTTP/1.1 304 Not Modified
< date: Sun, 26 May 2024 00:24:55 GMT
< server: uvicorn

... there is a problem here. Reading the RFC: https://www.rfc-editor.org/rfc/rfc9110#name-304-not-modified

"The server generating a 304 response MUST generate any of the following header fields that would have been sent in a 200 response to the same request: Content-Location, Date, Etag, Vary, ..., Cache-Control and Expires"

(and no others; Last-Modified is optional but useful). Date is always returned anyway, it's compulsory and uvicorn adds it. The others were not necessarily there in the 200 response; but Etag definitely must be, since that is how we can generate a response to an if-none-match.

So, minimally, we want to add etag to any 304 responses. Testing that, I added it in a few places - this one was for a TextResource in the router in simple.py:

            if etag == request.headers.get("if-none-match"):
                raise HTTPException(304,
                    headers={
                        "etag": etag
                    }
                )
            return PlainTextResponse(
                content=resource.text,
                headers={'ETag': etag},
            )

... and this works; uv no longer complained that the 304 is invalid and doesn't send a spurious extra request.

pelson commented 1 week ago

Thanks for the clear report! Looks right to me. Handing over to @ivany4 to integrate this.

It didn't make it into the update for v0.5.0.

pelson commented 6 days ago

Fixed in v0.6.0. Please re-open if you encounter any issue.