diff --git a/README.md b/README.md index 04d5e1246..51180e581 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,8 @@ openenv validate my_env --level static --skip-build # quick check against the openenv push my_env # deploy it to Hugging Face Spaces ``` +For local development, run `openenv serve my_env` after installing its dependencies. This runs on the host and binds to `127.0.0.1`; use Docker or Spaces for isolated training and deployment. + See [Your First Environment](https://huggingface.co/docs/openenv/guides/first-environment) and [Deploying an Environment](https://huggingface.co/docs/openenv/getting_started/environment-builder). `openenv import` wraps an existing environment from ORS/OpenReward or Verifiers. ## Environments diff --git a/src/openenv/cli/__main__.py b/src/openenv/cli/__main__.py index bdaafb09c..b4f32159e 100644 --- a/src/openenv/cli/__main__.py +++ b/src/openenv/cli/__main__.py @@ -51,9 +51,10 @@ name="push", help="Push an OpenEnv environment to Hugging Face Spaces or custom registry", )(push.push) -app.command(name="serve", help="Serve environments locally (TODO: Phase 4)")( - serve.serve -) +app.command( + name="serve", + help="Run an environment on the host for local development (no container isolation)", +)(serve.serve) app.command( name="fork", help="Fork (duplicate) a Hugging Face Space to your account", diff --git a/src/openenv/cli/commands/serve.py b/src/openenv/cli/commands/serve.py index fa847f8d0..c2d0e3243 100644 --- a/src/openenv/cli/commands/serve.py +++ b/src/openenv/cli/commands/serve.py @@ -1,20 +1,20 @@ # SPDX-License-Identifier: BSD-3-Clause -"""Serve OpenEnv environments locally (TO BE IMPLEMENTED).""" +"""Serve an OpenEnv environment locally (uvicorn, from ``openenv.yaml``).""" from __future__ import annotations +import os +import sys from pathlib import Path from typing import Annotated import typer +import yaml -from .._cli_utils import console +from .._cli_utils import console, validate_env_structure -app = typer.Typer(help="Serve OpenEnv environments locally") - -@app.command() def serve( env_path: Annotated[ str | None, @@ -23,68 +23,133 @@ def serve( ), ] = None, port: Annotated[ - int, - typer.Option("--port", "-p", help="Port to serve on"), - ] = 8000, + int | None, + typer.Option( + "--port", + "-p", + help="Port to bind (default: ``port`` in openenv.yaml, else 8000)", + ), + ] = None, host: Annotated[ str, - typer.Option("--host", help="Host to bind to"), - ] = "0.0.0.0", + typer.Option("--host", help="Host interface to bind (local development only)"), + ] = "127.0.0.1", reload: Annotated[ bool, - typer.Option("--reload", help="Enable auto-reload on code changes"), + typer.Option("--reload", help="Enable autoreload (development)"), ] = False, ) -> None: """ - Serve an OpenEnv environment locally. - - TODO: This command is currently not implemented and has been deferred for later. + Run the environment FastAPI app on the host for local development. - Planned functionality: - - Run environment server locally without Docker - - Support multiple deployment modes (local, notebook, cluster) - - Auto-reload for development - - Integration with environment's [project.scripts] entry point + Uses ``openenv.yaml`` fields ``app`` (e.g. ``server.app:app``), ``port``, and + ``runtime`` (must be ``fastapi``). Matches ``uv run --project . server`` layout: + the environment directory is the working directory. Package-qualified app + paths also support the environment package and the repo's ``envs`` package. - For now, use Docker-based serving: - 1. Build the environment: openenv build - 2. Run the container: docker run -p 8000:8000 - - Or use uv directly: - uv run --project . server --port 8000 + For production or training, use Docker (``openenv build``) — this command runs + on the host for local development only. """ - console.print("[bold yellow]⚠ This command is not yet implemented[/bold yellow]\n") - - console.print( - "The [bold cyan]openenv serve[/bold cyan] command has been deferred for later." - ) - - console.print("[bold]Alternative approaches:[/bold]\n") + import uvicorn - console.print("[cyan]Option 1: Docker-based serving (recommended)[/cyan]") - console.print(" 1. Build the environment:") - console.print(" [dim]$ openenv build[/dim]") - console.print(" 2. Run the Docker container:") - console.print( - f" [dim]$ docker run -p {port}:{port} openenv-:latest[/dim]\n" + env_path_obj = ( + Path.cwd().resolve() if env_path is None else Path(env_path).resolve() ) - console.print("[cyan]Option 2: Direct execution with uv[/cyan]") - - # Determine environment path - if env_path is None: - env_path_obj = Path.cwd() - else: - env_path_obj = Path(env_path) - - # Check for openenv.yaml - openenv_yaml = env_path_obj / "openenv.yaml" - if openenv_yaml.exists(): - console.print(" From your environment directory:") - console.print(f" [dim]$ cd {env_path_obj}[/dim]") - console.print(f" [dim]$ uv run --project . server --port {port}[/dim]\n") - else: - console.print(" From an environment directory with pyproject.toml:") - console.print(f" [dim]$ uv run --project . server --port {port}[/dim]\n") - - raise typer.Exit(1) + if not env_path_obj.exists(): + typer.echo(f"Error: Path does not exist: {env_path_obj}", err=True) + raise typer.Exit(1) + + if not env_path_obj.is_dir(): + typer.echo(f"Error: Path is not a directory: {env_path_obj}", err=True) + raise typer.Exit(1) + + try: + validate_env_structure(env_path_obj) + except FileNotFoundError as exc: + raise typer.BadParameter(f"Not a valid OpenEnv environment: {exc}") from exc + + manifest_path = env_path_obj / "openenv.yaml" + try: + with manifest_path.open("r", encoding="utf-8") as handle: + manifest = yaml.safe_load(handle) + except OSError as exc: + raise typer.BadParameter(f"Failed to read openenv.yaml: {exc}") from exc + except yaml.YAMLError as exc: + raise typer.BadParameter(f"Invalid YAML in openenv.yaml: {exc}") from exc + + if not isinstance(manifest, dict): + raise typer.BadParameter("openenv.yaml must be a YAML dictionary") + + app_spec = manifest.get("app") + if not app_spec or not isinstance(app_spec, str): + raise typer.BadParameter( + "openenv.yaml must contain a string 'app' field (e.g. server.app:app)" + ) + if ":" not in app_spec: + raise typer.BadParameter( + f"openenv.yaml 'app' must look like 'module.path:attribute', got {app_spec!r}" + ) + + runtime = str(manifest.get("runtime", "fastapi")).lower() + if runtime != "fastapi": + raise typer.BadParameter( + f"openenv serve only supports runtime 'fastapi' (got {runtime!r})" + ) + + raw_port = port if port is not None else manifest.get("port", 8000) + try: + listen_port = int(raw_port) + except (TypeError, ValueError) as exc: + raise typer.BadParameter( + f"Invalid port {raw_port!r}; expected an integer" + ) from exc + if not (1 <= listen_port <= 65535): + raise typer.BadParameter( + f"Invalid port {listen_port}; expected a value between 1 and 65535" + ) + + original_path = list(sys.path) + prev_cwd = os.getcwd() + env_root = str(env_path_obj) + app_dir = env_path_obj + module_name = app_spec.split(":", 1)[0] + # Prefer a module inside the environment, even when its package name matches + # the environment directory (for example, /work/server/server/app.py). + local_module = env_path_obj.joinpath(*module_name.split(".")) + if not ( + local_module.with_suffix(".py").is_file() + or (local_module / "__init__.py").is_file() + ): + if env_path_obj.parent.name == "envs" and module_name.startswith( + f"envs.{env_path_obj.name}." + ): + app_dir = env_path_obj.parent.parent + elif module_name.startswith(f"{env_path_obj.name}."): + app_dir = env_path_obj.parent + + try: + os.chdir(env_root) + + console.print( + f"[bold green]Serving[/bold green] [cyan]{app_spec}[/cyan] on " + f"[bold]http://{host}:{listen_port}/[/bold] (cwd: {env_root})" + ) + + try: + uvicorn.run( + app_spec, + host=host, + port=listen_port, + reload=reload, + app_dir=str(app_dir), + ) + except OSError as exc: + typer.echo(f"Error: {exc}", err=True) + raise typer.Exit(1) from exc + finally: + try: + os.chdir(prev_cwd) + except OSError: + pass + sys.path[:] = original_path diff --git a/tests/test_cli/test_serve.py b/tests/test_cli/test_serve.py new file mode 100644 index 000000000..845b4fbdc --- /dev/null +++ b/tests/test_cli/test_serve.py @@ -0,0 +1,253 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. + +"""Tests for ``openenv serve``.""" + +from __future__ import annotations + +import os +import socket +import subprocess +import sys +import tempfile +import time +from pathlib import Path +from unittest.mock import patch + +import pytest +from openenv.cli.__main__ import app +from typer.testing import CliRunner + + +REPO_ROOT = Path(__file__).resolve().parents[2] +ECHO_ENV = REPO_ROOT / "envs" / "echo_env" +runner = CliRunner() + + +def _pick_free_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return int(s.getsockname()[1]) + + +def test_serve_calls_uvicorn_with_echo_manifest() -> None: + with patch("uvicorn.run") as mock_run: + result = runner.invoke( + app, + [ + "serve", + str(ECHO_ENV), + "--port", + "9911", + "--host", + "127.0.0.1", + ], + ) + assert result.exit_code == 0, result.stdout + mock_run.assert_called_once() + (app_arg,), kwargs = mock_run.call_args + assert app_arg == "server.app:app" + assert kwargs["host"] == "127.0.0.1" + assert kwargs["port"] == 9911 + assert kwargs["reload"] is False + + +def test_serve_rejects_invalid_env_dir() -> None: + result = runner.invoke( + app, + ["serve", str(REPO_ROOT / "nonexistent_env_dir_xyz")], + ) + assert result.exit_code != 0 + + +def test_serve_uses_manifest_port_when_omitted() -> None: + with patch("uvicorn.run") as mock_run: + result = runner.invoke( + app, + ["serve", str(ECHO_ENV), "--host", "127.0.0.1"], + ) + assert result.exit_code == 0, result.stdout + _, kwargs = mock_run.call_args + assert kwargs["port"] == 8000 + + +@pytest.mark.integration +@pytest.mark.parametrize("reload", [False, True]) +def test_serve_echo_env_health_subprocess(reload) -> None: + _serve_health_subprocess(ECHO_ENV, reload) + + +@pytest.mark.integration +@pytest.mark.parametrize("reload", [False, True]) +@pytest.mark.parametrize( + ("env_name", "app_spec"), + [ + ("sample_env", "server.app:app"), + ("sample_env", "sample_env.server.app:app"), + ("sample_env", "envs.sample_env.server.app:app"), + ("server", "server.app:app"), + ("server", "server.server.app:app"), + ("server", "envs.server.server.app:app"), + ("envs", "envs.server.app:app"), + ("envs", "envs.envs.server.app:app"), + ], +) +def test_serve_manifest_import_layouts(tmp_path, env_name, app_spec, reload): + env_dir = tmp_path / "envs" / env_name + (env_dir / "server").mkdir(parents=True) + for name in ( + "__init__.py", + "client.py", + "README.md", + "pyproject.toml", + "server/__init__.py", + "server/Dockerfile", + ): + (env_dir / name).touch() + (env_dir / "openenv.yaml").write_text(f"app: {app_spec}\nruntime: fastapi\n") + (env_dir / "server/app.py").write_text( + "from fastapi import FastAPI\n" + "from pathlib import Path\n" + "app = FastAPI()\n" + "@app.get('/health')\n" + "def health(): return {'cwd': str(Path.cwd())}\n" + ) + assert _serve_health_subprocess(env_dir, reload) == {"cwd": str(env_dir)} + + +def _serve_health_subprocess(env_dir, reload): + requests = pytest.importorskip("requests") + port = _pick_free_port() + env = os.environ.copy() + env["PYTHONPATH"] = str(REPO_ROOT / "src") + cmd = [ + sys.executable, + "-m", + "openenv.cli", + "serve", + str(env_dir), + "--port", + str(port), + "--host", + "127.0.0.1", + ] + if reload: + cmd.append("--reload") + output = tempfile.TemporaryFile(mode="w+", encoding="utf-8") + proc = subprocess.Popen( + cmd, + cwd=str(REPO_ROOT), + env=env, + stdout=output, + stderr=subprocess.STDOUT, + text=True, + encoding="utf-8", + errors="replace", + ) + try: + deadline = time.time() + 60.0 + last_exc: Exception | None = None + ok = False + while time.time() < deadline: + try: + r = requests.get(f"http://127.0.0.1:{port}/health", timeout=2.0) + if r.status_code == 200: + ok = True + response = r.json() + break + except Exception as exc: + last_exc = exc + if proc.poll() is not None: + output.seek(0) + out = output.read() + pytest.fail( + f"serve process exited early (code={proc.returncode}): {out}" + ) + time.sleep(0.4) + if not ok: + output.seek(0) + pytest.fail(f"/health never OK (last error={last_exc!r}): {output.read()}") + finally: + if proc.poll() is None: + proc.terminate() + try: + proc.wait(timeout=15) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait(timeout=5) + output.close() + return response + + +def test_serve_does_not_add_unrelated_ancestor_source_paths(tmp_path, monkeypatch): + (tmp_path / "src/openenv").mkdir(parents=True) + env_dir = tmp_path / "work/sample_env" + env_dir.mkdir(parents=True) + for name in ( + "__init__.py", + "client.py", + "README.md", + "pyproject.toml", + "Dockerfile", + ): + (env_dir / name).touch() + (env_dir / "openenv.yaml").write_text("app: server.app:app\n") + monkeypatch.setenv("PYTHONPATH", "existing-python-path") + + def inspect_paths(*args, **kwargs): + assert str(tmp_path) not in sys.path + assert str(tmp_path / "src") not in sys.path + assert os.environ["PYTHONPATH"] == "existing-python-path" + + with patch("uvicorn.run", side_effect=inspect_paths): + result = runner.invoke(app, ["serve", str(env_dir)]) + assert result.exit_code == 0, result.exception + + +def test_serve_defaults_to_loopback_and_restores_process_state(monkeypatch): + before = os.getcwd(), list(sys.path) + monkeypatch.setenv("PYTHONPATH", "existing-python-path") + with patch("uvicorn.run", autospec=True) as run: + result = runner.invoke(app, ["serve", str(ECHO_ENV)]) + assert result.exit_code == 0, result.stdout + assert run.call_args.kwargs["host"] == "127.0.0.1" + assert (os.getcwd(), sys.path) == before + assert os.environ["PYTHONPATH"] == "existing-python-path" + + +def test_serve_restores_process_state_after_uvicorn_error(monkeypatch): + before = os.getcwd(), list(sys.path) + monkeypatch.delenv("PYTHONPATH", raising=False) + with patch("uvicorn.run", autospec=True, side_effect=OSError("cannot bind")): + result = runner.invoke(app, ["serve", str(ECHO_ENV)]) + assert result.exit_code == 1 + assert "cannot bind" in result.output + assert (os.getcwd(), sys.path) == before + assert "PYTHONPATH" not in os.environ + + +def test_serve_module_import_does_not_load_uvicorn(): + code = """ +import importlib.util +import sys +from pathlib import Path +sys.modules['uvicorn'] = None +spec = importlib.util.spec_from_file_location( + 'openenv.cli.commands.serve', Path('src/openenv/cli/commands/serve.py') +) +module = importlib.util.module_from_spec(spec) +spec.loader.exec_module(module) +assert callable(module.serve) +""" + result = subprocess.run( + [sys.executable, "-c", code], + cwd=REPO_ROOT, + env={**os.environ, "PYTHONPATH": str(REPO_ROOT / "src")}, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stderr