From c49299bb95eacd41a52c2ab32e75dbc3300faf15 Mon Sep 17 00:00:00 2001 From: Ihor Kalnytskyi Date: Wed, 7 Dec 2016 11:32:39 +0200 Subject: [PATCH] Add workarounds to fly on aiohttp >= 1.1 Since aiohttp 1.1 some backward incompatible changes have been implemented. Despite each of them has its own issue on GitHub: - https://github.com/KeepSafe/aiohttp/issues/1373 - https://github.com/KeepSafe/aiohttp/issues/1416 it's not clear when they become resolved. In order to be able to fly on latest aiohttp version, this commit resolves those issues by implementing workarounds on our side. --- setup.py | 2 +- tests/test_application.py | 2 +- tests/test_router.py | 17 +++++++++++++---- xsnippet_api/application.py | 32 ++++++++++++++++++++------------ xsnippet_api/router.py | 6 ++++-- 5 files changed, 39 insertions(+), 20 deletions(-) diff --git a/setup.py b/setup.py index 44a646b..cb5ccf2 100644 --- a/setup.py +++ b/setup.py @@ -32,7 +32,7 @@ 'pytest-runner', ], install_requires=[ - 'aiohttp >= 0.21.2, < 1.1', + 'aiohttp >= 1.1', 'cerberus >= 0.9.2', 'motor >= 0.7, < 1.0', 'python-jose >= 1.3.2', diff --git a/tests/test_application.py b/tests/test_application.py index 0c8f926..32390c9 100644 --- a/tests/test_application.py +++ b/tests/test_application.py @@ -27,7 +27,7 @@ def setup(self): @pytest.mark.parametrize('name, value', [ ('Accept', 'application/json'), ('Accept-Encoding', 'gzip'), - ('Api-Version', '1'), + ('Api-Version', '1.0'), ]) async def test_http_vary_header(self, name, value): async with AIOTestApp(self.app) as testapp: diff --git a/tests/test_router.py b/tests/test_router.py index e84f3b9..082dece 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -27,18 +27,27 @@ async def get(self): return web.Response(text='I am Batman!') def setup(self): - router_v1 = web_urldispatcher.UrlDispatcher() + self.app = web.Application() + + router_v1 = web_urldispatcher.UrlDispatcher(self.app) router_v1.add_route('*', '/test', self._TestResource1) - router_v2 = web_urldispatcher.UrlDispatcher() + router_v2 = web_urldispatcher.UrlDispatcher(self.app) router_v2.add_route('*', '/test', self._TestResource2) - self.app = web.Application(router=router.VersionRouter( + # Since aiohttp 1.1, UrlDispatcher has one mandatory attribute - + # application instance, that's used internally only in subapps + # feature. Unfortunately, there's no legal way to pass application + # when router is created or vice versa. Either way we need to + # access internal variable in order to do so. + # + # See https://github.com/KeepSafe/aiohttp/issues/1373 for details. + self.app._router = router.VersionRouter( { '1': router_v1, '2': router_v2, } - )) + ) async def test_version_1(self): async with AIOTestApp(self.app) as testapp: diff --git a/xsnippet_api/application.py b/xsnippet_api/application.py index 24b2bc0..88f3e91 100644 --- a/xsnippet_api/application.py +++ b/xsnippet_api/application.py @@ -56,22 +56,30 @@ def create_app(conf): :return: an application instance :rtype: :class:`aiohttp.web.Application` """ - # we need to import all the resources in order to evaluate @endpoint - # decorator, so they can be collected and passed to VersionRouter + # We need to import all the resources in order to evaluate @endpoint + # decorator, so they can be collected and passed to VersionRouter. from . import resources # noqa app = web.Application( - router=router.VersionRouter(endpoint.collect()), middlewares=[ functools.partial(middlewares.auth, conf['auth']), ]) - # attach settings to the application instance in order to make them - # accessible at any point of execution (e.g. request handling) + # Since aiohttp 1.1, UrlDispatcher has one mandatory attribute - + # application instance, that's used internally only in subapps + # feature. Unfortunately, there's no legal way to pass application + # when router is created or vice versa. Either way we need to + # access internal variable in order to do so. + # + # See https://github.com/KeepSafe/aiohttp/issues/1373 for details. + app._router = router.VersionRouter(endpoint.collect(app)) + + # Attach settings to the application instance in order to make them + # accessible at any point of execution (e.g. request handling). app['conf'] = conf app['db'] = database.create_connection(conf) - # we need to respond with Vary header time to time in order to avoid - # issues with cache on client side + # We need to respond with Vary header time to time in order to avoid + # issues with cache on client side. app.on_response_prepare.append(_inject_vary_header) return app @@ -109,18 +117,18 @@ def __call__(self, resource): return resource @classmethod - def collect(cls): - rv = collections.defaultdict(web_urldispatcher.UrlDispatcher) + def collect(cls, app): + rv = {} # Create routers for each discovered API version. The main reason why # we need that so early is to register resources in all needed routers # according to supported version range. for item in cls._registry: - rv[item.version] + rv[item.version] = web_urldispatcher.UrlDispatcher(app) for item in cls._registry: - # if there's no end_version then a resource is still working, and - # latest discovered version should be considered as end_version + # If there's no end_version then a resource is still working, and + # latest discovered version should be considered as end_version. end_version = item.end_version or sorted( rv.keys(), key=pkg_resources.parse_version diff --git a/xsnippet_api/router.py b/xsnippet_api/router.py index 3b85876..1ed8e9c 100644 --- a/xsnippet_api/router.py +++ b/xsnippet_api/router.py @@ -12,7 +12,7 @@ import pkg_resources -from aiohttp import abc, web +from aiohttp import abc, web, web_urldispatcher def _get_latest_version(versions, stable=True): @@ -77,6 +77,8 @@ async def resolve(self, request): version = self._latest if version not in self._routers: - raise web.HTTPPreconditionFailed() + return web_urldispatcher.MatchInfoError( + web.HTTPPreconditionFailed() + ) return await self._routers[version].resolve(request)