From aa5cd39035b20830f8164b6f4573756eb900a923 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 24 Nov 2017 12:41:51 +0000 Subject: [PATCH] improved loop handling --- aiohttp_devtools/runserver/config.py | 37 ++++++++++----------- aiohttp_devtools/runserver/main.py | 12 +++---- aiohttp_devtools/runserver/serve.py | 6 ++-- aiohttp_devtools/start/template/app/main.py | 2 +- aiohttp_devtools/version.py | 2 +- tests/conftest.py | 2 +- tests/test_runserver_config.py | 8 +++-- tests/test_runserver_main.py | 8 +++-- tests/test_serve.py | 9 +++-- tests/test_start.py | 9 +++-- 10 files changed, 51 insertions(+), 44 deletions(-) diff --git a/aiohttp_devtools/runserver/config.py b/aiohttp_devtools/runserver/config.py index 3c73b179..624b271e 100644 --- a/aiohttp_devtools/runserver/config.py +++ b/aiohttp_devtools/runserver/config.py @@ -1,3 +1,4 @@ +import asyncio import inspect import re import sys @@ -67,7 +68,6 @@ def __init__(self, *, self.main_port = main_port self.aux_port = aux_port or (main_port + 1) self.code_directory = None - self._import_app_factory() logger.debug('config loaded:\n%s', self) @property @@ -122,13 +122,7 @@ def _resolve_path(self, _path: str, check: str, arg_name: str): raise AdevConfigError('{} is not a directory'.format(path)) return path - @property - def app_factory(self): - # can't store app_factory on Config because it's not picklable for transfer to the subprocess via - # multiprocessing.Process (especially with uvloop) - return self._import_app_factory() - - def _import_app_factory(self): + def import_app_factory(self): """ Import attribute/class from from a python module. Raise AdevConfigError if the import failed. @@ -165,7 +159,7 @@ def _import_app_factory(self): self.code_directory = Path(module.__file__).parent return attr - def check(self, loop): + def check(self): """ run the app factory as a very basic check it's working and returns the right thing, this should catch config errors and database connection errors. @@ -174,23 +168,27 @@ def check(self, loop): logger.debug('pre-check disabled, not checking app factory') return logger.info('pre-check enabled, checking app factory') - if not callable(self.app_factory): + app_factory = self.import_app_factory() + if not callable(app_factory): raise AdevConfigError('app_factory "{.app_factory_name}" is not callable or an ' 'instance of aiohttp.web.Application'.format(self)) - loop.run_until_complete(self._startup_and_clean(loop)) + loop = asyncio.get_event_loop() + loop.run_until_complete(self._startup_and_clean()) - def load_app(self, loop): - if isinstance(self.app_factory, Application): - app = self.app_factory + def load_app(self): + app_factory = self.import_app_factory() + if isinstance(app_factory, Application): + app = app_factory else: # app_factory should be a proper factory with signature (loop): -> Application - signature = inspect.signature(self.app_factory) + signature = inspect.signature(app_factory) if 'loop' in signature.parameters: - app = self.app_factory(loop=loop) + loop = asyncio.get_event_loop() + app = app_factory(loop=loop) else: # loop argument missing, assume no arguments - app = self.app_factory() + app = app_factory() if not isinstance(app, Application): raise AdevConfigError('app factory "{.app_factory_name}" returned "{.__class__.__name__}" not an ' @@ -198,9 +196,8 @@ def load_app(self, loop): return app - async def _startup_and_clean(self, loop): - app = self.load_app(loop) - app._set_loop(loop) + async def _startup_and_clean(self): + app = self.load_app() logger.debug('app "%s" successfully created', app) logger.debug('running app startup...') await app.startup() diff --git a/aiohttp_devtools/runserver/main.py b/aiohttp_devtools/runserver/main.py index 3e824f9c..e6d8522f 100644 --- a/aiohttp_devtools/runserver/main.py +++ b/aiohttp_devtools/runserver/main.py @@ -32,11 +32,10 @@ def run_app(app, port, loop): loop.close() -def runserver(*, loop: asyncio.AbstractEventLoop=None, **config_kwargs): +def runserver(**config_kwargs): """ Prepare app ready to run development server. - :param loop: asyncio loop to use :param config_kwargs: see config.Config for more details :return: tuple (auxiliary app, auxiliary app port, event loop) """ @@ -44,8 +43,8 @@ def runserver(*, loop: asyncio.AbstractEventLoop=None, **config_kwargs): set_start_method('spawn') config = Config(**config_kwargs) - loop = loop or asyncio.get_event_loop() - config.check(loop) + config.check() + loop = asyncio.get_event_loop() loop.run_until_complete(check_port_open(config.main_port, loop)) @@ -75,11 +74,10 @@ def runserver(*, loop: asyncio.AbstractEventLoop=None, **config_kwargs): return aux_app, config.aux_port, loop -def serve_static(*, static_path: str, livereload: bool=True, port: int=8000, - loop: asyncio.AbstractEventLoop=None): +def serve_static(*, static_path: str, livereload: bool=True, port: int=8000): logger.debug('Config: path="%s", livereload=%s, port=%s', static_path, livereload, port) - loop = loop or asyncio.get_event_loop() + loop = asyncio.get_event_loop() app = create_auxiliary_app(static_path=static_path, livereload=livereload) if livereload: diff --git a/aiohttp_devtools/runserver/serve.py b/aiohttp_devtools/runserver/serve.py index 28803ead..954038e0 100644 --- a/aiohttp_devtools/runserver/serve.py +++ b/aiohttp_devtools/runserver/serve.py @@ -105,13 +105,13 @@ def set_tty(tty_path): # pragma: no cover yield -def serve_main_app(config: Config, tty_path: Optional[str], loop: asyncio.AbstractEventLoop=None): +def serve_main_app(config: Config, tty_path: Optional[str]): with set_tty(tty_path): setup_logging(config.verbose) - loop = loop or asyncio.get_event_loop() + app = config.load_app() - app = config.load_app(loop) + loop = asyncio.get_event_loop() modify_main_app(app, config) diff --git a/aiohttp_devtools/start/template/app/main.py b/aiohttp_devtools/start/template/app/main.py index 5d4a6ab0..5c80ee47 100644 --- a/aiohttp_devtools/start/template/app/main.py +++ b/aiohttp_devtools/start/template/app/main.py @@ -124,7 +124,7 @@ def setup_routes(app): # {% endif %} -def create_app(loop): +def create_app(): app = web.Application() settings = Settings() app.update( diff --git a/aiohttp_devtools/version.py b/aiohttp_devtools/version.py index 96f93003..03ad6166 100644 --- a/aiohttp_devtools/version.py +++ b/aiohttp_devtools/version.py @@ -2,4 +2,4 @@ __all__ = ['VERSION'] -VERSION = StrictVersion('0.6.3') +VERSION = StrictVersion('0.6.4') diff --git a/tests/conftest.py b/tests/conftest.py index f33a9959..e56e1599 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,7 +13,7 @@ def pytest_addoption(parser): async def hello(request): return web.Response(text='hello world') -def create_app(loop): +def create_app(): app = web.Application() app.router.add_get('/', hello) return app""" diff --git a/tests/test_runserver_config.py b/tests/test_runserver_config.py index 4c24e447..820f7b79 100644 --- a/tests/test_runserver_config.py +++ b/tests/test_runserver_config.py @@ -1,3 +1,5 @@ +import asyncio + import pytest from pytest_toolbox import mktree @@ -16,8 +18,9 @@ async def test_load_simple_app(tmpworkdir): async def test_create_app_wrong_name(tmpworkdir, loop): mktree(tmpworkdir, SIMPLE_APP) + config = Config(app_path='app.py', app_factory_name='missing') with pytest.raises(AiohttpDevConfigError) as excinfo: - Config(app_path='app.py', app_factory_name='missing') + config.import_app_factory() assert excinfo.value.args[0] == 'Module "app.py" does not define a "missing" attribute/class' @@ -56,7 +59,8 @@ def app_factory(loop): @if_boxed @pytest.mark.parametrize('files,exc', invalid_apps, ids=['%s...' % v[1][:40] for v in invalid_apps]) def test_invalid_options(tmpworkdir, files, exc, loop): + asyncio.set_event_loop(loop) mktree(tmpworkdir, files) with pytest.raises(AiohttpDevConfigError) as excinfo: - Config(app_path='.').check(loop) + Config(app_path='.').check() assert exc.format(tmpworkdir=tmpworkdir) == excinfo.value.args[0] diff --git a/tests/test_runserver_main.py b/tests/test_runserver_main.py index 29847461..ec7eaedc 100644 --- a/tests/test_runserver_main.py +++ b/tests/test_runserver_main.py @@ -151,10 +151,11 @@ def test_run_app(loop, unused_port): @if_boxed -async def test_run_app_test_client(loop, tmpworkdir, test_client): +async def test_run_app_test_client(tmpworkdir, test_client): mktree(tmpworkdir, SIMPLE_APP) config = Config(app_path='app.py') - app = config.app_factory(loop=loop) + app_factory = config.import_app_factory() + app = app_factory() modify_main_app(app, config) assert isinstance(app, aiohttp.web.Application) cli = await test_client(app) @@ -179,13 +180,14 @@ async def test_aux_app(tmpworkdir, test_client): @if_boxed @slow def test_serve_main_app(tmpworkdir, loop, mocker): + asyncio.set_event_loop(loop) mktree(tmpworkdir, SIMPLE_APP) mocker.spy(loop, 'create_server') mock_modify_main_app = mocker.patch('aiohttp_devtools.runserver.serve.modify_main_app') loop.call_later(0.5, loop.stop) config = Config(app_path='app.py') - serve_main_app(config, '/dev/tty', loop=loop) + serve_main_app(config, '/dev/tty') assert loop.is_closed() loop.create_server.assert_called_with(mock.ANY, '0.0.0.0', 8000, backlog=128) diff --git a/tests/test_serve.py b/tests/test_serve.py index a790fc6f..0ee621ee 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -1,3 +1,5 @@ +import asyncio + import pytest from pytest_toolbox import mktree @@ -6,7 +8,8 @@ @pytest.yield_fixture def cli(loop, tmpworkdir, test_client): - app, _, _ = serve_static(static_path=str(tmpworkdir), livereload=False, loop=loop) + asyncio.set_event_loop(loop) + app, _, _ = serve_static(static_path=str(tmpworkdir), livereload=False) yield loop.run_until_complete(test_client(app)) @@ -29,7 +32,7 @@ async def test_file_missing(cli): async def test_html_file_livereload(loop, test_client, tmpworkdir): - app, port, _ = serve_static(static_path=str(tmpworkdir), livereload=True, loop=loop) + app, port, _ = serve_static(static_path=str(tmpworkdir), livereload=True) assert port == 8000 cli = await test_client(app) mktree(tmpworkdir, { @@ -48,7 +51,7 @@ async def test_html_file_livereload(loop, test_client, tmpworkdir): async def test_serve_index(loop, test_client, tmpworkdir): - app, port, _ = serve_static(static_path=str(tmpworkdir), livereload=False, loop=loop) + app, port, _ = serve_static(static_path=str(tmpworkdir), livereload=False) assert port == 8000 cli = await test_client(app) mktree(tmpworkdir, { diff --git a/tests/test_start.py b/tests/test_start.py index 2fea0647..366b427e 100644 --- a/tests/test_start.py +++ b/tests/test_start.py @@ -64,7 +64,8 @@ async def test_start_other_dir(tmpdir, loop, test_client, caplog): example: message-board adev.main INFO: project created, 16 files generated\n""" == caplog.log.replace(str(tmpdir), '/') config = Config(app_path='the-path/app/', root_path=str(tmpdir)) - app = config.app_factory(loop=loop) + app_factory = config.import_app_factory() + app = app_factory() modify_main_app(app, config) assert isinstance(app, aiohttp.web.Application) @@ -109,7 +110,8 @@ async def test_all_options(tmpdir, test_client, loop, template_engine, session, return config = Config(app_path='app/main.py', root_path=str(tmpdir)) - app = config.app_factory(loop=loop) + app_factory = config.import_app_factory() + app = app_factory() modify_main_app(app, config) cli = await test_client(app) r = await cli.get('/') @@ -147,7 +149,8 @@ async def test_db_creation(tmpdir, test_client, loop): os.environ['APP_DB_PASSWORD'] = db_password config = Config(app_path='app/main.py', root_path=str(tmpdir)) - app = config.app_factory(loop=loop) + app_factory = config.import_app_factory() + app = app_factory() modify_main_app(app, config) cli = await test_client(app) r = await cli.get('/')