From 8a0dcc6310cc11096fcbb2dbe3182800a6f67ac6 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Tue, 22 Feb 2022 17:28:52 +0200 Subject: [PATCH 1/9] change to suggest --simple and --factory in cli --- sanic/cli/app.py | 18 ++++++++++++++++-- tests/fake/factory.py | 6 ++++++ tests/test_cli.py | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 tests/fake/factory.py diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 95a572a8bd..f282829323 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -76,7 +76,12 @@ def run(self): kwargs = self._build_run_kwargs() app.run(**kwargs) except ValueError: - error_logger.exception("Failed to run app") + if os.path.isdir(self.args.module): + error_logger.exception( + "Please attach --simple to run your app from a directory." + ) + else: + error_logger.exception("Failed to run app") def _precheck(self): # # Custom TLS mismatch handling for better diagnostics @@ -125,9 +130,18 @@ def _get_app(self): app_type_name = type(app).__name__ if not isinstance(app, Sanic): + if hasattr(app, "__call__"): + solution = f"sanic {self.args.module} --factory" + raise ValueError( + "Module is not a Sanic app, it is a" + f"{app_type_name}\n" + " If your function returns a" + f"Sanic instance try: {solution}" + ) + raise ValueError( f"Module is not a Sanic app, it is a {app_type_name}\n" - f" Perhaps you meant {self.args.module}.app?" + f" Perhaps you meant {self.args.module}:app?" ) except ImportError as e: if module_name.startswith(e.name): diff --git a/tests/fake/factory.py b/tests/fake/factory.py new file mode 100644 index 0000000000..17a815cd97 --- /dev/null +++ b/tests/fake/factory.py @@ -0,0 +1,6 @@ +from sanic import Sanic + + +def run(): + app = Sanic("FactoryTest") + return app diff --git a/tests/test_cli.py b/tests/test_cli.py index 7ec1c28ce7..954cd48562 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -57,6 +57,20 @@ def test_server_run(appname): assert firstline == b"Goin' Fast @ http://127.0.0.1:8000" +def test_error_with_function_as_instance_with_no_factory(): + command = ["sanic", "fake.factory.run"] + out, err, exitcode = capture(command) + assert b"try: sanic fake.factory.run --factory" in err + assert exitcode != 1 + + +def test_error_with_path_as_instance_without_simple(): + command = ["sanic", "./fake/"] + out, err, exitcode = capture(command) + assert b"Please attach --simple to run your app from a directory." in err + assert exitcode != 1 + + @pytest.mark.parametrize( "cmd", ( @@ -250,6 +264,8 @@ def test_access_logs(cmd, expected): lines = out.split(b"\n") info = read_app_info(lines) + print(lines) + assert ( info["access_log"] is expected ), f"Lines found: {lines}\nErr output: {err}" From e11efc6cd3fe884452a98d7a89e9c0012065c881 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Tue, 22 Feb 2022 17:30:56 +0200 Subject: [PATCH 2/9] cleanup print statement --- tests/test_cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 954cd48562..0e16014760 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -264,8 +264,6 @@ def test_access_logs(cmd, expected): lines = out.split(b"\n") info = read_app_info(lines) - print(lines) - assert ( info["access_log"] is expected ), f"Lines found: {lines}\nErr output: {err}" From c9fc513f9a0ae3fe87c5f5d5f735ed0e2aa40728 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Tue, 22 Feb 2022 17:41:48 +0200 Subject: [PATCH 3/9] better test function names --- tests/test_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 0e16014760..19d493a89d 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -57,14 +57,14 @@ def test_server_run(appname): assert firstline == b"Goin' Fast @ http://127.0.0.1:8000" -def test_error_with_function_as_instance_with_no_factory(): +def test_error_with_function_as_instance_without_factory_arg(): command = ["sanic", "fake.factory.run"] out, err, exitcode = capture(command) assert b"try: sanic fake.factory.run --factory" in err assert exitcode != 1 -def test_error_with_path_as_instance_without_simple(): +def test_error_with_path_as_instance_without_simple_arg(): command = ["sanic", "./fake/"] out, err, exitcode = capture(command) assert b"Please attach --simple to run your app from a directory." in err From 822035027afb915a2fe11c9d52e086147f3e72a9 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Wed, 23 Feb 2022 10:17:37 +0200 Subject: [PATCH 4/9] moved the path check to _get_app --- sanic/cli/app.py | 13 +++++++------ tests/test_cli.py | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index f282829323..9976b3dad6 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -76,12 +76,7 @@ def run(self): kwargs = self._build_run_kwargs() app.run(**kwargs) except ValueError: - if os.path.isdir(self.args.module): - error_logger.exception( - "Please attach --simple to run your app from a directory." - ) - else: - error_logger.exception("Failed to run app") + error_logger.exception("Failed to run app") def _precheck(self): # # Custom TLS mismatch handling for better diagnostics @@ -118,6 +113,12 @@ def _get_app(self): delimiter = ":" if ":" in self.args.module else "." module_name, app_name = self.args.module.rsplit(delimiter, 1) + if module_name == "" and os.path.isdir(self.args.module): + raise ValueError("App not found." + " Please use --simple if you are passing a directory to sanic." + f" eg. sanic {self.args.module} --simple" + ) + if app_name.endswith("()"): self.args.factory = True app_name = app_name[:-2] diff --git a/tests/test_cli.py b/tests/test_cli.py index 19d493a89d..e16b10e0d5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -67,7 +67,7 @@ def test_error_with_function_as_instance_without_factory_arg(): def test_error_with_path_as_instance_without_simple_arg(): command = ["sanic", "./fake/"] out, err, exitcode = capture(command) - assert b"Please attach --simple to run your app from a directory." in err + assert b"Please use --simple if you are passing a directory to sanic." in err assert exitcode != 1 From cab3738ec9780d953e05612f13c10a75524b2991 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Wed, 23 Feb 2022 10:20:23 +0200 Subject: [PATCH 5/9] changes hasattr to callable --- sanic/cli/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 9976b3dad6..3a68b9f5aa 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -131,7 +131,7 @@ def _get_app(self): app_type_name = type(app).__name__ if not isinstance(app, Sanic): - if hasattr(app, "__call__"): + if callable(app): solution = f"sanic {self.args.module} --factory" raise ValueError( "Module is not a Sanic app, it is a" From 55094859397c173e4fb43141da7ecd5fa8c5e5fb Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Wed, 23 Feb 2022 10:22:37 +0200 Subject: [PATCH 6/9] changes function working to callable --- sanic/cli/app.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 3a68b9f5aa..152829260a 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -114,8 +114,8 @@ def _get_app(self): module_name, app_name = self.args.module.rsplit(delimiter, 1) if module_name == "" and os.path.isdir(self.args.module): - raise ValueError("App not found." - " Please use --simple if you are passing a directory to sanic." + raise ValueError("App not found.\n" + " Please use --simple if you are passing a directory to sanic.\n" f" eg. sanic {self.args.module} --simple" ) @@ -136,8 +136,8 @@ def _get_app(self): raise ValueError( "Module is not a Sanic app, it is a" f"{app_type_name}\n" - " If your function returns a" - f"Sanic instance try: {solution}" + " If this callable returns a" + f"Sanic instance try: \n{solution}" ) raise ValueError( From 87f6d708d99895e2720540263b7314ba53ef6b76 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Wed, 23 Feb 2022 10:26:56 +0200 Subject: [PATCH 7/9] make pretty --- sanic/cli/app.py | 7 ++++--- tests/test_cli.py | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 152829260a..05baf66c9a 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -114,9 +114,10 @@ def _get_app(self): module_name, app_name = self.args.module.rsplit(delimiter, 1) if module_name == "" and os.path.isdir(self.args.module): - raise ValueError("App not found.\n" - " Please use --simple if you are passing a directory to sanic.\n" - f" eg. sanic {self.args.module} --simple" + raise ValueError( + "App not found.\n" + " Please use --simple if you are passing a directory to sanic.\n" + f" eg. sanic {self.args.module} --simple" ) if app_name.endswith("()"): diff --git a/tests/test_cli.py b/tests/test_cli.py index e16b10e0d5..c1c7976a72 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -67,7 +67,9 @@ def test_error_with_function_as_instance_without_factory_arg(): def test_error_with_path_as_instance_without_simple_arg(): command = ["sanic", "./fake/"] out, err, exitcode = capture(command) - assert b"Please use --simple if you are passing a directory to sanic." in err + assert ( + b"Please use --simple if you are passing a directory to sanic." in err + ) assert exitcode != 1 From 18e164798cf52ad5d0b8e1346fc9e7b61945d2c0 Mon Sep 17 00:00:00 2001 From: Callum Fleming Date: Wed, 23 Feb 2022 10:30:46 +0200 Subject: [PATCH 8/9] fixed tests --- tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index c1c7976a72..b12bb41454 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -60,7 +60,7 @@ def test_server_run(appname): def test_error_with_function_as_instance_without_factory_arg(): command = ["sanic", "fake.factory.run"] out, err, exitcode = capture(command) - assert b"try: sanic fake.factory.run --factory" in err + assert b"try: \nsanic fake.factory.run --factory" in err assert exitcode != 1 From 07139728e30243752e88002d0ee510bc07b05fe9 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Tue, 22 Mar 2022 23:20:04 +0200 Subject: [PATCH 9/9] Fix linting --- sanic/cli/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sanic/cli/app.py b/sanic/cli/app.py index 05baf66c9a..db830e09a5 100644 --- a/sanic/cli/app.py +++ b/sanic/cli/app.py @@ -116,7 +116,8 @@ def _get_app(self): if module_name == "" and os.path.isdir(self.args.module): raise ValueError( "App not found.\n" - " Please use --simple if you are passing a directory to sanic.\n" + " Please use --simple if you are passing a " + "directory to sanic.\n" f" eg. sanic {self.args.module} --simple" )