From a4beed3545dbefd58e1430bb13e9f1da1dfff428 Mon Sep 17 00:00:00 2001 From: Ashley Sommer Date: Fri, 7 Jan 2022 10:01:21 +1000 Subject: [PATCH 1/7] New config option to skip touchup step. Sets fail_not_found to False if touchup skipped. --- sanic/app.py | 10 ++++++++-- sanic/config.py | 2 ++ tests/test_touchup.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 97c7a177b2..426a4f5844 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -101,7 +101,7 @@ from sanic.server import serve, serve_multiple, serve_single, try_use_uvloop from sanic.server.protocols.websocket_protocol import WebSocketProtocol from sanic.server.websockets.impl import ConnectionClosed -from sanic.signals import Signal, SignalRouter +from sanic.signals import RESERVED_NAMESPACES, Signal, SignalRouter from sanic.tls import process_to_context from sanic.touchup import TouchUp, TouchUpMeta @@ -443,6 +443,11 @@ def dispatch( inline: bool = False, reverse: bool = False, ) -> Coroutine[Any, Any, Awaitable[Any]]: + if self.config.get("SKIP_TOUCHUP", False): + event_ns = event.split(".", 1)[0] + if event_ns in RESERVED_NAMESPACES: + fail_not_found = False + return self.signal_router.dispatch( event, context=context, @@ -1976,7 +1981,8 @@ async def _startup(self): # Startup time optimizations ErrorHandler.finalize(self.error_handler, config=self.config) - TouchUp.run(self) + if not self.config.get("SKIP_TOUCHUP", False): + TouchUp.run(self) self.state.is_started = True diff --git a/sanic/config.py b/sanic/config.py index ac147ef352..e8ed22da9b 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -38,6 +38,7 @@ "REQUEST_MAX_SIZE": 100000000, # 100 megabytes "REQUEST_TIMEOUT": 60, # 60 seconds "RESPONSE_TIMEOUT": 60, # 60 seconds + "SKIP_TOUCHUP": False, "USE_UVLOOP": _default, "WEBSOCKET_MAX_SIZE": 2 ** 20, # 1 megabyte "WEBSOCKET_PING_INTERVAL": 20, @@ -81,6 +82,7 @@ class Config(dict, metaclass=DescriptorMeta): REQUEST_TIMEOUT: int RESPONSE_TIMEOUT: int SERVER_NAME: str + SKIP_TOUCHUP: bool USE_UVLOOP: Union[Default, bool] WEBSOCKET_MAX_SIZE: int WEBSOCKET_PING_INTERVAL: int diff --git a/tests/test_touchup.py b/tests/test_touchup.py index 031a15e80d..ed21c21c91 100644 --- a/tests/test_touchup.py +++ b/tests/test_touchup.py @@ -2,6 +2,8 @@ import pytest +from sanic_routing.exceptions import NotFound + from sanic.signals import RESERVED_NAMESPACES from sanic.touchup import TouchUp @@ -28,3 +30,30 @@ async def test_ode_removes_dispatch_events(app, caplog, verbosity, result): ) in logs ) is result + + +@pytest.mark.parametrize("skip_it,result", ((False, True), (True, False))) +async def test_skip_touchup(app, caplog, skip_it, result): + app.config.SKIP_TOUCHUP = skip_it + with caplog.at_level(logging.DEBUG, logger="sanic.root"): + app.state.verbosity = 2 + await app._startup() + logs = caplog.record_tuples + + for signal in RESERVED_NAMESPACES["http"]: + assert ( + ( + "sanic.root", + logging.DEBUG, + f"Disabling event: {signal}", + ) + in logs + ) is result + not_found_exceptions = 0 + # Skip-touchup disables NotFound exceptions on the dispatcher + for signal in RESERVED_NAMESPACES["http"]: + try: + await app.dispatch(event=signal, inline=True) + except NotFound: + not_found_exceptions += 1 + assert (not_found_exceptions > 0) is result From f831b71bc4db1828380ce046563ce911911075c3 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Tue, 18 Jan 2022 11:44:49 +0200 Subject: [PATCH 2/7] Downgrade warnings to backwater debug messages --- sanic/asgi.py | 56 +++++++++++++++++++++++++--------------------- tests/test_asgi.py | 6 ++--- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/sanic/asgi.py b/sanic/asgi.py index 5ef15a916e..2614016866 100644 --- a/sanic/asgi.py +++ b/sanic/asgi.py @@ -1,14 +1,15 @@ +from __future__ import annotations + import warnings -from typing import Optional +from typing import TYPE_CHECKING, Optional from urllib.parse import quote -import sanic.app # noqa - from sanic.compat import Header from sanic.exceptions import ServerError from sanic.helpers import _default from sanic.http import Stage +from sanic.log import logger from sanic.models.asgi import ASGIReceive, ASGIScope, ASGISend, MockTransport from sanic.request import Request from sanic.response import BaseHTTPResponse @@ -16,30 +17,35 @@ from sanic.server.websockets.connection import WebSocketConnection +if TYPE_CHECKING: # no cov + from sanic import Sanic + + class Lifespan: - def __init__(self, asgi_app: "ASGIApp") -> None: + def __init__(self, asgi_app: ASGIApp) -> None: self.asgi_app = asgi_app - if ( - "server.init.before" - in self.asgi_app.sanic_app.signal_router.name_index - ): - warnings.warn( - 'You have set a listener for "before_server_start" ' - "in ASGI mode. " - "It will be executed as early as possible, but not before " - "the ASGI server is started." - ) - if ( - "server.shutdown.after" - in self.asgi_app.sanic_app.signal_router.name_index - ): - warnings.warn( - 'You have set a listener for "after_server_stop" ' - "in ASGI mode. " - "It will be executed as late as possible, but not after " - "the ASGI server is stopped." - ) + if self.asgi_app.sanic_app.state.verbosity > 0: + if ( + "server.init.before" + in self.asgi_app.sanic_app.signal_router.name_index + ): + logger.debug( + 'You have set a listener for "before_server_start" ' + "in ASGI mode. " + "It will be executed as early as possible, but not before " + "the ASGI server is started." + ) + if ( + "server.shutdown.after" + in self.asgi_app.sanic_app.signal_router.name_index + ): + logger.debug( + 'You have set a listener for "after_server_stop" ' + "in ASGI mode. " + "It will be executed as late as possible, but not after " + "the ASGI server is stopped." + ) async def startup(self) -> None: """ @@ -88,7 +94,7 @@ async def __call__( class ASGIApp: - sanic_app: "sanic.app.Sanic" + sanic_app: Sanic request: Request transport: MockTransport lifespan: Lifespan diff --git a/tests/test_asgi.py b/tests/test_asgi.py index d00a70bdb8..24137fcc95 100644 --- a/tests/test_asgi.py +++ b/tests/test_asgi.py @@ -82,8 +82,7 @@ def install_signal_handlers(self): config = uvicorn.Config(app=app, loop="asyncio", limit_max_requests=0) server = CustomServer(config=config) - with pytest.warns(UserWarning): - server.run() + server.run() all_tasks = asyncio.all_tasks(asyncio.get_event_loop()) for task in all_tasks: @@ -132,8 +131,7 @@ def install_signal_handlers(self): config = uvicorn.Config(app=app, loop="asyncio", limit_max_requests=0) server = CustomServer(config=config) - with pytest.warns(UserWarning): - server.run() + server.run() all_tasks = asyncio.all_tasks(asyncio.get_event_loop()) for task in all_tasks: From ad47ff7c434d479d4d4338cbbca42948da88dae5 Mon Sep 17 00:00:00 2001 From: Ashley Sommer Date: Wed, 19 Jan 2022 11:06:55 +1000 Subject: [PATCH 3/7] move skip-not-found logic into signal router code. Only check it at startup. Add a test to ensure user-defined signals will still throw not-found. Add comment to cli args about future change in default behaviour. --- sanic/app.py | 12 ++++-------- sanic/cli/app.py | 6 +++--- sanic/cli/arguments.py | 6 +++--- sanic/signals.py | 4 +++- tests/test_touchup.py | 19 +++++++++++++++++++ 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 426a4f5844..c7905f5968 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -443,11 +443,6 @@ def dispatch( inline: bool = False, reverse: bool = False, ) -> Coroutine[Any, Any, Awaitable[Any]]: - if self.config.get("SKIP_TOUCHUP", False): - event_ns = event.split(".", 1)[0] - if event_ns in RESERVED_NAMESPACES: - fail_not_found = False - return self.signal_router.dispatch( event, context=context, @@ -1947,7 +1942,8 @@ def finalize(self): if not Sanic.test_mode: raise e - def signalize(self): + def signalize(self, allow_fail_builtin=True): + self.signal_router.allow_fail_builtin = allow_fail_builtin try: self.signal_router.finalize() except FinalizationError as e: @@ -1964,7 +1960,7 @@ async def _startup(self): self.ext._display() # Setup routers - self.signalize() + self.signalize(not self.config.SKIP_TOUCHUP) self.finalize() # TODO: Replace in v22.6 to check against apps in app registry @@ -1981,7 +1977,7 @@ async def _startup(self): # Startup time optimizations ErrorHandler.finalize(self.error_handler, config=self.config) - if not self.config.get("SKIP_TOUCHUP", False): + if not self.config.SKIP_TOUCHUP: TouchUp.run(self) self.state.is_started = True diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 3001b6e1fa..6be257a389 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -81,9 +81,9 @@ def run(self): def _precheck(self): if self.args.debug and self.main_process: error_logger.warning( - "Starting in v22.3, --debug will no " - "longer automatically run the auto-reloader.\n Switch to " - "--dev to continue using that functionality." + "Starting in v22.3, --debug will no longer automatically run " + "the auto-reloader or perform runtime code optimization.\n" + "Switch to --dev to continue using that functionality." ) # # Custom TLS mismatch handling for better diagnostics diff --git a/sanic/cli/arguments.py b/sanic/cli/arguments.py index 20644bdc45..7e80292b65 100644 --- a/sanic/cli/arguments.py +++ b/sanic/cli/arguments.py @@ -189,9 +189,9 @@ def attach(self): action="store_true", help=( "Currently is an alias for --debug. But starting in v22.3, \n" - "--debug will no longer automatically trigger auto_restart. \n" - "However, --dev will continue, effectively making it the \n" - "same as debug + auto_reload." + "--debug will no longer automatically trigger auto_restart or " + "runtime code optimization.\n--dev will continue, effectively " + "making it the same as debug + auto_reload + optimization." ), ) self.container.add_argument( diff --git a/sanic/signals.py b/sanic/signals.py index f4061b69cc..d62a117c52 100644 --- a/sanic/signals.py +++ b/sanic/signals.py @@ -80,6 +80,7 @@ def __init__(self) -> None: group_class=SignalGroup, stacking=True, ) + self.allow_fail_builtin = True self.ctx.loop = None def get( # type: ignore @@ -129,7 +130,8 @@ async def _dispatch( try: group, handlers, params = self.get(event, condition=condition) except NotFound as e: - if fail_not_found: + is_reserved = event.split(".", 1)[0] in RESERVED_NAMESPACES + if fail_not_found and (not is_reserved or self.allow_fail_builtin): raise e else: if self.ctx.app.debug and self.ctx.app.state.verbosity >= 1: diff --git a/tests/test_touchup.py b/tests/test_touchup.py index ed21c21c91..999b4dcf3b 100644 --- a/tests/test_touchup.py +++ b/tests/test_touchup.py @@ -38,6 +38,7 @@ async def test_skip_touchup(app, caplog, skip_it, result): with caplog.at_level(logging.DEBUG, logger="sanic.root"): app.state.verbosity = 2 await app._startup() + assert app.signal_router.allow_fail_builtin is (not skip_it) logs = caplog.record_tuples for signal in RESERVED_NAMESPACES["http"]: @@ -57,3 +58,21 @@ async def test_skip_touchup(app, caplog, skip_it, result): except NotFound: not_found_exceptions += 1 assert (not_found_exceptions > 0) is result + + +@pytest.mark.parametrize("skip_it,result", ((False, True), (True, True))) +async def test_skip_touchup_non_reserved(app, caplog, skip_it, result): + app.config.SKIP_TOUCHUP = skip_it + + @app.signal("foo.bar.one") + def sync_signal(*_): + ... + await app._startup() + assert app.signal_router.allow_fail_builtin is (not skip_it) + not_found_exception = False + # Skip-touchup doesn't disable NotFound exceptions for user-defined signals + try: + await app.dispatch(event="foo.baz.two", inline=True) + except NotFound: + not_found_exception = True + assert not_found_exception is result From a6aaaed7adee5f705cf1253b55fe78e1b876ab7f Mon Sep 17 00:00:00 2001 From: Ashley Sommer Date: Wed, 19 Jan 2022 11:31:12 +1000 Subject: [PATCH 4/7] remove unused import --- sanic/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic/app.py b/sanic/app.py index 8914aa6ed1..8d120e59ef 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -89,7 +89,7 @@ from sanic.response import BaseHTTPResponse, HTTPResponse, ResponseStream from sanic.router import Router from sanic.server.websockets.impl import ConnectionClosed -from sanic.signals import RESERVED_NAMESPACES, Signal, SignalRouter +from sanic.signals import Signal, SignalRouter from sanic.touchup import TouchUp, TouchUpMeta From 87769083bdbc43f32f8ced69ea171d884fe78b72 Mon Sep 17 00:00:00 2001 From: Ashley Sommer Date: Wed, 19 Jan 2022 11:41:02 +1000 Subject: [PATCH 5/7] revert accidental asgi.py changes after merge conflict --- sanic/asgi.py | 56 +++++++++++++++++++++------------------------- tests/test_asgi.py | 6 +++-- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/sanic/asgi.py b/sanic/asgi.py index 2614016866..5ef15a916e 100644 --- a/sanic/asgi.py +++ b/sanic/asgi.py @@ -1,15 +1,14 @@ -from __future__ import annotations - import warnings -from typing import TYPE_CHECKING, Optional +from typing import Optional from urllib.parse import quote +import sanic.app # noqa + from sanic.compat import Header from sanic.exceptions import ServerError from sanic.helpers import _default from sanic.http import Stage -from sanic.log import logger from sanic.models.asgi import ASGIReceive, ASGIScope, ASGISend, MockTransport from sanic.request import Request from sanic.response import BaseHTTPResponse @@ -17,35 +16,30 @@ from sanic.server.websockets.connection import WebSocketConnection -if TYPE_CHECKING: # no cov - from sanic import Sanic - - class Lifespan: - def __init__(self, asgi_app: ASGIApp) -> None: + def __init__(self, asgi_app: "ASGIApp") -> None: self.asgi_app = asgi_app - if self.asgi_app.sanic_app.state.verbosity > 0: - if ( - "server.init.before" - in self.asgi_app.sanic_app.signal_router.name_index - ): - logger.debug( - 'You have set a listener for "before_server_start" ' - "in ASGI mode. " - "It will be executed as early as possible, but not before " - "the ASGI server is started." - ) - if ( - "server.shutdown.after" - in self.asgi_app.sanic_app.signal_router.name_index - ): - logger.debug( - 'You have set a listener for "after_server_stop" ' - "in ASGI mode. " - "It will be executed as late as possible, but not after " - "the ASGI server is stopped." - ) + if ( + "server.init.before" + in self.asgi_app.sanic_app.signal_router.name_index + ): + warnings.warn( + 'You have set a listener for "before_server_start" ' + "in ASGI mode. " + "It will be executed as early as possible, but not before " + "the ASGI server is started." + ) + if ( + "server.shutdown.after" + in self.asgi_app.sanic_app.signal_router.name_index + ): + warnings.warn( + 'You have set a listener for "after_server_stop" ' + "in ASGI mode. " + "It will be executed as late as possible, but not after " + "the ASGI server is stopped." + ) async def startup(self) -> None: """ @@ -94,7 +88,7 @@ async def __call__( class ASGIApp: - sanic_app: Sanic + sanic_app: "sanic.app.Sanic" request: Request transport: MockTransport lifespan: Lifespan diff --git a/tests/test_asgi.py b/tests/test_asgi.py index 24137fcc95..d00a70bdb8 100644 --- a/tests/test_asgi.py +++ b/tests/test_asgi.py @@ -82,7 +82,8 @@ def install_signal_handlers(self): config = uvicorn.Config(app=app, loop="asyncio", limit_max_requests=0) server = CustomServer(config=config) - server.run() + with pytest.warns(UserWarning): + server.run() all_tasks = asyncio.all_tasks(asyncio.get_event_loop()) for task in all_tasks: @@ -131,7 +132,8 @@ def install_signal_handlers(self): config = uvicorn.Config(app=app, loop="asyncio", limit_max_requests=0) server = CustomServer(config=config) - server.run() + with pytest.warns(UserWarning): + server.run() all_tasks = asyncio.all_tasks(asyncio.get_event_loop()) for task in all_tasks: From ca5ff14682f6d92287f5060eded5b6360ee6338b Mon Sep 17 00:00:00 2001 From: Ashley Sommer Date: Thu, 24 Mar 2022 12:48:06 +1000 Subject: [PATCH 6/7] invert SKIP_TOUCHUP logic, its now TOUCHUP, it defaults to True Now set TOUCHUP config to False when running in debug mode --- sanic/app.py | 7 +++++-- sanic/config.py | 4 ++-- tests/test_touchup.py | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 8d120e59ef..2d563aa2e3 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1526,8 +1526,11 @@ async def _startup(self): if hasattr(self, "_ext"): self.ext._display() + if self.state.is_debug: + self.config.TOUCHUP = False + # Setup routers - self.signalize(not self.config.SKIP_TOUCHUP) + self.signalize(self.config.TOUCHUP) self.finalize() # TODO: Replace in v22.6 to check against apps in app registry @@ -1547,7 +1550,7 @@ async def _startup(self): # TODO: # - Raise warning if secondary apps have error handler config ErrorHandler.finalize(self.error_handler, config=self.config) - if not self.config.SKIP_TOUCHUP: + if self.config.TOUCHUP: TouchUp.run(self) self.state.is_started = True diff --git a/sanic/config.py b/sanic/config.py index e8ed22da9b..2152814799 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -38,7 +38,7 @@ "REQUEST_MAX_SIZE": 100000000, # 100 megabytes "REQUEST_TIMEOUT": 60, # 60 seconds "RESPONSE_TIMEOUT": 60, # 60 seconds - "SKIP_TOUCHUP": False, + "TOUCHUP": True, "USE_UVLOOP": _default, "WEBSOCKET_MAX_SIZE": 2 ** 20, # 1 megabyte "WEBSOCKET_PING_INTERVAL": 20, @@ -82,7 +82,7 @@ class Config(dict, metaclass=DescriptorMeta): REQUEST_TIMEOUT: int RESPONSE_TIMEOUT: int SERVER_NAME: str - SKIP_TOUCHUP: bool + TOUCHUP: bool USE_UVLOOP: Union[Default, bool] WEBSOCKET_MAX_SIZE: int WEBSOCKET_PING_INTERVAL: int diff --git a/tests/test_touchup.py b/tests/test_touchup.py index 999b4dcf3b..3250c9ef2d 100644 --- a/tests/test_touchup.py +++ b/tests/test_touchup.py @@ -34,7 +34,7 @@ async def test_ode_removes_dispatch_events(app, caplog, verbosity, result): @pytest.mark.parametrize("skip_it,result", ((False, True), (True, False))) async def test_skip_touchup(app, caplog, skip_it, result): - app.config.SKIP_TOUCHUP = skip_it + app.config.TOUCHUP = (not skip_it) with caplog.at_level(logging.DEBUG, logger="sanic.root"): app.state.verbosity = 2 await app._startup() From 0cd6227c43967f6c8250f322ccaead6cfd35f582 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Thu, 24 Mar 2022 12:16:50 +0200 Subject: [PATCH 7/7] Update unit test to new config name --- tests/test_touchup.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_touchup.py b/tests/test_touchup.py index 3250c9ef2d..8514268544 100644 --- a/tests/test_touchup.py +++ b/tests/test_touchup.py @@ -34,7 +34,7 @@ async def test_ode_removes_dispatch_events(app, caplog, verbosity, result): @pytest.mark.parametrize("skip_it,result", ((False, True), (True, False))) async def test_skip_touchup(app, caplog, skip_it, result): - app.config.TOUCHUP = (not skip_it) + app.config.TOUCHUP = not skip_it with caplog.at_level(logging.DEBUG, logger="sanic.root"): app.state.verbosity = 2 await app._startup() @@ -62,11 +62,12 @@ async def test_skip_touchup(app, caplog, skip_it, result): @pytest.mark.parametrize("skip_it,result", ((False, True), (True, True))) async def test_skip_touchup_non_reserved(app, caplog, skip_it, result): - app.config.SKIP_TOUCHUP = skip_it + app.config.TOUCHUP = not skip_it @app.signal("foo.bar.one") def sync_signal(*_): ... + await app._startup() assert app.signal_router.allow_fail_builtin is (not skip_it) not_found_exception = False