From e61876ac53f690f5170847de0a3fd54447828835 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Sat, 26 Oct 2024 15:12:49 +0200 Subject: [PATCH] CLI: Load from file or module. Add software tests and documentation. --- .github/workflows/test.yaml | 3 + .gitignore | 1 + README.md | 7 +- docs/source/cli.rst | 90 +++++++++++++++++ docs/source/index.rst | 1 + examples/helloworld.py | 2 + pyproject.toml | 15 +++ responder/__init__.py | 7 ++ responder/cli.py | 70 ++++++++++---- responder/util/__init__.py | 0 responder/util/cmd.py | 139 ++++++++++++++++++++++++++ responder/util/python.py | 98 +++++++++++++++++++ tests/test_cli.py | 188 +++++++++++++++++++++++++++++++++++- tests/util.py | 54 +++++++++++ 14 files changed, 652 insertions(+), 23 deletions(-) create mode 100644 docs/source/cli.rst create mode 100644 responder/util/__init__.py create mode 100644 responder/util/cmd.py create mode 100644 responder/util/python.py create mode 100644 tests/util.py diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index fbe76a9b..65b8f380 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -60,6 +60,9 @@ jobs: - uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} + - uses: actions/setup-node@v4 + with: + node-version: 22 - uses: yezz123/setup-uv@v4 - name: Install package and run software tests (Python 3.6) diff --git a/.gitignore b/.gitignore index 6860b5d1..30637d33 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ .pytest_cache .DS_Store coverage.xml +.coverage* __pycache__ tests/__pycache__ diff --git a/README.md b/README.md index b2a08d2f..291977c2 100644 --- a/README.md +++ b/README.md @@ -44,11 +44,14 @@ Install the most recent stable release: pip install --upgrade 'responder' -Install package including CLI interface and GraphQL extension: +Install package with CLI and GraphQL support: pip install --upgrade 'responder[cli,graphql]' -Or, install directly from the repository: +The CLI provides commands for running and building applications, while GraphQL +adds support for GraphQL query endpoints. + +Alternatively, install directly from the repository: pip install 'responder @ git+https://github.com/kennethreitz/responder.git' diff --git a/docs/source/cli.rst b/docs/source/cli.rst new file mode 100644 index 00000000..14230b9a --- /dev/null +++ b/docs/source/cli.rst @@ -0,0 +1,90 @@ +Responder CLI +============= + +Responder installs a command line program ``responder``. Use it to launch +a Responder application from a file or module. + +Launch application from file +---------------------------- + +Acquire minimal example application, `helloworld.py`_, +implementing a basic echo handler, and launch the HTTP service. + +.. code-block:: shell + + wget https://github.com/kennethreitz/responder/raw/refs/heads/main/examples/helloworld.py + responder run helloworld.py + +In another terminal, invoke a HTTP request, for example using `HTTPie`_. + +.. code-block:: shell + + http http://127.0.0.1:5042/hello + +The response is no surprise. + +:: + + HTTP/1.1 200 OK + content-length: 13 + content-type: text/plain + date: Sat, 26 Oct 2024 13:16:55 GMT + encoding: utf-8 + server: uvicorn + + hello, world! + + +Launch application from module +------------------------------ + +If your Responder application has been implemented as a Python module, +launch it like this: + +.. code-block:: shell + + responder run acme.app + +That assumes a Python package ``acme`` including an ``app`` module +``acme/app.py`` that includes an attribute ``api`` that refers +to a ``responder.API`` instance, reflecting the typical layout of +a standard Responder application. + +.. rubric:: Non-standard instance name + +When your attribute that references the ``responder.API`` instance +is called differently than ``api``, append it to the launch target +address like this: + +.. code-block:: shell + + responder run acme.app:service + +Within your ``app.py``, the instance would have been defined like this: + +.. code-block:: python + + service = responder.API() + + +Build JavaScript application +---------------------------- + +The ``build`` subcommand invokes ``npm run build``, optionally accepting +a target directory. By default, it uses the current working directory, +where it expects a regular NPM ``package.json`` file. + +.. code-block:: shell + + responder build + +When specifying a target directory, responder will change to that +directory beforehand. + +.. code-block:: shell + + responder build /path/to/project + + +.. _helloworld.py: https://github.com/kennethreitz/responder/blob/main/examples/helloworld.py +.. _HTTPie: https://httpie.io/docs/cli diff --git a/docs/source/index.rst b/docs/source/index.rst index 88683e46..dfe95941 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -96,6 +96,7 @@ User Guides deployment testing api + cli Installing Responder diff --git a/examples/helloworld.py b/examples/helloworld.py index 4327e9da..96047d2b 100644 --- a/examples/helloworld.py +++ b/examples/helloworld.py @@ -1,3 +1,5 @@ +# Example HTTP service definition, using Responder. +# https://pypi.org/project/responder/ import responder api = responder.API() diff --git a/pyproject.toml b/pyproject.toml index d36e57c2..bef3f519 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,6 +69,21 @@ markers = [ ] xfail_strict = true +[tool.coverage.run] +branch = false +omit = [ + "*.html", + "tests/*", +] + +[tool.coverage.report] +fail_under = 0 +show_missing = true +exclude_lines = [ + "# pragma: no cover", + "raise NotImplemented", +] + [tool.poe.tasks] check = [ diff --git a/responder/__init__.py b/responder/__init__.py index c573a7ae..dcbaeae9 100644 --- a/responder/__init__.py +++ b/responder/__init__.py @@ -1,3 +1,10 @@ +""" +Responder - a familiar HTTP Service Framework. + +This module exports the core functionality of the Responder framework, +including the API, Request, Response classes and CLI interface. +""" + from . import ext from .core import API, Request, Response diff --git a/responder/cli.py b/responder/cli.py index d9e5e5e1..16a8bd07 100644 --- a/responder/cli.py +++ b/responder/cli.py @@ -1,46 +1,78 @@ -"""Responder. +""" +Responder CLI. + +A web framework for Python. + +Commands: + run Start the application server + build Build frontend assets using npm Usage: responder - responder run [--build] - responder build + responder run [--debug] [--limit-max-requests=] + responder build [] responder --version Options: -h --help Show this screen. -v --version Show version. + --debug Enable debug mode. + --limit-max-requests= Maximum number of requests to handle before shutting down. + +Arguments: + For run: Python module path (e.g., "app:api" loads api from app.py) + For build: Directory containing package.json (default: current directory) +Examples: + responder run app:api # Run the API from app.py + responder build # Build frontend assets """ +import logging +import os import subprocess +import typing as t import docopt from .__version__ import __version__ +from .util.python import load_target +logger = logging.getLogger(__name__) -def cli(): + +def cli() -> None: """ - CLI interface handler of the Responder package. + Main entry point for the Responder CLI. + + Parses command line arguments and executes the appropriate command. + Supports running the application, building assets, and displaying version info. """ args = docopt.docopt(__doc__, argv=None, version=__version__, options_first=False) - module = args[""] - build = args["build"] or args["--build"] - run = args["run"] + target: t.Optional[str] = args[""] + build: bool = args["build"] + debug: bool = args["--debug"] + run: bool = args["run"] if build: - # S603, S607 are suppressed as we're using fixed arguments, not user input - subprocess.check_call(["npm", "run", "build"]) # noqa: S603, S607 + # FIXME: Windows can't find `npm`, at least not on GHA. + # Using `npm.exe` also doesn't work? + subprocess.check_call(["npm", "run", "build"], cwd=target, env=dict(os.environ)) # noqa: S603, S607 if run: - split_module = module.split(":") - - if len(split_module) > 1: - module = split_module[0] - prop = split_module[1] - else: - prop = "api" + # Maximum request limit. Terminating afterward. Suitable for software testing. + limit_max_requests = args["--limit-max-requests"] + if limit_max_requests is not None: + try: + limit_max_requests = int(limit_max_requests) + if limit_max_requests <= 0: + logger.error("limit-max-requests must be a positive integer") + exit(1) + except ValueError: + logger.error("limit-max-requests must be a valid integer") + exit(1) - app = __import__(module) - getattr(app, prop).run() + # Launch Responder API server (uvicorn). + api = load_target(target=target) + api.run(debug=debug, limit_max_requests=limit_max_requests) diff --git a/responder/util/__init__.py b/responder/util/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/responder/util/cmd.py b/responder/util/cmd.py new file mode 100644 index 00000000..35c615c9 --- /dev/null +++ b/responder/util/cmd.py @@ -0,0 +1,139 @@ +# ruff: noqa: S603, S607 +# Security considerations for subprocess usage: +# 1. Only execute the 'responder' binary from PATH +# 2. Validate all user inputs before passing to subprocess +# 3. Use Path.resolve() to prevent path traversal +import logging +import os +import shutil +import signal +import subprocess +import sys +import threading +from pathlib import Path + +logger = logging.getLogger(__name__) + + +class ResponderProgram: + """ + Provide full path to the `responder` program. + """ + + @staticmethod + def path(): + name = "responder" + if sys.platform == "win32": + name = "responder.exe" + program = shutil.which(name) + if program is None: + raise RuntimeError(f"Could not find '{name}' executable in PATH") + logger.info(f"Found responder program: {program}") + return program + + @classmethod + def build(cls, path: Path): + """ + Invoke `responder build`, and return stdout and stderr. + """ + command = [ + cls.path(), + "build", + str(path), + ] + return subprocess.call(command) + + +class ResponderServer(threading.Thread): + """ + A threaded wrapper around the `responder run` command for testing purposes. + + This class allows running a Responder application in a separate thread, + making it suitable for integration testing scenarios. + + Args: + target (str): The path to the Responder application to run + port (int, optional): The port to run the server on. Defaults to 5042. + limit_max_requests (int, optional): Maximum number of requests to handle + before shutting down. Useful for testing scenarios. + + Example: + >>> server = ResponderServer("app.py", port=8000) + >>> server.start() + >>> # Run tests + >>> server.stop() + """ + + def __init__(self, target: str, port: int = 5042, limit_max_requests: int = None): + super().__init__() + + # Sanity checks, as suggested by @coderabbitai. Thanks. + + # Validate path, also prevent path traversal. + if not target or not isinstance(target, str): + raise ValueError("Target must be a non-empty string") + target_path = Path(target).resolve() + if not target_path.exists(): + raise ValueError(f"Target path does not exist: {target}") + if ".." in target_path.parts: + raise ValueError("Path traversal detected in target") + + # Validate port and other options. + if not isinstance(port, int) or port < 1: + raise ValueError("Port must be a positive integer") + if limit_max_requests is not None and ( + not isinstance(limit_max_requests, int) or limit_max_requests < 1 + ): + raise ValueError("limit_max_requests must be a positive integer if specified") + + self.target = target + self.port = port + self.limit_max_requests = limit_max_requests + + # Allow the thread to be terminated when the main program exits. + self.process = None + self.daemon = True + + # Setup signal handlers + signal.signal(signal.SIGTERM, self._signal_handler) + signal.signal(signal.SIGINT, self._signal_handler) + + def run(self): + command = [ + ResponderProgram.path(), + "run", + self.target, + ] + if self.limit_max_requests is not None: + command += [f"--limit-max-requests={self.limit_max_requests}"] + + # Preserve existing environment + env = os.environ.copy() + + if self.port is not None: + env["PORT"] = str(self.port) + + self.process = subprocess.Popen( + command, + env=env, + universal_newlines=True, + ) + self.process.wait() + + def stop(self): + """ + Gracefully stop the process. + """ + if self.process and self.process.poll() is None: + self.process.terminate() + try: + self.process.wait(timeout=5) # Wait up to 5 seconds for graceful shutdown + except subprocess.TimeoutExpired: + self.process.kill() # Force kill if not terminated + + def _signal_handler(self, signum, frame): + """ + Handle termination signals gracefully. + """ + logger.info("Received signal %d, shutting down...", signum) + self.stop() diff --git a/responder/util/python.py b/responder/util/python.py new file mode 100644 index 00000000..a503be9e --- /dev/null +++ b/responder/util/python.py @@ -0,0 +1,98 @@ +import importlib +import importlib.util +import sys +import typing as t +from pathlib import Path +from types import ModuleType + + +def load_target(target: str, default_property: str = "api", method: str = "run") -> t.Any: + """ + Load Python code from a file path or module name. + + Args: + target: Module address (e.g., 'acme.app:foo') or file path (e.g., '/path/to/acme/app.py') + default_property: Name of the property to load if not specified in target (default: "api") + method: Name of the method to verify on the loaded property (default: "run") + + Returns: + The loaded property from the module + + Raises: + ValueError: If target format is invalid + ImportError: If module cannot be imported + AttributeError: If property or method is not found + + Example: + >>> api = load_target("myapp.api:server") + >>> api.run() + """ # noqa: E501 + + # Sanity checks, as suggested by @coderabbitai. Thanks. + if not target or (":" in target and len(target.split(":")) != 2): + raise ValueError( + "Invalid target format. " + "Use '', ':', " + "'/path/to/app.py', or '/path/to/app.py:" + ) + + # Decode launch target location address. + # Module: acme.app:foo + # Path: /path/to/acme/app.py:foo + target_fragments = target.split(":") + if len(target_fragments) > 1: + target = target_fragments[0] + prop = target_fragments[1] + else: + prop = default_property + + # Import launch target. Treat input location either as a filesystem path + # (/path/to/acme/app.py), or as a module address specification (acme.app). + path = Path(target) + if path.is_file(): + app = load_file_module(path) + else: + app = importlib.import_module(target) + + # Invoke launch target. + msg_prefix = f"Failed to import target '{target}'" + try: + api = getattr(app, prop, None) + if api is None: + raise AttributeError( + f"{msg_prefix}: Module has no API instance attribute '{prop}'" + ) + if not hasattr(api, method): + raise AttributeError( + f"{msg_prefix}: API instance '{prop}' has no method '{method}'" + ) + return api + except ImportError as ex: + raise ImportError(f"{msg_prefix}: {ex}") from ex + + +def load_file_module(path: Path) -> ModuleType: + """ + Load a Python file as a module using importlib. + + Args: + path: Path to the Python file to load + + Returns: + The loaded module object + + Raises: + ImportError: If the module cannot be loaded + """ + name = f"__{path.stem}__" + spec = importlib.util.spec_from_file_location(name, path) + if spec is None: + raise ImportError(f"Failed loading module from file: {path}") + app = importlib.util.module_from_spec(spec) + sys.modules[name] = app + try: + spec.loader.exec_module(app) + return app + except Exception as ex: + sys.modules.pop(name, None) + raise ImportError(f"Failed to execute module '{app}': {ex}") from ex diff --git a/tests/test_cli.py b/tests/test_cli.py index 5975105a..cdcb79ff 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,18 +1,202 @@ +""" +Test module for Responder CLI functionality. + +This module tests the following CLI commands: +- responder --version: Version display +- responder build: Build command execution +- responder run: Server execution + +Requirements: +- docopt-ng package must be installed +- Example application at examples/helloworld.py +""" + +import json +import os import subprocess +import sys +import time +import typing as t +from pathlib import Path import pytest +import requests +from _pytest.capture import CaptureFixture +from requests_toolbelt.multipart.encoder import to_list from responder.__version__ import __version__ +from responder.util.cmd import ResponderProgram, ResponderServer +from tests.util import random_port +# Skip test if optional CLI dependency is not installed. pytest.importorskip("docopt", reason="docopt-ng package not installed") +# Maximum time to wait for server startup (adjust for slower systems) +SERVER_STARTUP_TIMEOUT = int(os.getenv("RESPONDER_SERVER_STARTUP_TIMEOUT", "10")) + +# Maximum time to wait for HTTP requests (adjust for slower networks) +REQUEST_TIMEOUT = int(os.getenv("RESPONDER_REQUEST_TIMEOUT", "5")) + +# Endpoint to use for `responder run`. +HELLO_ENDPOINT = "/hello" + + def test_cli_version(capfd): - # S603, S607 are suppressed as we're using fixed arguments, not user input + """ + Verify that `responder --version` works as expected. + """ try: + # Suppress security checks for subprocess calls in tests. + # S603: subprocess call - safe as we use fixed command + # S607: start process with partial path - safe as we use installed package subprocess.check_call(["responder", "--version"]) # noqa: S603, S607 except subprocess.CalledProcessError as ex: - pytest.fail(f"CLI command failed with exit code {ex.returncode}") + pytest.fail( + f"responder --version failed with exit code {ex.returncode}. Error: {ex}" + ) stdout = capfd.readouterr().out.strip() assert stdout == __version__ + + +def responder_build(path: Path, capfd: CaptureFixture) -> t.Tuple[str, str]: + """ + Execute responder build command and capture its output. + + Args: + path: Directory containing package.json + capfd: Pytest fixture for capturing output + + Returns: + tuple: (stdout, stderr) containing the captured output + """ + + if sys.platform == "win32": + raise pytest.skip("Testing 'responder build' does not work on Windows") + + if not path.exists(): + raise FileNotFoundError(f"Path does not exist: {path}") + if not path.is_dir(): + raise FileNotFoundError(f"Path is not a directory: {path}") + + ResponderProgram.build(path=path) + output = capfd.readouterr() + + stdout = output.out.strip() + stderr = output.err.strip() + + return stdout, stderr + + +def test_cli_build_success(capfd, tmp_path): + """ + Verify that `responder build` works as expected. + """ + + # Temporary surrogate `package.json` file. + package_json = {"scripts": {"build": "echo Hotzenplotz"}} + package_json_file = tmp_path / "package.json" + package_json_file.write_text(json.dumps(package_json)) + + # Invoke `responder build`. + stdout, stderr = responder_build(tmp_path, capfd) + assert "Hotzenplotz" in stdout + + +def test_cli_build_missing_package_json(capfd, tmp_path): + """ + Verify `responder build`, while `package.json` file is missing. + """ + + # Invoke `responder build`. + stdout, stderr = responder_build(tmp_path, capfd) + assert "npm error code ENOENT" in stderr + assert "Could not read package.json" in stderr + assert "Error: ENOENT: no such file or directory" in stderr + + +@pytest.mark.parametrize( + "invalid_content,npm_error,expected_error", + [ + ("foobar", "code EJSONPARSE", ["is not valid JSON", "Failed to parse JSON data"]), + ("{", "code EJSONPARSE", "Unexpected end of JSON input"), + ('{"scripts": }', "code EJSONPARSE", "Unexpected token"), + ( + '{"scripts": null}', + "Cannot convert undefined or null to object", + "scripts.build script not found", + ), + ('{"scripts": {"build": null}}', "Missing script", '"build"'), + ('{"scripts": {"build": 123}}', "Missing script", '"build"'), + ], + ids=[ + "invalid_json_content", + "incomplete_json", + "syntax_error", + "null_scripts", + "missing_script_null", + "missing_script_number", + ], +) +def test_cli_build_invalid_package_json( + capfd, tmp_path, invalid_content, npm_error, expected_error +): + """ + Verify `responder build` using an invalid `package.json` file. + """ + + # Temporary surrogate `package.json` file. + package_json_file = tmp_path / "package.json" + package_json_file.write_text(invalid_content) + + # Invoke `responder build`. + stdout, stderr = responder_build(tmp_path, capfd) + assert f"npm error {npm_error}" in stderr + assert any(item in stderr for item in to_list(expected_error)) + + +def test_cli_run(capfd): + """ + Verify that `responder run` works as expected. + """ + + # Invoke `responder run`. + # Start a Responder service instance in the background, using its CLI. + # Make it terminate itself after serving one HTTP request. + target = Path("examples") / "helloworld.py" + if not target.is_file(): + raise FileNotFoundError(f"Example file not found: {target}") + + server = ResponderServer(target=str(target), port=random_port(), limit_max_requests=1) + try: + server.start() + # Can't use `wait_server` here, because probing + # requests would terminate the server. + # wait_server(server.port) + # Need to fall back to naive sleeping for now. + time.sleep(0.75) + response = requests.get( + f"http://127.0.0.1:{server.port}{HELLO_ENDPOINT}", timeout=REQUEST_TIMEOUT + ) + assert "hello, world!" == response.text + finally: + server.join() + + # Capture process output. + output = capfd.readouterr() + + stdout = output.out.strip() + assert f'"GET {HELLO_ENDPOINT} HTTP/1.1" 200 OK' in stdout + + stderr = output.err.strip() + + assert "Started server process" in stderr + assert "Waiting for application startup" in stderr + assert "Application startup complete" in stderr + assert "Uvicorn running" in stderr + + assert "Shutting down" in stderr + assert "Waiting for application shutdown" in stderr + assert "Application shutdown complete" in stderr + assert "Finished server process" in stderr diff --git a/tests/util.py b/tests/util.py new file mode 100644 index 00000000..995df520 --- /dev/null +++ b/tests/util.py @@ -0,0 +1,54 @@ +import socket +import time + +import requests + + +def random_port() -> int: + """ + Return a random available port by binding to port 0. + + Returns: + int: An available port number that can be used for testing. + """ + sock = socket.socket() + try: + sock.bind(("", 0)) + return sock.getsockname()[1] + finally: + sock.close() + + +def wait_server( + port: int, + host: str = "127.0.0.1", + protocol: str = "http", + attempts: int = 20, + delay: float = 0.1, +): + """ + Wait for server to be ready by attempting to connect to it. + + Args: + port: The port number to connect to + host: The host to connect to (default: "127.0.0.1") + protocol: The protocol to use (default: "http") + attempts: Number of connection attempts (default: 20) + delay: Delay per attempt in seconds (default: 0.1) + + Raises: + RuntimeError: If server is not ready after all attempts + """ + url = f"{protocol}://{host}:{port}/" + for attempt in range(1, attempts + 1): + try: + requests.get(url, timeout=delay / 2) # Shorter timeout for connection + break + except requests.exceptions.RequestException: + if attempt < attempts: # Don't sleep on last attempt + time.sleep(delay) + else: + raise RuntimeError( + f"Server at {url} failed to respond after {attempts} attempts " + f"(total wait time: {attempts * delay:.1f}s)" + )