micropip icon indicating copy to clipboard operation
micropip copied to clipboard

Should package index support "text/html; charset=utf-8" ?

Open Carreau opened this issue 1 year ago • 2 comments

I think that this line:

https://github.com/pyodide/micropip/blob/main/micropip/package_index.py#L227

Should likely be modified to:

-        case "application/vnd.pypi.simple.v1+html" | "text/html":
+        case "application/vnd.pypi.simple.v1+html" | "text/html" | "text/html; charset=utf-8":
            return partial(ProjectInfo.from_simple_html_api, pkgname=pkgname)

Or maybe be properly parsed ? because there might be more charset ?

It seem also that those kind of errors where reaching/parsing an index fails are swallowed (code behave as if the wheel was not found). I think the behavior should be stricter and if the index is wrong/malformatted unreachable we should errors and not behave as if the wheel was not found.

Carreau avatar Jul 18 '24 14:07 Carreau

Or maybe be properly parsed

Sounds reasonable to me. Could you open a PR?

It seem also that those kind of errors where reaching/parsing an index fails are swallowed

Huh, it is not the behavior that I intended and I agree that it should show a more proper error message. I thought that the exception here:

https://github.com/pyodide/micropip/blob/5d4a8696f7daf302f060ffded34f9a84455fcffc/micropip/package_index.py#L230C13-L230C74

would be propagated to the user when the content type is not valid, but it doesn't work as expected, I guess?

ryanking13 avatar Jul 19 '24 07:07 ryanking13

Yeah, i need to re-dive into why it's not propagating exactly , but IIRC if it fails for any reason to find a package in the index it falls back into looking in the lock file. Which make sense if the wheel is not found.

I think that returning enums instead of raising in many places could help, but it would affect a large number of public APIs

Carreau avatar Jul 19 '24 09:07 Carreau

This works now.

Carreau avatar Nov 06 '24 09:11 Carreau