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

improved loop handling #158

Merged
merged 1 commit into from
Nov 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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