Skip to content
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

Improve redirect handling #157

Merged
merged 2 commits into from
Oct 9, 2014
Merged

Improve redirect handling #157

merged 2 commits into from
Oct 9, 2014

Conversation

ludovic-gasc
Copy link
Contributor

Hi,

I've added 307 handling (#155) + for 301 and 302, I mimic IE behaviour, to replace HTTP verb by GET after redirection. For technical details: https://github.com/kennethreitz/requests/pull/269

It's a really an issue for us, because we need to interact with some old fashion webapp without WS API that use a lot this assumption.
Moreover, this behaviour is now official, 307 HTTP code has been created to handle correctly redirections.

Thanks for reviewing.

Regards.

fafhrd91 added a commit that referenced this pull request Oct 9, 2014
@fafhrd91 fafhrd91 merged commit e494364 into aio-libs:master Oct 9, 2014
@fafhrd91
Copy link
Member

fafhrd91 commented Oct 9, 2014

Thank you!

@ludovic-gasc
Copy link
Contributor Author

Hi @fafhrd91,

Thanks for the merge, it's a real pleasure to work with a collaborative guy ;-)

BTW, if somebody could make a release of aiohttp in a near future, it should be cool.
Not a stress for us, we use the patched version in github via requirements.txt, but for newbies on aiohttp, they will have quickly this issue if they use aiohttp for web scrapping.

@ludovic-gasc ludovic-gasc deleted the improve_redirect_handling branch October 9, 2014 12:24
@asvetlov
Copy link
Member

asvetlov commented Oct 9, 2014

+1 for next bugfix release

On Thu, Oct 9, 2014 at 3:24 PM, Ludovic Gasc notifications@github.com
wrote:

Hi @fafhrd91 https://github.com/fafhrd91,

Thanks for the merge, it's a real pleasure to work with a collaborative
guy ;-)

BTW, if somebody could make a release of aiohttp in a near future, it
should be cool.
Not a stress for us, we use the patched version in github via
requirements.txt, but for newbies on aiohttp, they will have quickly this
issue if they use aiohttp for web scrapping.


Reply to this email directly or view it on GitHub
#157 (comment).

Thanks,
Andrew Svetlov

@fafhrd91
Copy link
Member

fafhrd91 commented Oct 9, 2014

I'd like to merge PR for chunking requests first. I am planing to make
release later next week.

On Thursday, October 9, 2014, Andrew Svetlov notifications@github.com
wrote:

+1 for next bugfix release

On Thu, Oct 9, 2014 at 3:24 PM, Ludovic Gasc <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Hi @fafhrd91 https://github.com/fafhrd91,

Thanks for the merge, it's a real pleasure to work with a collaborative
guy ;-)

BTW, if somebody could make a release of aiohttp in a near future, it
should be cool.
Not a stress for us, we use the patched version in github via
requirements.txt, but for newbies on aiohttp, they will have quickly
this
issue if they use aiohttp for web scrapping.


Reply to this email directly or view it on GitHub
#157 (comment).

Thanks,
Andrew Svetlov


Reply to this email directly or view it on GitHub
#157 (comment).

@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants