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.

Sign up for GitHub

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

Rfc/git 1630 signals #1902

Closed
wants to merge 6 commits into from
Closed

Rfc/git 1630 signals #1902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d81e0e5
:sparkles: add initial signals class definitions
harshanarayana Jan 21, 2020
6de1bd5
enable `aiosignal` dependency and add additional mechanism to wrap th…
harshanarayana Feb 1, 2020
f0736c4
:white_check_mark: fix unit tests and linter issues
harshanarayana Feb 4, 2020
4ad9501
:rotating_light: fix linter warnings for unused imports
harshanarayana Feb 4, 2020
39dd8f7
:rotating_light: fix linter issues
harshanarayana Feb 4, 2020
98f97d7
Merge conflicts
ahopkins Aug 2, 2020
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
5 changes: 5 additions & 0 deletions mypy.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[mypy]
warn_unused_configs = True

[mypy-aiosignal]
ignore_missing_imports = True
134 changes: 47 additions & 87 deletions sanic/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import os
import re
import warnings

from asyncio import CancelledError, Protocol, ensure_future, get_event_loop
from collections import defaultdict, deque
from functools import partial
Expand All @@ -21,16 +20,12 @@
from sanic.constants import HTTP_METHODS
from sanic.exceptions import SanicException, ServerError, URLBuildError
from sanic.handlers import ErrorHandler
from sanic.helpers import publish, subscribe
from sanic.log import LOGGING_CONFIG_DEFAULTS, error_logger, logger
from sanic.response import HTTPResponse, StreamingHTTPResponse
from sanic.router import Router
from sanic.server import (
AsyncioServer,
HttpProtocol,
Signal,
serve,
serve_multiple,
)
from sanic.server import AsyncioServer, HttpProtocol, Signal, serve, serve_multiple
from sanic.signals import Namespace
from sanic.static import register as static_register
from sanic.testing import SanicASGITestClient, SanicTestClient
from sanic.views import CompositionView
Expand Down Expand Up @@ -92,6 +87,18 @@ def __init__(
self.go_fast = self.run
self.test_mode = False

# Signal Handlers and Registry
self._signals = {
"server": Namespace(namespace="server", owner=self, classic_event=True,),
"request": Namespace(namespace="request", owner=self),
"response": Namespace(namespace="response", owner=self),
"middleware": Namespace(namespace="middleware", owner=self),
}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like 4 Namespaces of signals are defined, but signal emitters are only implemented for server signals. Are the others planned to be implemented?


@property
def signals(self) -> Dict[str, Namespace]:
return self._signals

@property
def loop(self):
"""Synonymous with asyncio.get_event_loop().
Expand Down Expand Up @@ -121,9 +128,7 @@ def add_task(self, task):
loop = self.loop # Will raise SanicError if loop is not started
self._loop_add_task(task, self, loop)
except SanicException:
self.listener("before_server_start")(
partial(self._loop_add_task, task)
)
self.listener("before_server_start")(partial(self._loop_add_task, task))

# Decorator
def listener(self, event):
Expand All @@ -133,6 +138,7 @@ def listener(self, event):
"""

def decorator(listener):
subscribe(event, self.signals, listener)
self.listeners[event].append(listener)
return listener

Expand All @@ -146,7 +152,7 @@ def register_listener(self, listener, event):
:param event: when to register listener i.e. 'before_server_start'
:return: listener
"""

subscribe(event_name=event, callback=listener, signals=self.signals)
Copy link
Member

Choose a reason for hiding this comment

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

If self.listener() decorator calls subscribe() (on line 141 above) then why does this helper function also need to call subscribe()? Wouldn't this result in two subscribes with the same listener and signals?

return self.listener(event)(listener)

# Decorator
Expand Down Expand Up @@ -219,9 +225,7 @@ def response(handler):
return response

# Shorthand method decorators
def get(
self, uri, host=None, strict_slashes=None, version=None, name=None
):
def get(self, uri, host=None, strict_slashes=None, version=None, name=None):
Copy link
Member

Choose a reason for hiding this comment

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

Looks like quite a lot of the changes in this file are formatting changes, was that just a result of merging in latest master? Or should these formatting changes be filtered out?

"""
Add an API URL under the **GET** *HTTP* method

Expand Down Expand Up @@ -302,9 +306,7 @@ def put(
name=name,
)

def head(
self, uri, host=None, strict_slashes=None, version=None, name=None
):
def head(self, uri, host=None, strict_slashes=None, version=None, name=None):
return self.route(
uri,
methods=frozenset({"HEAD"}),
Expand All @@ -314,9 +316,7 @@ def head(
name=name,
)

def options(
self, uri, host=None, strict_slashes=None, version=None, name=None
):
def options(self, uri, host=None, strict_slashes=None, version=None, name=None):
"""
Add an API URL under the **OPTIONS** *HTTP* method

Expand Down Expand Up @@ -367,9 +367,7 @@ def patch(
name=name,
)

def delete(
self, uri, host=None, strict_slashes=None, version=None, name=None
):
def delete(self, uri, host=None, strict_slashes=None, version=None, name=None):
"""
Add an API URL under the **DELETE** *HTTP* method

Expand Down Expand Up @@ -491,9 +489,7 @@ def response(handler):
websocket_handler = partial(
self._websocket_handler, handler, subprotocols=subprotocols
)
websocket_handler.__name__ = (
"websocket_handler_" + handler.__name__
)
websocket_handler.__name__ = "websocket_handler_" + handler.__name__
routes.extend(
self.router.add(
uri=uri,
Expand Down Expand Up @@ -604,9 +600,7 @@ def register_middleware(self, middleware, attach_to="request"):
self.response_middleware.appendleft(middleware)
return middleware

def register_named_middleware(
self, middleware, route_names, attach_to="request"
):
def register_named_middleware(self, middleware, route_names, attach_to="request"):
if attach_to == "request":
for _rn in route_names:
if _rn not in self.named_request_middleware:
Expand Down Expand Up @@ -635,9 +629,7 @@ def middleware(self, middleware_or_request):
return self.register_middleware(middleware_or_request)

else:
return partial(
self.register_middleware, attach_to=middleware_or_request
)
return partial(self.register_middleware, attach_to=middleware_or_request)

# Static Files
def static(
Expand Down Expand Up @@ -766,9 +758,7 @@ def url_for(self, view_name: str, **kwargs):

uri, route = self.router.find_route_by_view_name(view_name, **kw)
if not (uri and route):
raise URLBuildError(
f"Endpoint with name `{view_name}` was not found"
)
raise URLBuildError(f"Endpoint with name `{view_name}` was not found")

# If the route has host defined, split that off
# TODO: Retain netloc and path separately in Route objects
Expand Down Expand Up @@ -904,9 +894,7 @@ async def handle_request(self, request, write_callback, stream_callback):
# -------------------------------------------- #
# Request Middleware
# -------------------------------------------- #
response = await self._run_request_middleware(
request, request_name=name
)
response = await self._run_request_middleware(request, request_name=name)
# No middleware results
if not response:
# -------------------------------------------- #
Expand All @@ -923,13 +911,10 @@ async def handle_request(self, request, write_callback, stream_callback):
)
else:
if not getattr(handler, "__blueprintname__", False):
request.endpoint = self._build_endpoint_name(
handler.__name__
)
request.endpoint = self._build_endpoint_name(handler.__name__)
else:
request.endpoint = self._build_endpoint_name(
getattr(handler, "__blueprintname__", ""),
handler.__name__,
getattr(handler, "__blueprintname__", ""), handler.__name__,
)

# Run response handler
Expand All @@ -954,13 +939,10 @@ async def handle_request(self, request, write_callback, stream_callback):
response = await response
except Exception as e:
if isinstance(e, SanicException):
response = self.error_handler.default(
request=request, exception=e
)
response = self.error_handler.default(request=request, exception=e)
elif self.debug:
response = HTTPResponse(
f"Error while "
f"handling error: {e}\nStack: {format_exc()}",
f"Error while " f"handling error: {e}\nStack: {format_exc()}",
status=500,
)
else:
Expand All @@ -983,16 +965,13 @@ async def handle_request(self, request, write_callback, stream_callback):
cancelled = True
except BaseException:
error_logger.exception(
"Exception occurred in one of response "
"middleware handlers"
"Exception occurred in one of response " "middleware handlers"
)
if cancelled:
raise CancelledError()

# pass the response to the correct callback
if write_callback is None or isinstance(
response, StreamingHTTPResponse
):
if write_callback is None or isinstance(response, StreamingHTTPResponse):
if stream_callback:
await stream_callback(response)
else:
Expand Down Expand Up @@ -1088,15 +1067,12 @@ def run(
host, port = host or "127.0.0.1", port or 8000

if protocol is None:
protocol = (
WebSocketProtocol if self.websocket_enabled else HttpProtocol
)
protocol = WebSocketProtocol if self.websocket_enabled else HttpProtocol
if stop_event is not None:
if debug:
warnings.simplefilter("default")
warnings.warn(
"stop_event will be removed from future versions.",
DeprecationWarning,
"stop_event will be removed from future versions.", DeprecationWarning,
)
# if access_log is passed explicitly change config.ACCESS_LOG
if access_log is not None:
Expand Down Expand Up @@ -1125,14 +1101,13 @@ def run(
" using workers=1 instead"
)
workers = 1
self._signals.get("server").loop = self.loop
if workers == 1:
serve(**server_settings)
else:
serve_multiple(server_settings, workers)
except BaseException:
error_logger.exception(
"Experienced exception while trying to serve"
)
error_logger.exception("Experienced exception while trying to serve")
raise
finally:
self.is_running = False
Expand Down Expand Up @@ -1206,15 +1181,12 @@ async def create_server(
host, port = host or "127.0.0.1", port or 8000

if protocol is None:
protocol = (
WebSocketProtocol if self.websocket_enabled else HttpProtocol
)
protocol = WebSocketProtocol if self.websocket_enabled else HttpProtocol
if stop_event is not None:
if debug:
warnings.simplefilter("default")
warnings.warn(
"stop_event will be removed from future versions.",
DeprecationWarning,
"stop_event will be removed from future versions.", DeprecationWarning,
)
# if access_log is passed explicitly change config.ACCESS_LOG
if access_log is not None:
Expand All @@ -1235,8 +1207,7 @@ async def create_server(

# Trigger before_start events
await self.trigger_events(
server_settings.get("before_start", []),
server_settings.get("loop"),
server_settings.get("before_start", []), server_settings.get("loop"),
)

return await serve(
Expand All @@ -1252,12 +1223,11 @@ async def trigger_events(self, events, loop):
result = event(loop)
if isawaitable(result):
await result
await publish(event_name=event, signals=self.signals)

async def _run_request_middleware(self, request, request_name=None):
# The if improves speed. I don't know why
named_middleware = self.named_request_middleware.get(
request_name, deque()
)
named_middleware = self.named_request_middleware.get(request_name, deque())
applicable_middleware = self.request_middleware + named_middleware
if applicable_middleware:
for middleware in applicable_middleware:
Expand All @@ -1268,12 +1238,8 @@ async def _run_request_middleware(self, request, request_name=None):
return response
return None

async def _run_response_middleware(
self, request, response, request_name=None
):
named_middleware = self.named_response_middleware.get(
request_name, deque()
)
async def _run_response_middleware(self, request, response, request_name=None):
named_middleware = self.named_response_middleware.get(request_name, deque())
applicable_middleware = self.response_middleware + named_middleware
if applicable_middleware:
for middleware in applicable_middleware:
Expand Down Expand Up @@ -1316,8 +1282,7 @@ def _helper(
if debug:
warnings.simplefilter("default")
warnings.warn(
"stop_event will be removed from future versions.",
DeprecationWarning,
"stop_event will be removed from future versions.", DeprecationWarning,
)
if self.config.PROXIES_COUNT and self.config.PROXIES_COUNT < 0:
raise ValueError(
Expand Down Expand Up @@ -1363,14 +1328,9 @@ def _helper(
if self.configure_logging and debug:
logger.setLevel(logging.DEBUG)

if (
self.config.LOGO
and os.environ.get("SANIC_SERVER_RUNNING") != "true"
):
if self.config.LOGO and os.environ.get("SANIC_SERVER_RUNNING") != "true":
logger.debug(
self.config.LOGO
if isinstance(self.config.LOGO, str)
else BASE_LOGO
self.config.LOGO if isinstance(self.config.LOGO, str) else BASE_LOGO
)

if run_async:
Expand Down
Loading