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

fix for redirect issue #2030

Merged
merged 10 commits into from
Jun 29, 2017
Merged
30 changes: 16 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# Some simple testing tasks (sorry, UNIX only).

pytest := python3 -m pytest
Copy link
Member

Choose a reason for hiding this comment

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

What is intention for this variable? It's basically an alias for just pytest, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is so I could run the tests locally, where I have py2 + py3 installed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, with this change you can run the tests outside virtualenv, still want me revert? it makes no different inside virtual env but fixes it outside virtual env


all: test

.install-deps: requirements/dev.txt
@pip install -U -r requirements/dev.txt
@pip3 install -U -r requirements/dev.txt
Copy link
Member

Choose a reason for hiding this comment

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

aiohttp development assumes virtual environment usage.
In proper virtual env pip is just an alias for pip3 if python is python3

@touch .install-deps

isort:
Expand All @@ -19,9 +21,9 @@ flake: .flake
$(shell find examples -type f) \
$(shell find demos -type f)
@flake8 aiohttp --exclude=aiohttp/backport_cookies.py
@if python -c "import sys; sys.exit(sys.version_info < (3,5))"; then \
@if python3 -c "import sys; sys.exit(sys.version_info < (3,5))"; then \
flake8 examples tests demos && \
python setup.py check -rms; \
python3 setup.py check -rms; \
fi
@if ! isort -c -rc aiohttp tests examples; then \
echo "Import sort errors, run 'make isort' to fix them!!!"; \
Expand All @@ -34,31 +36,31 @@ check_changes:
@./tools/check_changes.py

.develop: .install-deps $(shell find aiohttp -type f) .flake check_changes
@pip install -e .
@pip3 install -e .
@touch .develop

test: .develop
@py.test -q ./tests
$(pytest) -q ./tests

vtest: .develop
@py.test -s -v ./tests
$(pytest) -s -v ./tests

cov cover coverage:
tox

cov-dev: .develop
@echo "Run without extensions"
@AIOHTTP_NO_EXTENSIONS=1 py.test --cov=aiohttp tests
@py.test --cov=aiohttp --cov-report=term --cov-report=html --cov-append tests
@AIOHTTP_NO_EXTENSIONS=1 $(pytest) --cov=aiohttp tests
$(pytest) --cov=aiohttp --cov-report=term --cov-report=html --cov-append tests
@echo "open file://`pwd`/coverage/index.html"

cov-dev-full: .develop
@echo "Run without extensions"
@AIOHTTP_NO_EXTENSIONS=1 py.test --cov=aiohttp tests
@AIOHTTP_NO_EXTENSIONS=1 $(pytest) --cov=aiohttp tests
@echo "Run in debug mode"
@PYTHONASYNCIODEBUG=1 py.test --cov=aiohttp --cov-append tests
@PYTHONASYNCIODEBUG=1 $(pytest) --cov=aiohttp --cov-append tests
@echo "Regular run"
@py.test --cov=aiohttp --cov-report=term --cov-report=html --cov-append tests
$(pytest) --cov=aiohttp --cov-report=term --cov-report=html --cov-append tests
@echo "open file://`pwd`/coverage/index.html"

clean:
Expand All @@ -75,7 +77,7 @@ clean:
@rm -rf build
@rm -rf cover
@make -C docs clean
@python setup.py clean
@python3 setup.py clean
@rm -f aiohttp/_multidict.html
@rm -f aiohttp/_multidict.c
@rm -f aiohttp/_multidict.*.so
Expand All @@ -101,7 +103,7 @@ doc-spelling:
@make -C docs spelling SPHINXOPTS="-W -E"

install:
@pip install -U pip
@pip install -Ur requirements/dev.txt
@pip3 install -U pip
@pip3 install -Ur requirements/dev.txt

