Skip to content

bpo-40321: Support HTTP response status code 308 in urllib.request #19588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 6, 2021
Merged

bpo-40321: Support HTTP response status code 308 in urllib.request #19588

merged 4 commits into from
Oct 6, 2021

Conversation

jschulenklopper
Copy link
Contributor

@jschulenklopper jschulenklopper commented Apr 18, 2020

Add support for HTTP response status code 308 (permanent redirect), the 307-variant of 301 as defined in IETF RFC 7538. The http library seems to support it, the urllib.request library missed it yet.

https://bugs.python.org/issue40321

HTTP response status code 308 is defined in https://tools.ietf.org/html/rfc7538 to be the permanent redirect variant of 307 (temporary redirect).
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@jschulenklopper

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@jschulenklopper
Copy link
Contributor Author

Contributor Agreement has been signed in the mean time.

@csabella
Copy link
Contributor

@jschulenklopper, it seems that the bot can't find the contributor agreement. Can you check that the github name is the same that you've used on this PR? Thanks.

@jschulenklopper
Copy link
Contributor Author

@jschulenklopper, it seems that the bot can't find the contributor agreement. Can you check that the github name is the same that you've used on this PR? Thanks.

Ah, the GitHub user name just wasn't included on the profile in b.p.o. Fixed now.

@csabella csabella requested a review from orsenthil May 24, 2020 20:08
@balki
Copy link

balki commented Mar 22, 2021

I faced the error today and was surprised it is not already supported. I hope this is merged soon! 👍

urllib.error.HTTPError: HTTP Error 308: Permanent Redirect

@rolandcrosby
Copy link
Contributor

@csabella @orsenthil, does anything need to be changed for this to get merged? I ran into this issue when using urllib today and would love to see a fix make it into the standard library eventually.

I see the NEWS.d CI check failed - is that all that is blocking this? If so, I would be happy to add a blurb if @jschulenklopper is unavailable.

@jschulenklopper
Copy link
Contributor Author

jschulenklopper commented Jul 22, 2021

I see the NEWS.d CI check failed - is that all that is blocking this? If so, I would be happy to add a blurb if @jschulenklopper is unavailable.

Feel free to do so. I don't know where/how to do that, and I guess you figured that out already. Thanks.

@rolandcrosby
Copy link
Contributor

@jschulenklopper I put up a PR (jschulenklopper/cpython#2) that should add the relevant blurb file; if you merge that into your branch, it should get picked up by this PR.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@rolandcrosby

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@jschulenklopper
Copy link
Contributor Author

@jschulenklopper I put up a PR (jschulenklopper#2) that should add the relevant blurb file; if you merge that into your branch, it should get picked up by this PR.

Thanks @rolandcrosby - it's merged into my branch. But now you apparently need to sign the CLA or connect your GitHub account to the b.p.o. :-) Happened to me before as well.

@rolandcrosby
Copy link
Contributor

Haha, I should have known. I signed the CLA so this should be unblocked as soon as the bot updates the status.

@z3ntu
Copy link

z3ntu commented Sep 25, 2021

Any news on this PR?

@rolandcrosby
Copy link
Contributor

I don't think it's blocked on anything at the moment. Not sure who can merge it.

@jschulenklopper
Copy link
Contributor Author

The "CLA not signed" label is still applied, so if @rolandcrosby did sign the CLA indeed, the bot needs to update the status. Not sure how to trigger that thing...

Other than that, it seems that it can be (automatically) merged by project collaborators.

@rolandcrosby
Copy link
Contributor

Just added a note to the bpo issue to ask if any humans can fix the CLA status and help move this along.

@orsenthil
Copy link
Member

Hi Roland, Thanks for the ping. I will review and merge this.

Copy link
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: 4 of 6 required status checks are expected..

@orsenthil orsenthil closed this Oct 6, 2021
@orsenthil orsenthil reopened this Oct 6, 2021
@orsenthil orsenthil merged commit c379bc5 into python:main Oct 6, 2021
@bedevere-bot
Copy link

@orsenthil: Please replace # with GH- in the commit message next time. Thanks!

@orsenthil orsenthil added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Oct 6, 2021
@miss-islington
Copy link
Contributor

Thanks @jschulenklopper for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @jschulenklopper for the PR, and @orsenthil for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-28750 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed needs backport to 3.10 only security fixes needs backport to 3.9 only security fixes labels Oct 6, 2021
@bedevere-bot
Copy link

GH-28751 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 6, 2021
…ythonGH-19588)

* Support HTTP response status code 308 in urllib.

HTTP response status code 308 is defined in https://tools.ietf.org/html/rfc7538 to be the permanent redirect variant of 307 (temporary redirect).

* Update documentation to include http_error_308()

* Add blurb for bpo-40321 fix

Co-authored-by: Roland Crosby <[email protected]>
(cherry picked from commit c379bc5)

Co-authored-by: Jochem Schulenklopper <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 6, 2021
…ythonGH-19588)

* Support HTTP response status code 308 in urllib.

HTTP response status code 308 is defined in https://tools.ietf.org/html/rfc7538 to be the permanent redirect variant of 307 (temporary redirect).

* Update documentation to include http_error_308()

* Add blurb for bpo-40321 fix

Co-authored-by: Roland Crosby <[email protected]>
(cherry picked from commit c379bc5)

Co-authored-by: Jochem Schulenklopper <[email protected]>
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.

10 participants