From 59ccf711fada880492b4435f030850c4bb60ee6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=CC=80ng=20X=2E=20Le=CC=82?= Date: Thu, 18 Apr 2019 12:03:33 +0700 Subject: [PATCH 01/22] Fix watchdog reload worker repeatedly if there are multiple changed files --- sanic/reloader_helpers.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/sanic/reloader_helpers.py b/sanic/reloader_helpers.py index b58391f64b..5e1338a43b 100644 --- a/sanic/reloader_helpers.py +++ b/sanic/reloader_helpers.py @@ -121,7 +121,7 @@ def kill_process_children(pid): pass # should signal error here -def kill_program_completly(proc): +def kill_program_completely(proc): """Kill worker and it's child processes and exit. :param proc: worker process (process ID) @@ -141,12 +141,14 @@ def watchdog(sleep_interval): mtimes = {} worker_process = restart_with_reloader() signal.signal( - signal.SIGTERM, lambda *args: kill_program_completly(worker_process) + signal.SIGTERM, lambda *args: kill_program_completely(worker_process) ) signal.signal( - signal.SIGINT, lambda *args: kill_program_completly(worker_process) + signal.SIGINT, lambda *args: kill_program_completely(worker_process) ) while True: + need_reload = False + for filename in _iter_module_files(): try: mtime = os.stat(filename).st_mtime @@ -156,12 +158,13 @@ def watchdog(sleep_interval): old_time = mtimes.get(filename) if old_time is None: mtimes[filename] = mtime - continue elif mtime > old_time: - kill_process_children(worker_process.pid) - worker_process.terminate() - worker_process = restart_with_reloader() mtimes[filename] = mtime - break + need_reload = True + + if need_reload: + kill_process_children(worker_process.pid) + worker_process.terminate() + worker_process = restart_with_reloader() sleep(sleep_interval) From 8ebac9bca0bcef06096ee4c6c19c8a6cf325f418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 13:03:00 +0300 Subject: [PATCH 02/22] Simplify autoreloader, don't need multiprocessing.Process. Now works on OSX py38. --- sanic/reloader_helpers.py | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/sanic/reloader_helpers.py b/sanic/reloader_helpers.py index 5e1338a43b..89242439c1 100644 --- a/sanic/reloader_helpers.py +++ b/sanic/reloader_helpers.py @@ -3,7 +3,6 @@ import subprocess import sys -from multiprocessing import Process from time import sleep @@ -35,35 +34,27 @@ def _iter_module_files(): def _get_args_for_reloading(): """Returns the executable.""" - rv = [sys.executable] main_module = sys.modules["__main__"] mod_spec = getattr(main_module, "__spec__", None) + if sys.argv[0] in ("", "-c"): + raise RuntimeError( + f"Autoreloader cannot work with argv[0]={sys.argv[0]!r}" + ) if mod_spec: # Parent exe was launched as a module rather than a script - rv.extend(["-m", mod_spec.name]) - if len(sys.argv) > 1: - rv.extend(sys.argv[1:]) - else: - rv.extend(sys.argv) - return rv + return [sys.executable, "-m", mod_spec.name] + sys.argv[1:] + return [sys.executable] + sys.argv def restart_with_reloader(): """Create a new process and a subprocess in it with the same arguments as this one. """ - cwd = os.getcwd() - args = _get_args_for_reloading() - new_environ = os.environ.copy() - new_environ["SANIC_SERVER_RUNNING"] = "true" - cmd = " ".join(args) - worker_process = Process( - target=subprocess.call, - args=(cmd,), - kwargs={"cwd": cwd, "shell": True, "env": new_environ}, + return subprocess.Popen( + _get_args_for_reloading(), + cwd=os.getcwd(), + env={**os.environ, "SANIC_SERVER_RUNNING": "true"}, ) - worker_process.start() - return worker_process def kill_process_children_unix(pid): From 1bf7e2f1b0636c9a1a1af147f3c130f9734b1bf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 13:03:19 +0300 Subject: [PATCH 03/22] Allow autoreloader with multiple workers and run it earlier. --- sanic/app.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index c38f4f7cb0..d228e29256 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1117,6 +1117,15 @@ def run( # Allow for overriding either of the defaults auto_reload = kwargs.get("auto_reload", auto_reload) + if auto_reload: + if os.name != "posix": + # This condition must be removed after implementing + # auto reloader for other operating systems. + raise NotImplementedError + + if os.environ.get("SANIC_SERVER_RUNNING") != "true": + return reloader_helpers.watchdog(2) + if sock is None: host, port = host or "127.0.0.1", port or 8000 @@ -1158,18 +1167,7 @@ def run( ) workers = 1 if workers == 1: - if auto_reload and os.name != "posix": - # This condition must be removed after implementing - # auto reloader for other operating systems. - raise NotImplementedError - - if ( - auto_reload - and os.environ.get("SANIC_SERVER_RUNNING") != "true" - ): - reloader_helpers.watchdog(2) - else: - serve(**server_settings) + serve(**server_settings) else: serve_multiple(server_settings, workers) except BaseException: From c1bab69f380eec4e658e439abfbf216896141464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 04:32:18 -0700 Subject: [PATCH 04/22] This works OK on Windows too. --- sanic/app.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index d228e29256..a547cbea1c 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1118,11 +1118,6 @@ def run( auto_reload = kwargs.get("auto_reload", auto_reload) if auto_reload: - if os.name != "posix": - # This condition must be removed after implementing - # auto reloader for other operating systems. - raise NotImplementedError - if os.environ.get("SANIC_SERVER_RUNNING") != "true": return reloader_helpers.watchdog(2) From d04fbfc9bf80b41be7049c10ad86b0677b8f9162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 14:50:54 +0300 Subject: [PATCH 05/22] I don't see how cwd could be different here. --- sanic/reloader_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sanic/reloader_helpers.py b/sanic/reloader_helpers.py index 89242439c1..112744859b 100644 --- a/sanic/reloader_helpers.py +++ b/sanic/reloader_helpers.py @@ -52,7 +52,6 @@ def restart_with_reloader(): """ return subprocess.Popen( _get_args_for_reloading(), - cwd=os.getcwd(), env={**os.environ, "SANIC_SERVER_RUNNING": "true"}, ) From 261d15e32725f7d457a86d7c1fa2b114b95cb587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 14:51:30 +0300 Subject: [PATCH 06/22] app.run and app.create_server argument fixup. --- sanic/app.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index a547cbea1c..4d39738265 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1060,7 +1060,9 @@ def run( self, host: Optional[str] = None, port: Optional[int] = None, + *, debug: bool = False, + auto_reload: Optional[bool] = None, ssl: Union[dict, SSLContext, None] = None, sock: Optional[socket] = None, workers: int = 1, @@ -1069,7 +1071,7 @@ def run( stop_event: Any = None, register_sys_signals: bool = True, access_log: Optional[bool] = None, - **kwargs: Any, + loop: None = None, ) -> None: """Run the HTTP Server and listen until keyboard interrupt or term signal. On termination, drain connections before closing. @@ -1080,6 +1082,9 @@ def run( :type port: int :param debug: Enables debug output (slows server) :type debug: bool + :param auto_reload: Reload app whenever its source code is changed. + Enabled by default in debug mode. + :type auto_relaod: bool :param ssl: SSLContext, or location of certificate and key for SSL encryption of worker(s) :type ssl: SSLContext or dict @@ -1101,7 +1106,7 @@ def run( :type access_log: bool :return: Nothing """ - if "loop" in kwargs: + if loop is not None: raise TypeError( "loop is not a valid argument. To use an existing loop, " "change to create_server().\nSee more: " @@ -1109,15 +1114,7 @@ def run( "#asynchronous-support" ) - # Default auto_reload to false - auto_reload = False - # If debug is set, default it to true (unless on windows) - if debug and os.name == "posix": - auto_reload = True - # Allow for overriding either of the defaults - auto_reload = kwargs.get("auto_reload", auto_reload) - - if auto_reload: + if auto_reload or auto_reload is None and debug: if os.environ.get("SANIC_SERVER_RUNNING") != "true": return reloader_helpers.watchdog(2) @@ -1184,6 +1181,7 @@ async def create_server( self, host: Optional[str] = None, port: Optional[int] = None, + *, debug: bool = False, ssl: Union[dict, SSLContext, None] = None, sock: Optional[socket] = None, From 2cb410705d4f60c5810ebfbc0a4ab6701531102d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 17:53:18 +0300 Subject: [PATCH 07/22] Add test for auto_reload (coverage not working unfortunately). --- tests/test_auto_reload.py | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/test_auto_reload.py diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py new file mode 100644 index 0000000000..7a63249f5a --- /dev/null +++ b/tests/test_auto_reload.py @@ -0,0 +1,46 @@ +import os +import subprocess +import secrets +import sys + +from tempfile import TemporaryDirectory +from textwrap import dedent + +import pytest + +def write_app(filename, **runargs): + text = secrets.token_urlsafe() + with open(filename, "w") as f: + f.write(dedent(f"""\ + from sanic import Sanic + from sanic.response import text + + app = Sanic(__name__) + + @app.listener("after_server_start") + def complete(*args): + print("complete", {text!r}) + + kwargs = {runargs!r} + app.run(port=42101, **kwargs) + """ + )) + return text + +@pytest.mark.parametrize("runargs", [ + dict(auto_reload=True), + dict(debug=True), +]) +async def test_reloader_live(runargs): + with TemporaryDirectory() as tmpdir: + filename = os.path.join(tmpdir, "reloaderapp.py") + text = write_app(filename, **runargs) + with subprocess.Popen([sys.executable, filename], stdout=subprocess.PIPE) as proc: + line = (l.decode() for l in proc.stdout if l.startswith(b"complete")) + try: + assert next(line) == f"complete {text}\n" + # Edit source code and try again + text = write_app(filename) + assert next(line) == f"complete {text}\n" + finally: + proc.terminate() From e522c722fa40cf9535f0f3f4cf4375c7f0eded76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 17:55:55 +0300 Subject: [PATCH 08/22] Reloader cleanup, don't use external kill commands and exit normally. --- sanic/app.py | 2 +- sanic/reloader_helpers.py | 128 ++++++++++---------------------------- 2 files changed, 35 insertions(+), 95 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 4d39738265..0cae373c69 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1116,7 +1116,7 @@ def run( if auto_reload or auto_reload is None and debug: if os.environ.get("SANIC_SERVER_RUNNING") != "true": - return reloader_helpers.watchdog(2) + return reloader_helpers.watchdog(1.0) if sock is None: host, port = host or "127.0.0.1", port or 8000 diff --git a/sanic/reloader_helpers.py b/sanic/reloader_helpers.py index 112744859b..000dc07abe 100644 --- a/sanic/reloader_helpers.py +++ b/sanic/reloader_helpers.py @@ -56,105 +56,45 @@ def restart_with_reloader(): ) -def kill_process_children_unix(pid): - """Find and kill child processes of a process (maximum two level). - - :param pid: PID of parent process (process ID) - :return: Nothing - """ - root_process_path = "/proc/{pid}/task/{pid}/children".format(pid=pid) - if not os.path.isfile(root_process_path): - return - with open(root_process_path) as children_list_file: - children_list_pid = children_list_file.read().split() - - for child_pid in children_list_pid: - children_proc_path = "/proc/%s/task/%s/children" % ( - child_pid, - child_pid, - ) - if not os.path.isfile(children_proc_path): - continue - with open(children_proc_path) as children_list_file_2: - children_list_pid_2 = children_list_file_2.read().split() - for _pid in children_list_pid_2: - try: - os.kill(int(_pid), signal.SIGTERM) - except ProcessLookupError: - continue - try: - os.kill(int(child_pid), signal.SIGTERM) - except ProcessLookupError: - continue - - -def kill_process_children_osx(pid): - """Find and kill child processes of a process. - - :param pid: PID of parent process (process ID) - :return: Nothing - """ - subprocess.run(["pkill", "-P", str(pid)]) - - -def kill_process_children(pid): - """Find and kill child processes of a process. - - :param pid: PID of parent process (process ID) - :return: Nothing - """ - if sys.platform == "darwin": - kill_process_children_osx(pid) - elif sys.platform == "linux": - kill_process_children_unix(pid) - else: - pass # should signal error here - - -def kill_program_completely(proc): - """Kill worker and it's child processes and exit. - - :param proc: worker process (process ID) - :return: Nothing - """ - kill_process_children(proc.pid) - proc.terminate() - os._exit(0) - - def watchdog(sleep_interval): """Watch project files, restart worker process if a change happened. :param sleep_interval: interval in second. :return: Nothing """ + + def interrupt_self(*args): + raise KeyboardInterrupt + mtimes = {} + signal.signal(signal.SIGTERM, interrupt_self) worker_process = restart_with_reloader() - signal.signal( - signal.SIGTERM, lambda *args: kill_program_completely(worker_process) - ) - signal.signal( - signal.SIGINT, lambda *args: kill_program_completely(worker_process) - ) - while True: - need_reload = False - - for filename in _iter_module_files(): - try: - mtime = os.stat(filename).st_mtime - except OSError: - continue - - old_time = mtimes.get(filename) - if old_time is None: - mtimes[filename] = mtime - elif mtime > old_time: - mtimes[filename] = mtime - need_reload = True - - if need_reload: - kill_process_children(worker_process.pid) - worker_process.terminate() - worker_process = restart_with_reloader() - - sleep(sleep_interval) + + try: + while True: + need_reload = False + + for filename in _iter_module_files(): + try: + mtime = os.stat(filename).st_mtime + except OSError: + continue + + old_time = mtimes.get(filename) + if old_time is None: + mtimes[filename] = mtime + elif mtime > old_time: + mtimes[filename] = mtime + need_reload = True + + if need_reload: + worker_process.terminate() + worker_process.wait() + worker_process = restart_with_reloader() + + sleep(sleep_interval) + except KeyboardInterrupt: + pass + finally: + worker_process.terminate() + worker_process.wait() From 613086ac7dbd46fc1c45f3f444c39550500168e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 18:22:24 +0300 Subject: [PATCH 09/22] Strip newlines on test output (Windows-compat). --- tests/test_auto_reload.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 7a63249f5a..0a6083cefe 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -36,11 +36,11 @@ async def test_reloader_live(runargs): filename = os.path.join(tmpdir, "reloaderapp.py") text = write_app(filename, **runargs) with subprocess.Popen([sys.executable, filename], stdout=subprocess.PIPE) as proc: - line = (l.decode() for l in proc.stdout if l.startswith(b"complete")) + line = (l.decode().strip() for l in proc.stdout if l.startswith(b"complete")) try: - assert next(line) == f"complete {text}\n" + assert next(line) == f"complete {text}" # Edit source code and try again text = write_app(filename) - assert next(line) == f"complete {text}\n" + assert next(line) == f"complete {text}" finally: proc.terminate() From 58a122671d4aae255aae896ca1d478e40d92a9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Tue, 31 Mar 2020 18:43:44 +0300 Subject: [PATCH 10/22] Report failures in test_auto_reload to avoid timeouts. --- tests/test_auto_reload.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 0a6083cefe..09d698c208 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -16,13 +16,22 @@ def write_app(filename, **runargs): from sanic.response import text app = Sanic(__name__) + started = False @app.listener("after_server_start") def complete(*args): + global started + started = True print("complete", {text!r}) kwargs = {runargs!r} - app.run(port=42101, **kwargs) + try: + app.run(port=42101, **kwargs) + except Exception as e: + print("complete", repr(e)) + else: + if not started: + print("complete SERVER DID NOT START") """ )) return text From 3c4a280e14a18307f5747209978d260a1e0fea46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 11:59:01 +0300 Subject: [PATCH 11/22] Use different test server ports to avoid binding problems on Windows. --- tests/test_auto_reload.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 09d698c208..fa079c8f76 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -24,21 +24,20 @@ def complete(*args): started = True print("complete", {text!r}) - kwargs = {runargs!r} try: - app.run(port=42101, **kwargs) + app.run(**{runargs!r}) except Exception as e: print("complete", repr(e)) else: if not started: - print("complete SERVER DID NOT START") + print("complete DID NOT START") """ )) return text -@pytest.mark.parametrize("runargs", [ - dict(auto_reload=True), - dict(debug=True), +@pytest.mark.parametrize("runargs,port", [ + dict(port=42102, auto_reload=True), + dict(port=42103, debug=True), ]) async def test_reloader_live(runargs): with TemporaryDirectory() as tmpdir: From 2cadf51f82e777d62c5d41b5ade411d19265bfc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 12:13:22 +0300 Subject: [PATCH 12/22] Fix previous commit --- tests/test_auto_reload.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index fa079c8f76..5045e0b451 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -35,7 +35,7 @@ def complete(*args): )) return text -@pytest.mark.parametrize("runargs,port", [ +@pytest.mark.parametrize("runargs", [ dict(port=42102, auto_reload=True), dict(port=42103, debug=True), ]) From 016770855ceaa69aed41e217ffe681379dfbb8cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 03:32:15 -0700 Subject: [PATCH 13/22] Listen on same port after reload. --- tests/test_auto_reload.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 5045e0b451..fd4b6375bf 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -13,7 +13,6 @@ def write_app(filename, **runargs): with open(filename, "w") as f: f.write(dedent(f"""\ from sanic import Sanic - from sanic.response import text app = Sanic(__name__) started = False @@ -48,7 +47,7 @@ async def test_reloader_live(runargs): try: assert next(line) == f"complete {text}" # Edit source code and try again - text = write_app(filename) + text = write_app(filename, **runargs) assert next(line) == f"complete {text}" finally: proc.terminate() From 2504a62a76e7beff5fab6564261b7489aac6b1a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 13:34:41 +0300 Subject: [PATCH 14/22] Show Goin' Fast banner on reloads. --- sanic/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic/app.py b/sanic/app.py index 0cae373c69..9daa887e6f 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1424,7 +1424,7 @@ def _helper( server_settings["run_async"] = True # Serve - if host and port and os.environ.get("SANIC_SERVER_RUNNING") != "true": + if host and port: proto = "http" if ssl is not None: proto = "https" From 6a3c46229aace5ce9e745e1601bb6d67ff4c9c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 15:29:47 +0300 Subject: [PATCH 15/22] More robust testing, also -m sanic. --- tests/test_auto_reload.py | 50 ++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index fd4b6375bf..44007418df 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -1,17 +1,20 @@ import os -import subprocess import secrets import sys +from subprocess import PIPE, Popen from tempfile import TemporaryDirectory from textwrap import dedent +from time import sleep import pytest + def write_app(filename, **runargs): text = secrets.token_urlsafe() with open(filename, "w") as f: f.write(dedent(f"""\ + import os from sanic import Sanic app = Sanic(__name__) @@ -21,33 +24,46 @@ def write_app(filename, **runargs): def complete(*args): global started started = True - print("complete", {text!r}) + print("complete", os.getpid(), {text!r}) - try: + if __name__ == "__main__": app.run(**{runargs!r}) - except Exception as e: - print("complete", repr(e)) - else: - if not started: - print("complete DID NOT START") """ )) return text -@pytest.mark.parametrize("runargs", [ - dict(port=42102, auto_reload=True), - dict(port=42103, debug=True), +def scanner(proc): + for line in proc.stdout: + line = line.decode().strip() + print(">", line) + if line.startswith("complete"): + yield line + +argv = dict( + script=[sys.executable, "reloader.py"], + module=[sys.executable, "-m", "reloader"], + sanic=[sys.executable, "-m", "sanic", "--port", "42104", "--debug", "reloader.app"], +) + +@pytest.mark.parametrize("runargs, mode", [ + (dict(port=42102, auto_reload=True), "script"), + (dict(port=42103, debug=True), "module"), + (dict(), "sanic"), ]) -async def test_reloader_live(runargs): +async def test_reloader_live(runargs, mode): with TemporaryDirectory() as tmpdir: - filename = os.path.join(tmpdir, "reloaderapp.py") + filename = os.path.join(tmpdir, "reloader.py") text = write_app(filename, **runargs) - with subprocess.Popen([sys.executable, filename], stdout=subprocess.PIPE) as proc: - line = (l.decode().strip() for l in proc.stdout if l.startswith(b"complete")) + with Popen(argv[mode], cwd=tmpdir, stdout=PIPE) as proc: try: - assert next(line) == f"complete {text}" + # Python apparently keeps using the old source sometimes if + # we don't sleep before rewrite (pycache timestamp problem?) + sleep(1) + line = scanner(proc) + assert text in next(line) # Edit source code and try again text = write_app(filename, **runargs) - assert next(line) == f"complete {text}" + #print(f"Replaced reloader {text}") + assert text in next(line) finally: proc.terminate() From b314b4b740b14693eef0f7f17da3ce752a0ccaae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 15:58:30 +0300 Subject: [PATCH 16/22] Add a timeout to terminate process --- tests/test_auto_reload.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 44007418df..6a31fb64bb 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -5,6 +5,7 @@ from subprocess import PIPE, Popen from tempfile import TemporaryDirectory from textwrap import dedent +from threading import Timer from time import sleep import pytest @@ -55,6 +56,8 @@ async def test_reloader_live(runargs, mode): filename = os.path.join(tmpdir, "reloader.py") text = write_app(filename, **runargs) with Popen(argv[mode], cwd=tmpdir, stdout=PIPE) as proc: + timeout = Timer(5, lambda: proc.terminate()) + timeout.start() try: # Python apparently keeps using the old source sometimes if # we don't sleep before rewrite (pycache timestamp problem?) @@ -63,7 +66,7 @@ async def test_reloader_live(runargs, mode): assert text in next(line) # Edit source code and try again text = write_app(filename, **runargs) - #print(f"Replaced reloader {text}") assert text in next(line) finally: + timeout.cancel() proc.terminate() From b3d1d13a26ba9d7b24e17f6c56bcc4ed28ea0fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 16:04:00 +0300 Subject: [PATCH 17/22] Try a workaround for tmpdir deletion on Windows. --- tests/test_auto_reload.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 6a31fb64bb..699cdac844 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -70,3 +70,6 @@ async def test_reloader_live(runargs, mode): finally: timeout.cancel() proc.terminate() + # Wait a while so that no-one is using tmpdir, to allow deleting it + if os.name == "nt": + sleep(1) From e601238a7520cc8ec75c757e348b67b0b79ca0d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 16:11:07 +0300 Subject: [PATCH 18/22] Join process also on error (context manager doesn't). --- tests/test_auto_reload.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 699cdac844..1bde6d8df4 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -55,21 +55,19 @@ async def test_reloader_live(runargs, mode): with TemporaryDirectory() as tmpdir: filename = os.path.join(tmpdir, "reloader.py") text = write_app(filename, **runargs) - with Popen(argv[mode], cwd=tmpdir, stdout=PIPE) as proc: + proc = Popen(argv[mode], cwd=tmpdir, stdout=PIPE) + try: timeout = Timer(5, lambda: proc.terminate()) timeout.start() - try: - # Python apparently keeps using the old source sometimes if - # we don't sleep before rewrite (pycache timestamp problem?) - sleep(1) - line = scanner(proc) - assert text in next(line) - # Edit source code and try again - text = write_app(filename, **runargs) - assert text in next(line) - finally: - timeout.cancel() - proc.terminate() - # Wait a while so that no-one is using tmpdir, to allow deleting it - if os.name == "nt": + # Python apparently keeps using the old source sometimes if + # we don't sleep before rewrite (pycache timestamp problem?) sleep(1) + line = scanner(proc) + assert text in next(line) + # Edit source code and try again + text = write_app(filename, **runargs) + assert text in next(line) + finally: + timeout.cancel() + proc.terminate() + proc.wait(timeout=1) From c870752f1cbe8c6dc7241d4c02cd914af5c2773d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 07:14:55 -0700 Subject: [PATCH 19/22] Cleaner autoreloader termination on Windows. --- sanic/reloader_helpers.py | 3 +++ tests/test_auto_reload.py | 24 +++++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sanic/reloader_helpers.py b/sanic/reloader_helpers.py index 000dc07abe..78750cbaf2 100644 --- a/sanic/reloader_helpers.py +++ b/sanic/reloader_helpers.py @@ -68,6 +68,9 @@ def interrupt_self(*args): mtimes = {} signal.signal(signal.SIGTERM, interrupt_self) + if os.name == "nt": + signal.signal(signal.SIGBREAK, interrupt_self) + worker_process = restart_with_reloader() try: diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index 1bde6d8df4..c77ef1efc0 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -11,6 +11,23 @@ import pytest +# We need to interrupt the autoreloader without killing it, so that the server gets terminated +# https://stefan.sofa-rockers.org/2013/08/15/handling-sub-process-hierarchies-python-linux-os-x/ + +try: + from signal import CTRL_BREAK_EVENT + from subprocess import CREATE_NEW_PROCESS_GROUP + + flags = CREATE_NEW_PROCESS_GROUP +except ImportError: + flags = 0 + +def terminate(proc): + if flags: + proc.send_signal(CTRL_BREAK_EVENT) + else: + proc.terminate() + def write_app(filename, **runargs): text = secrets.token_urlsafe() with open(filename, "w") as f: @@ -40,6 +57,7 @@ def scanner(proc): if line.startswith("complete"): yield line + argv = dict( script=[sys.executable, "reloader.py"], module=[sys.executable, "-m", "reloader"], @@ -55,9 +73,9 @@ async def test_reloader_live(runargs, mode): with TemporaryDirectory() as tmpdir: filename = os.path.join(tmpdir, "reloader.py") text = write_app(filename, **runargs) - proc = Popen(argv[mode], cwd=tmpdir, stdout=PIPE) + proc = Popen(argv[mode], cwd=tmpdir, stdout=PIPE, creationflags=flags) try: - timeout = Timer(5, lambda: proc.terminate()) + timeout = Timer(5, terminate, [proc]) timeout.start() # Python apparently keeps using the old source sometimes if # we don't sleep before rewrite (pycache timestamp problem?) @@ -69,5 +87,5 @@ async def test_reloader_live(runargs, mode): assert text in next(line) finally: timeout.cancel() - proc.terminate() + terminate(proc) proc.wait(timeout=1) From 1859331afa8ba7f4fac2263ce5f1a23379f8dfc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 17:19:05 +0300 Subject: [PATCH 20/22] Remove unused code. --- tests/test_auto_reload.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_auto_reload.py b/tests/test_auto_reload.py index c77ef1efc0..a140f3ce4f 100644 --- a/tests/test_auto_reload.py +++ b/tests/test_auto_reload.py @@ -36,12 +36,9 @@ def write_app(filename, **runargs): from sanic import Sanic app = Sanic(__name__) - started = False @app.listener("after_server_start") def complete(*args): - global started - started = True print("complete", os.getpid(), {text!r}) if __name__ == "__main__": From ae6ab3facd7f83a01498f5faaa78020a40aadb31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 17:19:45 +0300 Subject: [PATCH 21/22] Rename test. --- tests/{test_auto_reload.py => test_reloader.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_auto_reload.py => test_reloader.py} (100%) diff --git a/tests/test_auto_reload.py b/tests/test_reloader.py similarity index 100% rename from tests/test_auto_reload.py rename to tests/test_reloader.py From c975cb1cffd6f5d518374eeb64d4f6efb3a64492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=2E=20K=C3=A4rkk=C3=A4inen?= Date: Wed, 1 Apr 2020 17:22:01 +0300 Subject: [PATCH 22/22] Longer timeout on test exit. --- tests/test_reloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_reloader.py b/tests/test_reloader.py index a140f3ce4f..5c6415b875 100644 --- a/tests/test_reloader.py +++ b/tests/test_reloader.py @@ -85,4 +85,4 @@ async def test_reloader_live(runargs, mode): finally: timeout.cancel() terminate(proc) - proc.wait(timeout=1) + proc.wait(timeout=3)