.PHONY: all build flake test vtest cov clean doc
10 changes: 4 additions & 6 deletions aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
from . import connector as connector_mod
from . import client_exceptions, client_reqrep, hdrs, http, payload
from .client_exceptions import * # noqa
from .client_exceptions import (ClientError, ClientOSError,
ClientRedirectError, ServerTimeoutError,
from .client_exceptions import (ClientError, ClientOSError, ServerTimeoutError,
WSServerHandshakeError)
from .client_reqrep import * # noqa
from .client_reqrep import ClientRequest, ClientResponse
Expand Down Expand Up @@ -275,10 +274,9 @@ def _request(self, method, url, *,
r_url = (resp.headers.get(hdrs.LOCATION) or
resp.headers.get(hdrs.URI))
if r_url is None:
raise ClientRedirectError(
resp.request_info,
resp.history,
resp.status)
# see github.com/aio-libs/aiohttp/issues/2022
break

r_url = URL(
r_url, encoded=not self.requote_redirect_url)

Expand Down
14 changes: 1 addition & 13 deletions aiohttp/client_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
'ServerConnectionError', 'ServerTimeoutError', 'ServerDisconnectedError',
'ServerFingerprintMismatch',

'ClientResponseError', 'ClientRedirectError', 'ClientPayloadError',
'ClientResponseError', 'ClientPayloadError',
'ClientHttpProxyError', 'WSServerHandshakeError')


Expand All @@ -37,18 +37,6 @@ def __init__(self, request_info, history, *,
super().__init__("%s, message='%s'" % (code, message))


class ClientRedirectError(ClientResponseError):
"""Redirection error.

Response is a redirect but Location or URI HTTP headers are
missing

"""
def __init__(self, request_info, history, code):
super().__init__(request_info, history, code=code,
message="Response has no Location or URI header")


class WSServerHandshakeError(ClientResponseError):
"""websocket server handshake error."""

Expand Down
3 changes: 2 additions & 1 deletion demos/polls/aiohttpdemo_polls/db.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import aiopg.sa
import sqlalchemy as sa

import aiopg.sa
Copy link
Member

Choose a reason for hiding this comment

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

Please drop this file from PR. The change is not related to issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when I run the tests locally flake said to run isort against this file, still want me to revert?



__all__ = ['question', 'choice']

Expand Down
7 changes: 0 additions & 7 deletions docs/client_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1447,12 +1447,6 @@ Response errors
handle redirection responses.


.. class:: ClientRedirectError

Response is a redirect but ``Location`` or ``URI`` headers are missing.

Derived from :exc:`ClientResponseError`

.. class:: WSServerHandshakeError

Web socket server response error.
Expand Down Expand Up @@ -1540,7 +1534,6 @@ Hierarchy of exceptions

* :exc:`ClientResponseError`

* :exc:`ClientRedirectError`
* :exc:`WSServerHandshakeError`
* :exc:`ClientHttpProxyError`

Expand Down
13 changes: 7 additions & 6 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from multidict import MultiDict

import aiohttp
from aiohttp import ClientRedirectError, ServerFingerprintMismatch, hdrs, web
from aiohttp import ServerFingerprintMismatch, hdrs, web
from aiohttp.helpers import create_future
from aiohttp.multipart import MultipartWriter

Expand Down Expand Up @@ -2097,18 +2097,19 @@ def redirect(request):

@asyncio.coroutine
def test_redirect_without_location_header(loop, test_client):
body = b'redirect'

@asyncio.coroutine
def handler_redirect(request):
return web.Response(status=301)
return web.Response(status=301, body=body)

app = web.Application()
app.router.add_route('GET', '/redirect', handler_redirect)
client = yield from test_client(app)

with pytest.raises(ClientRedirectError) as ctx:
yield from client.get('/redirect')
expected_msg = 'Response has no Location or URI header'
assert str(ctx.value.message) == expected_msg
resp = yield from client.get('/redirect')
data = yield from resp.read()
assert data == body


@asyncio.coroutine
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ deps =

commands =
flake8 aiohttp examples tests
python setup.py check -rm
python3 setup.py check -rm
Copy link
Member

Choose a reason for hiding this comment

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

Again we are in python3 virtual environment here.
Don't need python3 command.

coverage erase

basepython:
Expand Down