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

Add runner.serve_forever() #7688

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/2746.feature
@@ -0,0 +1 @@
Add method `BaseRunner.serve_forever()` that waits forever until being cancelled.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Expand Up @@ -215,6 +215,7 @@ Ludovic Gasc
Luis Pedrosa
Lukasz Marcin Dobrzanski
Makc Belousow
Maksim Vasilev
Manuel Miranda
Marat Sharafutdinov
Marc Mueller
Expand Down
24 changes: 23 additions & 1 deletion aiohttp/web_runner.py
Expand Up @@ -260,13 +260,21 @@


class BaseRunner(ABC):
__slots__ = ("starting_tasks", "_handle_signals", "_kwargs", "_server", "_sites")
__slots__ = (
"starting_tasks",
"_handle_signals",
"_kwargs",
"_server",
"_sites",
"_serve_forever_fut",
)

def __init__(self, *, handle_signals: bool = False, **kwargs: Any) -> None:
self._handle_signals = handle_signals
self._kwargs = kwargs
self._server: Optional[Server] = None
self._sites: List[BaseSite] = []
self._serve_forever_fut: Optional[asyncio.Future[None]] = None

@property
def server(self) -> Optional[Server]:
Expand Down Expand Up @@ -306,6 +314,20 @@
# so we just record all running tasks here and exclude them later.
self.starting_tasks = asyncio.all_tasks()

async def serve_forever(self) -> None:
if self._serve_forever_fut is not None:
raise RuntimeError("Concurrent calls to serve_forever() are not allowed")

Check warning on line 319 in aiohttp/web_runner.py

View check run for this annotation

Codecov / codecov/patch

aiohttp/web_runner.py#L319

Added line #L319 was not covered by tests
if self._server is None:
raise RuntimeError("Call setup() first")

loop = asyncio.get_event_loop()
self._serve_forever_fut = loop.create_future()

try:
await self._serve_forever_fut
Dismissed Show dismissed Hide dismissed
finally:
self._serve_forever_fut = None

@abstractmethod
async def shutdown(self) -> None:
pass # pragma: no cover
Expand Down
4 changes: 1 addition & 3 deletions docs/web_advanced.rst
Expand Up @@ -909,9 +909,7 @@ The simple startup code for serving HTTP site on ``'localhost'``, port
await runner.setup()
site = web.TCPSite(runner, 'localhost', 8080)
await site.start()

while True:
await asyncio.sleep(3600) # sleep forever
await runner.serve_forever()

To stop serving call :meth:`AppRunner.cleanup`::

Expand Down
8 changes: 7 additions & 1 deletion docs/web_reference.rst
Expand Up @@ -2511,7 +2511,7 @@ application on specific TCP or Unix socket, e.g.::
site = web.TCPSite(runner, 'localhost', 8080)
await site.start()
# wait for finish signal
await runner.cleanup()
await runner.serve_forever()


.. versionadded:: 3.0
Expand Down Expand Up @@ -2554,6 +2554,12 @@ application on specific TCP or Unix socket, e.g.::

Stop handling all registered sites and cleanup used resources.

.. method:: serve_forever()
:async:

Wait forever. Make sure to call :meth:`setup` prior calling this method.
Only one :meth:`serve_forever` task is allowed per one runner object.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Only one :meth:`serve_forever` task is allowed per one runner object.
Only one :meth:`serve_forever` task is allowed per runner object.



.. class:: AppRunner(app, *, handle_signals=False, **kwargs)

Expand Down
32 changes: 29 additions & 3 deletions tests/test_web_runner.py
Expand Up @@ -2,7 +2,7 @@
import asyncio
import platform
import signal
from typing import Any
from typing import Any, Optional
from unittest.mock import patch

import pytest
Expand All @@ -22,8 +22,8 @@
asyncio.set_event_loop(loop)
runners = []

def go(**kwargs):
runner = web.AppRunner(app, **kwargs)
def go(app_param: Optional[web.AppRunner] = None, **kwargs):
runner = web.AppRunner(app_param or app, **kwargs)
runners.append(runner)
return runner

Expand Down Expand Up @@ -265,3 +265,29 @@

web.run_app(app)
assert spy.called, "run_app() should work after asyncio.run()."


async def test_app_runner_serve_forever_uninitialized(make_runner) -> None:
runner = make_runner()
with pytest.raises(RuntimeError):
await runner.serve_forever()


async def test_app_runner_serve_forever_concurrent_call(make_runner, loop) -> None:
runner = make_runner()
task = loop.create_task(runner.serve_forever())
await asyncio.sleep(0.01)
with pytest.raises(RuntimeError):
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the match argument to match the text of the exception in these?
The codecov comment suggests this test isn't working correctly.

await runner.serve_forever()
task.cancel()


async def test_app_runner_serve_forever_multiple_times(make_runner, loop) -> None:
runner = make_runner()
for _ in range(3):
await runner.setup()
task = loop.create_task(runner.serve_forever())
await asyncio.sleep(0.01)
task.cancel()
with pytest.raises(asyncio.CancelledError):
await task
Dismissed Show dismissed Hide dismissed