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 SIGTERM handling in web.run_app #1946

Merged
merged 1 commit into from
Jun 10, 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
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Changes
- Retain method attributes (e.g. :code:`__doc__`) when registering synchronous
handlers for resources. #1953

-
- Added signal TERM handling in `run_app` to gracefully exit #1932

- Fix websocket issues caused by frame fragmentation. #1962

Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Brett Cannon
Brian C. Lane
Brian Muller
Carl George
Cecile Tonglet
Chien-Wei Huang
Chih-Yuan Chen
Chris AtLee
Expand Down
24 changes: 20 additions & 4 deletions aiohttp/web.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import asyncio
import os
import signal
import socket
import stat
import sys
Expand Down Expand Up @@ -318,10 +319,18 @@ def __repr__(self):
return "<Application 0x{:x}>".format(id(self))


class GracefulExit(SystemExit):
code = 1


def raise_graceful_exit():
raise GracefulExit()


def run_app(app, *, host=None, port=None, path=None, sock=None,
shutdown_timeout=60.0, ssl_context=None,
print=print, backlog=128, access_log_format=None,
access_log=access_logger, loop=None):
access_log=access_logger, handle_signals=True, loop=None):
Copy link
Member

Choose a reason for hiding this comment

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

maybe handle_sigterm is better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes actually I was thinking that you may want actually that run_app handles all the signals or none. For the user it's not really about handling SIGTERM in particular, it's more about handling the signals (INT and TERM) or not. It's unfortunate somehow that Python handles SIGINT for you by default.

I just realize now that maybe we should also handle SIGINT using https://docs.python.org/3/library/asyncio-eventloop.html#unix-signals but I'm not sure at all about it, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

SIGINT is handled out of the box, why should we care about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually answered later in the PR comments. It's not really "out of the box" for the event loop, it's out of the box for default synchronous usage of Python.

"""Run an app locally"""
user_supplied_loop = loop is not None
if loop is None:
Expand Down Expand Up @@ -414,12 +423,19 @@ def run_app(app, *, host=None, port=None, path=None, sock=None,
asyncio.gather(*server_creations, loop=loop)
)

print("======== Running on {} ========\n"
"(Press CTRL+C to quit)".format(', '.join(uris)))
if handle_signals:
try:
loop.add_signal_handler(signal.SIGINT, raise_graceful_exit)
loop.add_signal_handler(signal.SIGTERM, raise_graceful_exit)
except NotImplementedError: # pragma: no cover
# add_signal_handler is not implemented on Windows
pass

try:
print("======== Running on {} ========\n"
"(Press CTRL+C to quit)".format(', '.join(uris)))
loop.run_forever()
except KeyboardInterrupt: # pragma: no cover
except (GracefulExit, KeyboardInterrupt): # pragma: no cover
pass
finally:
server_closures = []
Expand Down
8 changes: 6 additions & 2 deletions docs/web_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2085,10 +2085,11 @@ Utilities


.. function:: run_app(app, *, host=None, port=None, path=None, \
loop=None, shutdown_timeout=60.0, \
sock=None, shutdown_timeout=60.0, \
ssl_context=None, print=print, backlog=128, \
access_log_format=None, \
access_log=aiohttp.log.access_logger)
access_log=aiohttp.log.access_logger, \
handle_signals=True, loop=None)

A utility function for running an application, serving it until
keyboard interrupt and performing a
Expand Down Expand Up @@ -2155,6 +2156,9 @@ Utilities
:ref:`aiohttp-logging-access-log-format-spec`
for details.

:param bool handle_signals: override signal TERM handling to gracefully
exit the application.

:param loop: an *event loop* used for running the application
(``None`` by default).

Expand Down
41 changes: 41 additions & 0 deletions tests/test_run_app.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import asyncio
import contextlib
import os
import platform
import signal
import socket
import ssl
import subprocess
import sys
from io import StringIO
from unittest import mock
from uuid import uuid4
Expand Down Expand Up @@ -46,6 +50,11 @@ def skip_if_no_dict(loop):
pytest.skip("can not override loop attributes")


def skip_if_on_windows():
if platform.system() == "Windows":
pytest.skip("the test is not valid for Windows")


def test_run_app_http(loop, mocker):
skip_if_no_dict(loop)

Expand Down Expand Up @@ -505,3 +514,35 @@ def test_run_app_multiple_preexisting_sockets(loop, mocker):
app.startup.assert_called_once_with()
assert "http://0.0.0.0:{}".format(port1) in printed.getvalue()
assert "http://0.0.0.0:{}".format(port2) in printed.getvalue()


_script_test_signal = """
from aiohttp import web

app = web.Application()
web.run_app(app, host=())
"""


def test_sigint(loop, mocker):
skip_if_on_windows()

proc = subprocess.Popen([sys.executable, "-u", "-c", _script_test_signal],
stdout=subprocess.PIPE)
for line in proc.stdout:
if line.startswith(b"======== Running on"):
break
proc.send_signal(signal.SIGINT)
assert proc.wait() == 0


def test_sigterm(loop, mocker):
skip_if_on_windows()

proc = subprocess.Popen([sys.executable, "-u", "-c", _script_test_signal],
stdout=subprocess.PIPE)
for line in proc.stdout:
if line.startswith(b"======== Running on"):
break
proc.terminate()
assert proc.wait() == 0