Skip to content

Commit

Permalink
improved loop handling (#158)
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelcolvin authored Nov 24, 2017
1 parent 96ee738 commit c03a16d
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 44 deletions.
37 changes: 17 additions & 20 deletions aiohttp_devtools/runserver/config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import inspect
import re
import sys
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -174,33 +168,36 @@ 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 '
'aiohttp.web.Application'.format(self, app))

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()
Expand Down
12 changes: 5 additions & 7 deletions aiohttp_devtools/runserver/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,19 @@ 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)
"""
# force a full reload in sub processes so they load an updated version of code, this must be called only once
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))

Expand Down Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions aiohttp_devtools/runserver/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion aiohttp_devtools/start/template/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def setup_routes(app):
# {% endif %}


def create_app(loop):
def create_app():
app = web.Application()
settings = Settings()
app.update(
Expand Down
2 changes: 1 addition & 1 deletion aiohttp_devtools/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

__all__ = ['VERSION']

VERSION = StrictVersion('0.6.3')
VERSION = StrictVersion('0.6.4')
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
8 changes: 6 additions & 2 deletions tests/test_runserver_config.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import asyncio

import pytest
from pytest_toolbox import mktree

Expand All @@ -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'


Expand Down Expand Up @@ -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]
8 changes: 5 additions & 3 deletions tests/test_runserver_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions tests/test_serve.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import asyncio

import pytest
from pytest_toolbox import mktree

Expand All @@ -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))


Expand All @@ -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, {
Expand All @@ -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, {
Expand Down
9 changes: 6 additions & 3 deletions tests/test_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -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), '/<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)

Expand Down Expand Up @@ -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('/')
Expand Down Expand Up @@ -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('/')
Expand Down

0 comments on commit c03a16d

Please sign in to comment.