Add CORS headers to HTTP exceptions - #16339
Merged
Merged
Conversation
Carreau
approved these changes
Jul 29, 2024
Carreau
left a comment
Carreau
left a comment
Contributor
There was a problem hiding this comment.
Thanks for looking into this, that is great !
I haven't tested locally yet though.
Contributor
Author
|
@di Can I ask for your review when you have time? |
miketheman
reviewed
Aug 16, 2024
miketheman
approved these changes
Aug 19, 2024
Contributor
|
Thanks ! |
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Aug 21, 2024
Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Aug 21, 2024
Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 16, 2024
Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 18, 2024
Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 18, 2024
Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 19, 2024
Currently only the not_in_pyodide will raise on non-success, because this is the default behavior of urllib, the in_pyodide will not, so I added a raise_for_status. It is better to raise, as otherwise the package parser will potentially get proper URL and not manage to parse it, and decide there is no wheels, while we actually just got an error (404, or maybe 500). In addition wraps both case in a custom local HttpStatusError, so that we can actually catch these errors in the right places when we encounter them. Also add handling for PyPI 404 Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 19, 2024
Currently only the not_in_pyodide will raise on non-success, because this is the default behavior of urllib, the in_pyodide will not, so I added a raise_for_status. It is better to raise, as otherwise the package parser will potentially get proper URL and not manage to parse it, and decide there is no wheels, while we actually just got an error (404, or maybe 500). In addition wraps both case in a custom local HttpStatusError, so that we can actually catch these errors in the right places when we encounter them. Also add handling for PyPI 404 Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 19, 2024
Currently only the not_in_pyodide will raise on non-success, because this is the default behavior of urllib, the in_pyodide will not, so I added a raise_for_status. It is better to raise, as otherwise the package parser will potentially get proper URL and not manage to parse it, and decide there is no wheels, while we actually just got an error (404, or maybe 500). In addition wraps both case in a custom local HttpStatusError, so that we can actually catch these errors in the right places when we encounter them. Also add handling for PyPI 404 Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
Carreau
added a commit
to Carreau/micropip
that referenced
this pull request
Sep 20, 2024
Currently only the not_in_pyodide will raise on non-success, because this is the default behavior of urllib, the in_pyodide will not, so I added a raise_for_status. It is better to raise, as otherwise the package parser will potentially get proper URL and not manage to parse it, and decide there is no wheels, while we actually just got an error (404, or maybe 500). In addition wraps both case in a custom local HttpStatusError, so that we can actually catch these errors in the right places when we encounter them. Also add handling for PyPI 404 Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors.
ryanking13
added a commit
to pyodide/micropip
that referenced
this pull request
Oct 5, 2024
* fetch_string_and_headers compat: raise in and out of pyodide Currently only the not_in_pyodide will raise on non-success, because this is the default behavior of urllib, the in_pyodide will not, so I added a raise_for_status. It is better to raise, as otherwise the package parser will potentially get proper URL and not manage to parse it, and decide there is no wheels, while we actually just got an error (404, or maybe 500). In addition wraps both case in a custom local HttpStatusError, so that we can actually catch these errors in the right places when we encounter them. Also add handling for PyPI 404 Now that warehouse set cors to 404, (pypi/warehouse#16339) we need to change the checked exceptions as there is no more network errors. * Remove compat with Pyodide < 0.27 * Update test_compat.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Gyeongjae Choi <def6488@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolve #14229
This adds CORS headers to the response when warehouse returns404, 410, 50x responses.
I added the same CORS headers to simple APIs in #13222, but they were not applied on an error case.
Context
In pyodide, we are implementing a fallback option for package index, so that if a package is not available in one package index, then search it in another package index. However, without the CORS headers, it is impossible to distinguish the 404 Not Found error.
cc: @Carreau