From e20b1f62e11a2583ebbfb327836f37af1349d63b Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sat, 1 Nov 2025 14:13:15 +0100 Subject: [PATCH 01/38] commit copy env --- src/envs/copy_env/README.md | 146 +++++++++++++++++++ src/envs/copy_env/__init__.py | 12 ++ src/envs/copy_env/client.py | 101 +++++++++++++ src/envs/copy_env/models.py | 30 ++++ src/envs/copy_env/server/Dockerfile | 25 ++++ src/envs/copy_env/server/__init__.py | 11 ++ src/envs/copy_env/server/app.py | 39 +++++ src/envs/copy_env/server/echo_environment.py | 95 ++++++++++++ 8 files changed, 459 insertions(+) create mode 100644 src/envs/copy_env/README.md create mode 100644 src/envs/copy_env/__init__.py create mode 100644 src/envs/copy_env/client.py create mode 100644 src/envs/copy_env/models.py create mode 100644 src/envs/copy_env/server/Dockerfile create mode 100644 src/envs/copy_env/server/__init__.py create mode 100644 src/envs/copy_env/server/app.py create mode 100644 src/envs/copy_env/server/echo_environment.py diff --git a/src/envs/copy_env/README.md b/src/envs/copy_env/README.md new file mode 100644 index 00000000..c4b7af37 --- /dev/null +++ b/src/envs/copy_env/README.md @@ -0,0 +1,146 @@ +--- +title: Echo Environment Server +emoji: 🔊 +colorFrom: '#00C9FF' +colorTo: '#1B2845' +sdk: docker +pinned: false +app_port: 8000 +base_path: /web +tags: + - openenv +--- + +# Echo Environment + +A simple test environment that echoes back messages. Perfect for testing the env APIs as well as demonstrating environment usage patterns. + +## Quick Start + +The simplest way to use the Echo environment is through the `EchoEnv` class: + +```python +from envs.echo_env import EchoAction, EchoEnv + +try: + # Create environment from Docker image + echo_env = EchoEnv.from_docker_image("echo-env:latest") + + # Reset + result = echo_env.reset() + print(f"Reset: {result.observation.echoed_message}") + + # Send multiple messages + messages = ["Hello, World!", "Testing echo", "Final message"] + + for msg in messages: + result = echo_env.step(EchoAction(message=msg)) + print(f"Sent: '{msg}'") + print(f" → Echoed: '{result.observation.echoed_message}'") + print(f" → Length: {result.observation.message_length}") + print(f" → Reward: {result.reward}") + +finally: + # Always clean up + echo_env.close() +``` + +That's it! The `EchoEnv.from_docker_image()` method handles: +- Starting the Docker container +- Waiting for the server to be ready +- Connecting to the environment +- Container cleanup when you call `close()` + +## Building the Docker Image + +Before using the environment, you need to build the Docker image: + +```bash +# From project root +docker build -t echo-env:latest -f src/envs/echo_env/server/Dockerfile . +``` + +## Environment Details + +### Action +**EchoAction**: Contains a single field +- `message` (str) - The message to echo back + +### Observation +**EchoObservation**: Contains the echo response and metadata +- `echoed_message` (str) - The message echoed back +- `message_length` (int) - Length of the message +- `reward` (float) - Reward based on message length (length × 0.1) +- `done` (bool) - Always False for echo environment +- `metadata` (dict) - Additional info like step count + +### Reward +The reward is calculated as: `message_length × 0.1` +- "Hi" → reward: 0.2 +- "Hello, World!" → reward: 1.3 +- Empty message → reward: 0.0 + +## Advanced Usage + +### Connecting to an Existing Server + +If you already have an Echo environment server running, you can connect directly: + +```python +from envs.echo_env import EchoEnv + +# Connect to existing server +echo_env = EchoEnv(base_url="") + +# Use as normal +result = echo_env.reset() +result = echo_env.step(EchoAction(message="Hello!")) +``` + +Note: When connecting to an existing server, `echo_env.close()` will NOT stop the server. + +## Development & Testing + +### Direct Environment Testing + +Test the environment logic directly without starting the HTTP server: + +```bash +# From the server directory +python3 src/envs/echo_env/server/test_echo_env.py +``` + +This verifies that: +- Environment resets correctly +- Step executes actions properly +- State tracking works +- Rewards are calculated correctly + +### Running the Full Example + +Run the complete example that demonstrates the full workflow: + +```bash +python3 examples/local_echo_env.py +``` + +This example shows: +- Creating an environment from a Docker image +- Resetting and stepping through the environment +- Automatic cleanup with `close()` + +## Project Structure + +``` +echo_env/ +├── __init__.py # Module exports +├── README.md # This file +├── client.py # EchoEnv client implementation +├── models.py # Action and Observation models +└── server/ + ├── __init__.py # Server module exports + ├── echo_environment.py # Core environment logic + ├── app.py # FastAPI application + ├── test_echo_env.py # Direct environment tests + └── Dockerfile # Container image definition +``` diff --git a/src/envs/copy_env/__init__.py b/src/envs/copy_env/__init__.py new file mode 100644 index 00000000..6da62ba4 --- /dev/null +++ b/src/envs/copy_env/__init__.py @@ -0,0 +1,12 @@ +# 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. + +"""Echo Environment - A simple test environment for HTTP server.""" + +from .client import EchoEnv +from .models import EchoAction, EchoObservation + +__all__ = ["EchoAction", "EchoObservation", "EchoEnv"] diff --git a/src/envs/copy_env/client.py b/src/envs/copy_env/client.py new file mode 100644 index 00000000..0f5bdd4c --- /dev/null +++ b/src/envs/copy_env/client.py @@ -0,0 +1,101 @@ +# 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. + +""" +Echo Environment HTTP Client. + +This module provides the client for connecting to an Echo Environment server +over HTTP. +""" + +from typing import Any, Dict + +from core.client_types import StepResult + +from core.env_server.types import State +from core.http_env_client import HTTPEnvClient + +from .models import EchoAction, EchoObservation + + +class EchoEnv(HTTPEnvClient[EchoAction, EchoObservation]): + """ + HTTP client for the Echo Environment. + + This client connects to an EchoEnvironment HTTP server and provides + methods to interact with it: reset(), step(), and state access. + + Example: + >>> # Connect to a running server + >>> client = EchoEnv(base_url="http://localhost:8000") + >>> result = client.reset() + >>> print(result.observation.echoed_message) + >>> + >>> # Send a message + >>> result = client.step(EchoAction(message="Hello!")) + >>> print(result.observation.echoed_message) + >>> print(result.reward) + + Example with Docker: + >>> # Automatically start container and connect + >>> client = EchoEnv.from_docker_image("echo-env:latest") + >>> result = client.reset() + >>> result = client.step(EchoAction(message="Test")) + """ + + def _step_payload(self, action: EchoAction) -> Dict: + """ + Convert EchoAction to JSON payload for step request. + + Args: + action: EchoAction instance + + Returns: + Dictionary representation suitable for JSON encoding + """ + return { + "message": action.message, + } + + def _parse_result(self, payload: Dict) -> StepResult[EchoObservation]: + """ + Parse server response into StepResult[EchoObservation]. + + Args: + payload: JSON response from server + + Returns: + StepResult with EchoObservation + """ + obs_data = payload.get("observation", {}) + observation = EchoObservation( + echoed_message=obs_data.get("echoed_message", ""), + message_length=obs_data.get("message_length", 0), + done=payload.get("done", False), + reward=payload.get("reward"), + metadata=obs_data.get("metadata", {}), + ) + + return StepResult( + observation=observation, + reward=payload.get("reward"), + done=payload.get("done", False), + ) + + def _parse_state(self, payload: Dict) -> State: + """ + Parse server response into State object. + + Args: + payload: JSON response from /state endpoint + + Returns: + State object with episode_id and step_count + """ + return State( + episode_id=payload.get("episode_id"), + step_count=payload.get("step_count", 0), + ) diff --git a/src/envs/copy_env/models.py b/src/envs/copy_env/models.py new file mode 100644 index 00000000..d73134ba --- /dev/null +++ b/src/envs/copy_env/models.py @@ -0,0 +1,30 @@ +# 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. + +""" +Data models for the Echo Environment. + +The Echo environment is a simple test environment that echoes back messages. +""" + +from dataclasses import dataclass + +from core.env_server.types import Action, Observation + + +@dataclass(kw_only=True) +class EchoAction(Action): + """Action for the Echo environment - just a message to echo.""" + + message: str + + +@dataclass(kw_only=True) +class EchoObservation(Observation): + """Observation from the Echo environment - the echoed message.""" + + echoed_message: str + message_length: int = 0 \ No newline at end of file diff --git a/src/envs/copy_env/server/Dockerfile b/src/envs/copy_env/server/Dockerfile new file mode 100644 index 00000000..a50efc09 --- /dev/null +++ b/src/envs/copy_env/server/Dockerfile @@ -0,0 +1,25 @@ +# 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. + +# Use the standard openenv base image +# Built from: docker build -t openenv-base:latest -f src/core/containers/images/Dockerfile . +# In GitHub Actions, this is overridden to use the GHCR base image +ARG BASE_IMAGE=openenv-base:latest +FROM ${BASE_IMAGE} + +# Copy only what's needed for this environment +COPY src/core/ /app/src/core/ +COPY src/envs/echo_env/ /app/src/envs/echo_env/ + +# Copy README for web interface documentation +COPY src/envs/echo_env/README.md /app/README.md + +# Health check +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD curl -f http://localhost:8000/health || exit 1 + +# Run the FastAPI server +CMD ["uvicorn", "envs.echo_env.server.app:app", "--host", "0.0.0.0", "--port", "8000"] diff --git a/src/envs/copy_env/server/__init__.py b/src/envs/copy_env/server/__init__.py new file mode 100644 index 00000000..f6e24590 --- /dev/null +++ b/src/envs/copy_env/server/__init__.py @@ -0,0 +1,11 @@ +# 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. + +"""Echo environment server components.""" + +from .echo_environment import EchoEnvironment + +__all__ = ["EchoEnvironment"] \ No newline at end of file diff --git a/src/envs/copy_env/server/app.py b/src/envs/copy_env/server/app.py new file mode 100644 index 00000000..2605d555 --- /dev/null +++ b/src/envs/copy_env/server/app.py @@ -0,0 +1,39 @@ +# 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. + +""" +FastAPI application for the Echo Environment. + +This module creates an HTTP server that exposes the EchoEnvironment +over HTTP endpoints, making it compatible with HTTPEnvClient. + +Usage: + # Development (with auto-reload): + uvicorn envs.echo_env.server.app:app --reload --host 0.0.0.0 --port 8000 + + # Production: + uvicorn envs.echo_env.server.app:app --host 0.0.0.0 --port 8000 --workers 4 + + # Or run directly: + python -m envs.echo_env.server.app +""" + +from core.env_server.http_server import create_app + +from ..models import EchoAction, EchoObservation +from .echo_environment import EchoEnvironment + +# Create the environment instance +env = EchoEnvironment() + +# Create the app with web interface and README integration +app = create_app(env, EchoAction, EchoObservation, env_name="echo_env") + + +if __name__ == "__main__": + import uvicorn + + uvicorn.run(app, host="0.0.0.0", port=8000) \ No newline at end of file diff --git a/src/envs/copy_env/server/echo_environment.py b/src/envs/copy_env/server/echo_environment.py new file mode 100644 index 00000000..5033a9c1 --- /dev/null +++ b/src/envs/copy_env/server/echo_environment.py @@ -0,0 +1,95 @@ +# 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. + +""" +Echo Environment Implementation. + +A simple test environment that echoes back messages sent to it. +Perfect for testing HTTP server infrastructure. +""" + +from uuid import uuid4 + +from core.env_server.interfaces import Environment +from core.env_server.types import State + +from ..models import EchoAction, EchoObservation + + +class EchoEnvironment(Environment): + """ + A simple echo environment that echoes back messages. + + This environment is designed for testing the HTTP server infrastructure. + It maintains minimal state and simply echoes back whatever message it receives. + + Example: + >>> env = EchoEnvironment() + >>> obs = env.reset() + >>> print(obs.echoed_message) # "Echo environment ready!" + >>> + >>> obs = env.step(EchoAction(message="Hello")) + >>> print(obs.echoed_message) # "Hello" + >>> print(obs.message_length) # 5 + """ + + def __init__(self): + """Initialize the echo environment.""" + self._state = State(episode_id=str(uuid4()), step_count=0) + self._reset_count = 0 + + def reset(self) -> EchoObservation: + """ + Reset the environment. + + Returns: + EchoObservation with a ready message + """ + self._state = State(episode_id=str(uuid4()), step_count=0) + self._reset_count += 1 + + return EchoObservation( + echoed_message="Echo environment ready!", + message_length=0, + done=False, + reward=0.0, + ) + + def step(self, action: EchoAction) -> EchoObservation: # type: ignore[override] + """ + Execute a step in the environment by echoing the message. + + Args: + action: EchoAction containing the message to echo + + Returns: + EchoObservation with the echoed message and its length + """ + self._state.step_count += 1 + + message = action.message + length = len(message) + + # Simple reward: longer messages get higher rewards + reward = length * 0.1 + + return EchoObservation( + echoed_message=message, + message_length=length, + done=False, + reward=reward, + metadata={"original_message": message, "step": self._state.step_count}, + ) + + @property + def state(self) -> State: + """ + Get the current environment state. + + Returns: + Current State with episode_id and step_count + """ + return self._state From 2ebce29b0b9188cb35721d5070db692777035c83 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sat, 1 Nov 2025 14:23:45 +0100 Subject: [PATCH 02/38] fix yaml on branch --- .github/workflows/pr-new-env-health-check.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/pr-new-env-health-check.yml b/.github/workflows/pr-new-env-health-check.yml index 5ff0c2fd..fe82f30d 100644 --- a/.github/workflows/pr-new-env-health-check.yml +++ b/.github/workflows/pr-new-env-health-check.yml @@ -182,5 +182,3 @@ jobs: fi exit 1 fi - ->>>>>>> gh-action-deploy-test From 92501b5eb7bf91c9c55ec6dc03e99065cdfa2b28 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sat, 1 Nov 2025 14:37:25 +0100 Subject: [PATCH 03/38] reduce wait to 3 minutes --- .github/workflows/pr-new-env-health-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-new-env-health-check.yml b/.github/workflows/pr-new-env-health-check.yml index fe82f30d..f022f041 100644 --- a/.github/workflows/pr-new-env-health-check.yml +++ b/.github/workflows/pr-new-env-health-check.yml @@ -142,7 +142,7 @@ jobs: - name: Wait for deployment to stabilize shell: bash - run: sleep 300 + run: sleep 180 - name: Perform environment health check shell: bash From 37cd0ac0b851e888e88a86fe0abc12161089e623 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sat, 1 Nov 2025 14:42:56 +0100 Subject: [PATCH 04/38] add commenting to space pr --- .github/workflows/pr-new-env-health-check.yml | 89 +++++++++++++++++-- 1 file changed, 81 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr-new-env-health-check.yml b/.github/workflows/pr-new-env-health-check.yml index f022f041..55d935e0 100644 --- a/.github/workflows/pr-new-env-health-check.yml +++ b/.github/workflows/pr-new-env-health-check.yml @@ -11,7 +11,7 @@ on: permissions: contents: read - pull-requests: read + pull-requests: write jobs: detect-new-envs: @@ -138,19 +138,20 @@ jobs: run: | set -euo pipefail chmod +x scripts/deploy_to_hf.sh - ./scripts/deploy_to_hf.sh --env "${{ matrix.environment }}" --space-suffix "${SPACE_SUFFIX}" --private + ./scripts/deploy_to_hf.sh --env "${{ matrix.environment }}" --space-suffix "${SPACE_SUFFIX}" - name: Wait for deployment to stabilize shell: bash run: sleep 180 - - name: Perform environment health check + - name: Compute Space URLs + id: urls shell: bash run: | set -euo pipefail if [ -z "${HF_NAMESPACE:-}" ]; then - echo "HF_NAMESPACE is not configured; unable to compute health check URL." >&2 + echo "HF_NAMESPACE is not configured; unable to compute space URLs." >&2 exit 1 fi @@ -158,14 +159,38 @@ jobs: space_name="${{ matrix.environment }}${SPACE_SUFFIX}" space_slug=$(echo "${space_name}" | tr '[:upper:]' '[:lower:]' | tr '_' '-') health_url="https://${namespace_slug}-${space_slug}.hf.space/health" + live_url="https://${namespace_slug}-${space_slug}.hf.space" + space_repo_url="https://huggingface.co/spaces/${HF_NAMESPACE}/${space_name}" + + echo "namespace_slug=${namespace_slug}" >> "$GITHUB_OUTPUT" + echo "space_name=${space_name}" >> "$GITHUB_OUTPUT" + echo "space_slug=${space_slug}" >> "$GITHUB_OUTPUT" + echo "health_url=${health_url}" >> "$GITHUB_OUTPUT" + echo "live_url=${live_url}" >> "$GITHUB_OUTPUT" + echo "space_repo_url=${space_repo_url}" >> "$GITHUB_OUTPUT" - echo "Checking health for ${space_name} at ${health_url}" + - name: Perform environment health check + id: health_check + continue-on-error: true + shell: bash + env: + HEALTH_URL: ${{ steps.urls.outputs.health_url }} + SPACE_NAME: ${{ steps.urls.outputs.space_name }} + run: | + set -euo pipefail + + if [ -z "${HEALTH_URL:-}" ]; then + echo "HEALTH_URL not provided; cannot perform health check." >&2 + exit 1 + fi + + echo "Checking health for ${SPACE_NAME} at ${HEALTH_URL}" success=0 for attempt in {1..5}; do - status=$(curl -sS -o response.json -w "%{http_code}" "$health_url" || echo "000") + status=$(curl -sS -o response.json -w "%{http_code}" "$HEALTH_URL" || echo "000") if [ "$status" = "200" ]; then - echo "Health check passed for ${space_name}" + echo "Health check passed for ${SPACE_NAME}" cat response.json success=1 break @@ -175,10 +200,58 @@ jobs: done if [ $success -ne 1 ]; then - echo "Health check failed for ${space_name}" >&2 + echo "Health check failed for ${SPACE_NAME}" >&2 if [ -f response.json ]; then echo "Last response payload:" cat response.json fi exit 1 fi + + - name: Comment on PR with deployment status + if: always() + uses: actions/github-script@v7 + env: + HEALTH_CONCLUSION: ${{ steps.health_check.conclusion }} + SPACE_NAME: ${{ steps.urls.outputs.space_name }} + LIVE_URL: ${{ steps.urls.outputs.live_url }} + SPACE_REPO_URL: ${{ steps.urls.outputs.space_repo_url }} + ENV_NAME: ${{ matrix.environment }} + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const status = process.env.HEALTH_CONCLUSION || 'failure'; + const spaceName = process.env.SPACE_NAME; + const liveUrl = process.env.LIVE_URL; + const repoUrl = process.env.SPACE_REPO_URL; + const envName = process.env.ENV_NAME; + + const header = status === 'success' + ? `✅ Deployment succeeded for \`${envName}\`` + : `âš ī¸ Deployment failed for \`${envName}\``; + + const summary = status === 'success' + ? 'Nice work! Wait for a code review and we\'re ready to go.' + : 'Please resolve your environment.'; + + const body = [ + header, + '', + `- Space repo: [${repoUrl}](${repoUrl})`, + `- Live URL: [${liveUrl}](${liveUrl})`, + '', + summary, + '', + 'You can iterate locally or validate fixes by running `scripts/deploy_to_hf.sh --env "' + envName + '"`.' + ].join('\n'); + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.pull_request.number, + body + }); + + - name: Fail job if health check failed + if: steps.health_check.conclusion == 'failure' + run: exit 1 From e0ba98702b226f7bd176de24c2ece77fb613a597 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sat, 1 Nov 2025 14:50:03 +0100 Subject: [PATCH 05/38] rename --- .github/workflows/pr-new-env-health-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-new-env-health-check.yml b/.github/workflows/pr-new-env-health-check.yml index 55d935e0..63cd65fb 100644 --- a/.github/workflows/pr-new-env-health-check.yml +++ b/.github/workflows/pr-new-env-health-check.yml @@ -1,4 +1,4 @@ -name: PR New Environment +name: Deploy Test Environment for Environment PR on: pull_request_target: From 69f6556c71cdcb1cfcf7602d687019b8066812ef Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 20:17:16 -0400 Subject: [PATCH 06/38] Add OpenEnv CLI for managing environments on HuggingFace Spaces - Introduced `openenv` command-line interface for environment management. - Implemented `push` command to deploy environments to HuggingFace Spaces. - Added authentication handling with HuggingFace API. - Created staging directory structure and prepared Dockerfiles and README files for deployment. - Included tests for authentication, builder, uploader, and push command functionalities. - Updated `pyproject.toml` to include new dependencies and scripts. --- pyproject.toml | 13 +- src/openenv_cli/README.md | 376 +++++++++++++++++++++++++++ src/openenv_cli/__init__.py | 9 + src/openenv_cli/__main__.py | 74 ++++++ src/openenv_cli/commands/__init__.py | 7 + src/openenv_cli/commands/push.py | 94 +++++++ src/openenv_cli/core/__init__.py | 7 + src/openenv_cli/core/auth.py | 97 +++++++ src/openenv_cli/core/builder.py | 223 ++++++++++++++++ src/openenv_cli/core/space.py | 77 ++++++ src/openenv_cli/core/uploader.py | 41 +++ src/openenv_cli/utils/__init__.py | 7 + src/openenv_cli/utils/env_loader.py | 91 +++++++ tests/__init__.py | 7 + tests/test_cli/__init__.py | 7 + tests/test_cli/test_auth.py | 156 +++++++++++ tests/test_cli/test_builder.py | 251 ++++++++++++++++++ tests/test_cli/test_push_command.py | 235 +++++++++++++++++ tests/test_cli/test_space.py | 135 ++++++++++ tests/test_cli/test_uploader.py | 87 +++++++ 20 files changed, 1993 insertions(+), 1 deletion(-) create mode 100644 src/openenv_cli/README.md create mode 100644 src/openenv_cli/__init__.py create mode 100644 src/openenv_cli/__main__.py create mode 100644 src/openenv_cli/commands/__init__.py create mode 100644 src/openenv_cli/commands/push.py create mode 100644 src/openenv_cli/core/__init__.py create mode 100644 src/openenv_cli/core/auth.py create mode 100644 src/openenv_cli/core/builder.py create mode 100644 src/openenv_cli/core/space.py create mode 100644 src/openenv_cli/core/uploader.py create mode 100644 src/openenv_cli/utils/__init__.py create mode 100644 src/openenv_cli/utils/env_loader.py create mode 100644 tests/__init__.py create mode 100644 tests/test_cli/__init__.py create mode 100644 tests/test_cli/test_auth.py create mode 100644 tests/test_cli/test_builder.py create mode 100644 tests/test_cli/test_push_command.py create mode 100644 tests/test_cli/test_space.py create mode 100644 tests/test_cli/test_uploader.py diff --git a/pyproject.toml b/pyproject.toml index 6c9fb689..7b6fafe5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,8 @@ dependencies = [ "requests>=2.25.0", "fastapi>=0.104.0", "uvicorn>=0.24.0", - "smolagents>=1.22.0,<2" + "smolagents>=1.22.0,<2", + "huggingface_hub>=0.20.0" ] [tool.setuptools] @@ -20,3 +21,13 @@ package-dir = {"" = "src"} [tool.setuptools.packages.find] where = ["src"] + +[project.scripts] +openenv = "openenv_cli.__main__:main" + +[project.optional-dependencies] +dev = [ + "pytest>=7.0.0", + "pytest-mock>=3.10.0", + "pytest-cov>=4.0.0" +] diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md new file mode 100644 index 00000000..b7cf31c2 --- /dev/null +++ b/src/openenv_cli/README.md @@ -0,0 +1,376 @@ +# OpenEnv CLI + +Command-line tool for managing and deploying OpenEnv environments to HuggingFace Spaces. + +## Overview + +The OpenEnv CLI provides a self-service workflow for publishing environments to HuggingFace Spaces, enabling community members to share environments without requiring GitHub PRs. The CLI handles authentication, space provisioning, building, and deployment automatically. + +## Installation + +The CLI is installed as part of the OpenEnv package: + +```bash +pip install -e . +``` + +Or install with development dependencies: + +```bash +pip install -e ".[dev]" +``` + +## Usage + +### Push Environment + +Push an environment to HuggingFace Spaces: + +```bash +openenv push [options] +``` + +**Arguments:** +- `env_name`: Name of the environment to push (e.g., `echo_env`, `coding_env`) + +**Options:** +- `--namespace `: HuggingFace namespace (organization or user). If not provided, uses authenticated user's username. +- `--private`: Create a private space (default: public) +- `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) +- `--dry-run`: Prepare files but don't upload to HuggingFace + +**Examples:** + +```bash +# Push echo_env to your personal namespace +openenv push echo_env + +# Push to a specific organization +openenv push coding_env --namespace my-org + +# Create a private space +openenv push echo_env --private + +# Use a custom base image +openenv push echo_env --base-image ghcr.io/my-org/custom-base:latest + +# Prepare files without uploading +openenv push echo_env --dry-run +``` + +### Authentication + +The CLI uses HuggingFace authentication. You can authenticate in two ways: + +1. **Environment Variable**: Set `HF_TOKEN` environment variable with your HuggingFace token + ```bash + export HF_TOKEN=your_token_here + ``` + +2. **Interactive Login**: If no token is found, the CLI will prompt for interactive login: + ```bash + # The CLI will automatically prompt when needed + openenv push echo_env + ``` + +To get a token: +1. Go to https://huggingface.co/settings/tokens +2. Create a new token with "write" permissions +3. Use it as `HF_TOKEN` or log in interactively + +## How It Works + +The `openenv push` command performs the following steps: + +1. **Validation**: Checks that the environment exists in `src/envs//` +2. **Authentication**: Ensures you're authenticated with HuggingFace (prompts if needed) +3. **Space Provisioning**: Checks if a Docker Space exists, creates it if needed +4. **Build Process**: + - Creates a staging directory + - Copies core and environment files + - Generates/modifies Dockerfile with web interface enabled + - Creates README with HuggingFace front matter +5. **Deployment**: Uploads all files to the HuggingFace Space +6. **Cleanup**: Removes staging directory after successful upload + +## Testing + +### Running Tests + +The CLI uses pytest for testing. Run all tests: + +```bash +# From project root +pytest tests/test_cli/ +``` + +Run specific test files: + +```bash +# Test authentication +pytest tests/test_cli/test_auth.py + +# Test space management +pytest tests/test_cli/test_space.py + +# Test builder +pytest tests/test_cli/test_builder.py + +# Test uploader +pytest tests/test_cli/test_uploader.py + +# Test push command (end-to-end) +pytest tests/test_cli/test_push_command.py +``` + +### Test Structure + +Tests follow pytest conventions and use mocking to avoid requiring actual HuggingFace API access: + +- **Unit Tests**: Each module (`auth`, `space`, `builder`, `uploader`) has dedicated tests +- **Integration Tests**: `test_push_command.py` tests the full workflow with mocks +- **Fixtures**: Reusable test fixtures for mock APIs and temporary directories + +### Writing Tests + +Follow these patterns when adding new tests: + +```python +from unittest.mock import Mock, patch +import pytest + +@patch("openenv_cli.core.module.external_api_call") +def test_feature(mock_api): + """Test description.""" + mock_api.return_value = {"result": "success"} + + result = function_under_test() + + assert result == expected_value + mock_api.assert_called_once() +``` + +### Test Coverage + +Run tests with coverage: + +```bash +pytest --cov=openenv_cli --cov-report=html tests/test_cli/ +``` + +## Architecture + +The CLI is organized into modular components: + +``` +src/openenv_cli/ +├── __main__.py # CLI entry point +├── commands/ +│ └── push.py # Push command implementation +├── core/ +│ ├── auth.py # HuggingFace authentication +│ ├── space.py # Space management (create/check) +│ ├── builder.py # Build staging directory and files +│ └── uploader.py # Upload to HuggingFace Spaces +└── utils/ + └── env_loader.py # Environment validation and metadata +``` + +### Module Responsibilities + +- **`auth.py`**: Handles HuggingFace authentication using `huggingface_hub` +- **`space.py`**: Manages Docker Spaces (check existence, create) +- **`builder.py`**: Prepares deployment package (copy files, generate Dockerfile/README) +- **`uploader.py`**: Uploads files to HuggingFace using `upload_folder` +- **`env_loader.py`**: Validates environment structure and loads metadata + +## Implementation Details + +### Using huggingface_hub Modules + +The CLI uses `huggingface_hub` Python modules directly (not the CLI), as specified: + +- `huggingface_hub.HfApi` for API calls +- `huggingface_hub.login()` for authentication +- `huggingface_hub.upload_folder()` for file uploads +- `huggingface_hub.utils.get_token` for token management + +This ensures consistency with HuggingFace tooling and allows for better error handling and programmatic control. + +### Web Interface + +All pushed environments automatically include the web interface: + +- The Dockerfile is modified to set `ENV ENABLE_WEB_INTERFACE=true` +- The web interface is available at `/web` on deployed spaces +- Includes HumanAgent interface, state observer, and WebSocket updates + +### Staging Directory + +The builder creates a staging directory structure: + +``` +hf-staging// +├── Dockerfile # Generated/modified Dockerfile +├── README.md # With HuggingFace front matter +└── src/ + ├── core/ # Copied from src/core/ + └── envs/ + └── / # Copied from src/envs// +``` + +The staging directory is cleaned up after successful upload. + +## Future Direction + +The CLI architecture is designed to support additional commands for a complete environment management workflow: + +### Planned Commands + +#### `openenv init` + +Initialize a new environment with the standard OpenEnv structure: + +```bash +openenv init my_new_env --template coding +``` + +**Features:** +- Create environment directory structure +- Generate boilerplate files (models.py, client.py, server/) +- Initialize Dockerfile and app.py +- Create README template + +#### `openenv upgrade` + +Upgrade an existing environment to a new OpenEnv version: + +```bash +openenv upgrade my_env --to-version 0.2.0 +``` + +**Features:** +- Check for breaking changes +- Update dependencies +- Migrate code to new API versions +- Backup existing environment + +#### `openenv test` + +Run local tests before pushing: + +```bash +openenv test my_env +# or +openenv test my_env --docker +``` + +**Features:** +- Run environment unit tests +- Test Docker build process +- Validate environment structure +- Check for common issues + +#### `openenv validate` + +Pre-submission validation: + +```bash +openenv validate my_env +``` + +**Features:** +- Validate environment structure +- Check Dockerfile syntax +- Verify README formatting +- Test web interface generation +- Validate models and client code + +### Extension Points + +The CLI architecture supports extension through: + +1. **Command Registration**: Add new commands in `commands/` and register in `__main__.py` +2. **Core Modules**: Add new core functionality (e.g., `core/validator.py`) +3. **Shared Utilities**: Extend `utils/` for reusable functions + +### Example: Adding a New Command + +```python +# src/openenv_cli/commands/validate.py +def validate_environment(env_name: str) -> None: + """Validate environment structure.""" + # Implementation + pass + +# src/openenv_cli/__main__.py +# Add to subparsers: +validate_parser = subparsers.add_parser("validate", help="Validate environment") +validate_parser.add_argument("env_name") +# ... handle command +``` + +### Integration with Phase 2 + +When HuggingFace native Environment primitives become available (Phase 2), the CLI will be updated to: + +- Use native Environment APIs instead of Docker Space orchestration +- Leverage HuggingFace's built-in build processes +- Simplify deployment workflow (less custom logic needed) +- Potentially deprecate or simplify the current `push` command + +The modular architecture ensures a smooth transition path. + +## Troubleshooting + +### Authentication Issues + +**Problem**: "Failed to retrieve token after login" + +**Solution**: +- Check that `huggingface_hub` is properly installed: `pip install --upgrade huggingface_hub` +- Verify token permissions (needs "write" access) +- Try logging in via CLI: `huggingface-cli login` + +### Space Creation Fails + +**Problem**: "Failed to create space" + +**Solution**: +- Check that namespace/username is correct +- Verify you have permission to create spaces in that namespace +- Ensure space name doesn't already exist (check on huggingface.co) + +### Upload Fails + +**Problem**: "Failed to upload to space" + +**Solution**: +- Check internet connection +- Verify token hasn't expired +- Try `--dry-run` first to check file preparation +- Check staging directory exists and has files + +### Environment Not Found + +**Problem**: "Environment 'xyz' not found" + +**Solution**: +- Verify environment exists in `src/envs//` +- Check spelling of environment name +- Ensure environment directory has required structure (models.py, server/, etc.) + +## Contributing + +When adding new features: + +1. **Write Tests First**: Follow TDD approach, write tests before implementation +2. **Use Mocks**: Mock external APIs (HuggingFace) to keep tests fast and isolated +3. **Follow Patterns**: Match existing code style and patterns +4. **Update Documentation**: Update this README and add docstrings + +## References + +- [HuggingFace Hub Python API](https://huggingface.co/docs/huggingface_hub/index) +- [OpenEnv Environment Structure](../envs/README.md) +- [OpenEnv RFCs](../../rfcs/) diff --git a/src/openenv_cli/__init__.py b/src/openenv_cli/__init__.py new file mode 100644 index 00000000..30f09e5b --- /dev/null +++ b/src/openenv_cli/__init__.py @@ -0,0 +1,9 @@ +# 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. + +"""OpenEnv CLI - Command-line tool for managing OpenEnv environments.""" + +__version__ = "0.1.0" diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py new file mode 100644 index 00000000..87d70a41 --- /dev/null +++ b/src/openenv_cli/__main__.py @@ -0,0 +1,74 @@ +# 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. + +"""CLI entry point for OpenEnv.""" + +import argparse +import sys + +from .commands.push import push_environment + + +def main(): + """Main entry point for OpenEnv CLI.""" + parser = argparse.ArgumentParser( + prog="openenv", + description="OpenEnv CLI - Manage and deploy OpenEnv environments", + ) + + subparsers = parser.add_subparsers(dest="command", help="Command to run") + + # Push command + push_parser = subparsers.add_parser( + "push", + help="Push an environment to HuggingFace Spaces", + ) + push_parser.add_argument( + "env_name", + help="Name of the environment to push (e.g., echo_env)", + ) + push_parser.add_argument( + "--namespace", + help="HuggingFace namespace (organization or user). " + "If not provided, uses authenticated user's username.", + ) + push_parser.add_argument( + "--private", + action="store_true", + help="Create a private space (default: public)", + ) + push_parser.add_argument( + "--base-image", + help="Base Docker image to use " + "(default: ghcr.io/meta-pytorch/openenv-base:latest)", + ) + push_parser.add_argument( + "--dry-run", + action="store_true", + help="Prepare files but don't upload to HuggingFace", + ) + + args = parser.parse_args() + + if args.command == "push": + try: + push_environment( + env_name=args.env_name, + namespace=args.namespace, + private=args.private, + base_image=args.base_image, + dry_run=args.dry_run, + ) + except Exception as e: + print(f"Error: {e}", file=sys.stderr) + sys.exit(1) + else: + parser.print_help() + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/src/openenv_cli/commands/__init__.py b/src/openenv_cli/commands/__init__.py new file mode 100644 index 00000000..1a9052af --- /dev/null +++ b/src/openenv_cli/commands/__init__.py @@ -0,0 +1,7 @@ +# 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. + +"""OpenEnv CLI commands.""" diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py new file mode 100644 index 00000000..41ffa67e --- /dev/null +++ b/src/openenv_cli/commands/push.py @@ -0,0 +1,94 @@ +# 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. + +"""Push command for deploying environments to HuggingFace Spaces.""" + +from pathlib import Path +from typing import Optional + +from huggingface_hub import HfApi + +from ..core.auth import ensure_authenticated +from ..core.builder import ( + prepare_staging_directory, + copy_environment_files, + prepare_dockerfile, + prepare_readme, +) +from ..core.space import space_exists, create_space, get_space_repo_id +from ..core.uploader import upload_to_space +from ..utils.env_loader import validate_environment + + +def push_environment( + env_name: str, + namespace: Optional[str] = None, + private: bool = False, + base_image: Optional[str] = None, + dry_run: bool = False, +) -> None: + """ + Push an environment to HuggingFace Spaces. + + Args: + env_name: Name of the environment to push. + namespace: Optional namespace (organization or user). If not provided, + uses the authenticated user's username. + private: Whether the space should be private (default: False). + base_image: Base Docker image to use (default: ghcr.io/meta-pytorch/openenv-base:latest). + dry_run: If True, prepare files but don't upload (default: False). + """ + # Validate environment exists + validate_environment(env_name) + + # Authenticate with HuggingFace + username, token = ensure_authenticated() + + # Determine target space repo ID + repo_id = get_space_repo_id(env_name, namespace=namespace) + + # Create HfApi instance + api = HfApi(token=token) + + # Check if space exists, create if needed + if not space_exists(api, repo_id): + create_space(api, repo_id, private=private) + else: + print(f"Space {repo_id} already exists, will update it") + + # Set default base image if not provided + if base_image is None: + base_image = "ghcr.io/meta-pytorch/openenv-base:latest" + + # Prepare staging directory + staging_dir = prepare_staging_directory(env_name, base_image) + + try: + # Copy files + copy_environment_files(env_name, staging_dir) + + # Prepare Dockerfile + prepare_dockerfile(env_name, staging_dir, base_image) + + # Prepare README + prepare_readme(env_name, staging_dir) + + if dry_run: + print(f"Dry run: Files prepared in {staging_dir}") + print(f"Would upload to: https://huggingface.co/spaces/{repo_id}") + return + + # Upload to space + print(f"Uploading to space: {repo_id}") + upload_to_space(api, repo_id, staging_dir, token) + + print(f"✅ Successfully pushed {env_name} to https://huggingface.co/spaces/{repo_id}") + + finally: + # Cleanup staging directory after upload (or if upload fails) + if not dry_run and staging_dir.exists(): + import shutil + shutil.rmtree(staging_dir) diff --git a/src/openenv_cli/core/__init__.py b/src/openenv_cli/core/__init__.py new file mode 100644 index 00000000..3d972e0f --- /dev/null +++ b/src/openenv_cli/core/__init__.py @@ -0,0 +1,7 @@ +# 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. + +"""OpenEnv CLI core modules.""" diff --git a/src/openenv_cli/core/auth.py b/src/openenv_cli/core/auth.py new file mode 100644 index 00000000..e949b507 --- /dev/null +++ b/src/openenv_cli/core/auth.py @@ -0,0 +1,97 @@ +# 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. + +"""Authentication module for HuggingFace.""" + +import os +from typing import Optional, Tuple + +from huggingface_hub import HfApi, login +from huggingface_hub.utils import get_token + + +def check_authentication() -> Optional[str]: + """ + Check if user is authenticated with HuggingFace. + + Returns: + Username if authenticated, None otherwise. + """ + # Check for token in environment variable first + token = os.environ.get("HF_TOKEN") + + if not token: + # Try to get token from stored credentials + token = get_token() + + if not token: + return None + + try: + api = HfApi(token=token) + user_info = api.whoami() + return user_info.get("name") + except Exception: + # Invalid token or network error + return None + + +def ensure_authenticated() -> Tuple[str, str]: + """ + Ensure user is authenticated, prompting for login if needed. + + Returns: + Tuple of (username, token). + + Raises: + Exception: If authentication fails. + """ + # Check for token in environment variable first + token = os.environ.get("HF_TOKEN") + + if token: + try: + api = HfApi(token=token) + user_info = api.whoami() + return user_info.get("name"), token + except Exception: + pass # Fall through to login + + # Check existing authentication + username = check_authentication() + if username: + token = get_token() or os.environ.get("HF_TOKEN") + if token: + return username, token + + # Need to login + username = login_interactive() + token = get_token() or os.environ.get("HF_TOKEN") + if not token: + raise Exception("Failed to retrieve token after login") + + return username, token + + +def login_interactive() -> str: + """ + Perform interactive login to HuggingFace. + + Returns: + Username after successful login. + + Raises: + Exception: If login fails. + """ + try: + login() + # Verify login was successful + username = check_authentication() + if not username: + raise Exception("Login failed: unable to verify authentication") + return username + except Exception as e: + raise Exception(f"Login failed: {str(e)}") diff --git a/src/openenv_cli/core/builder.py b/src/openenv_cli/core/builder.py new file mode 100644 index 00000000..d3339b41 --- /dev/null +++ b/src/openenv_cli/core/builder.py @@ -0,0 +1,223 @@ +# 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. + +"""Builder module for preparing environments for deployment.""" + +import os +import re +import shutil +from pathlib import Path +from typing import Optional + + +def prepare_staging_directory(env_name: str, base_image: str, staging_root: str = "hf-staging") -> Path: + """ + Prepare staging directory structure for deployment. + + Args: + env_name: Name of the environment. + base_image: Base Docker image to use. + staging_root: Root directory for staging (default: "hf-staging"). + + Returns: + Path to the staging directory. + """ + # Create staging directory + staging_dir = Path(staging_root) / env_name + if staging_dir.exists(): + shutil.rmtree(staging_dir) + staging_dir.mkdir(parents=True) + + # Create subdirectories + (staging_dir / "src" / "core").mkdir(parents=True) + (staging_dir / "src" / "envs" / env_name).mkdir(parents=True) + + return staging_dir + + +def copy_environment_files(env_name: str, staging_dir: Path) -> None: + """ + Copy environment files to staging directory. + + Args: + env_name: Name of the environment. + staging_dir: Staging directory path. + """ + # Copy core files + core_src = Path("src/core") + core_dst = staging_dir / "src" / "core" + if core_src.exists(): + shutil.copytree(core_src, core_dst, dirs_exist_ok=True) + + # Copy environment files + env_src = Path("src/envs") / env_name + env_dst = staging_dir / "src" / "envs" / env_name + if not env_src.exists(): + raise FileNotFoundError(f"Environment not found: {env_src}") + shutil.copytree(env_src, env_dst, dirs_exist_ok=True) + + +def prepare_dockerfile(env_name: str, staging_dir: Path, base_image: str) -> None: + """ + Prepare Dockerfile for deployment. + + Uses the environment's Dockerfile if it exists, otherwise creates a default one. + + Args: + env_name: Name of the environment. + staging_dir: Staging directory path. + base_image: Base Docker image to use. + """ + env_dockerfile = Path("src/envs") / env_name / "server" / "Dockerfile" + dockerfile_path = staging_dir / "Dockerfile" + + if env_dockerfile.exists(): + # Copy and modify existing Dockerfile + content = env_dockerfile.read_text() + + # Replace BASE_IMAGE references + content = re.sub( + r'ARG\s+BASE_IMAGE=.*', + f'ARG BASE_IMAGE={base_image}', + content + ) + content = re.sub( + r'FROM\s+\$\{BASE_IMAGE\}', + f'FROM {base_image}', + content + ) + # Also handle direct FROM statements + content = re.sub( + r'FROM\s+openenv-base:.*', + f'FROM {base_image}', + content + ) + + dockerfile_path.write_text(content) + else: + # Create default Dockerfile + default_dockerfile = f"""# 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. + +# Use the specified openenv-base image +FROM {base_image} + +# Copy only what's needed for this environment +COPY src/core/ /app/src/core/ +COPY src/envs/{env_name}/ /app/src/envs/{env_name}/ + +# Health check +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \\ + CMD curl -f http://localhost:8000/health || exit 1 + +# Run the FastAPI server +CMD ["uvicorn", "envs.{env_name}.server.app:app", "--host", "0.0.0.0", "--port", "8000"] +""" + dockerfile_path.write_text(default_dockerfile) + + # Ensure web interface is enabled + content = dockerfile_path.read_text() + if "ENABLE_WEB_INTERFACE" not in content: + # Add before the CMD line + content = re.sub( + r'(CMD\s+\[)', + r'ENV ENABLE_WEB_INTERFACE=true\n\n\1', + content + ) + dockerfile_path.write_text(content) + + +def prepare_readme(env_name: str, staging_dir: Path) -> None: + """ + Prepare README.md with HuggingFace front matter. + + Args: + env_name: Name of the environment. + staging_dir: Staging directory path. + """ + # Capitalize first letter of environment name + env_title = env_name[0].upper() + env_name[1:] if env_name else env_name + + # Set environment-specific colors and emoji + emoji_map = { + "atari_env": ("đŸ•šī¸", "red", "yellow"), + "coding_env": ("đŸ’ģ", "blue", "gray"), + "openspiel_env": ("🎮", "purple", "indigo"), + "echo_env": ("🔊", "blue", "gray"), + "chat_env": ("đŸ’Ŧ", "blue", "green"), + "textarena_env": ("📜", "green", "blue"), + } + + emoji, color_from, color_to = emoji_map.get(env_name, ("đŸŗ", "blue", "green")) + + # Create README with front matter + readme_content = f"""--- +title: {env_title} Environment Server +emoji: {emoji} +colorFrom: {color_from} +colorTo: {color_to} +sdk: docker +pinned: false +app_port: 8000 +base_path: /web +tags: + - openenv +--- + +# {env_title} Environment Server + +FastAPI server for {env_name} environment powered by Meta's OpenEnv. + +## About + +This Space provides a containerized environment for {env_name} interactions. +Built with FastAPI and OpenEnv framework. + +## Web Interface + +This deployment includes an interactive web interface for exploring the environment: +- **HumanAgent Interface**: Interact with the environment using a web form +- **State Observer**: Real-time view of environment state and action history +- **Live Updates**: WebSocket-based real-time updates + +Access the web interface at: `/web` + +## API Documentation + +Visit `/docs` for interactive API documentation. + +## Health Check + +The environment provides a health check endpoint at `/health`. +""" + + # Try to append content from original README if it exists + original_readme = Path("src/envs") / env_name / "README.md" + if original_readme.exists(): + original_content = original_readme.read_text() + + # Skip front matter if present + if original_content.startswith("---"): + # Find closing --- + lines = original_content.split("\n") + closing_idx = None + for i in range(1, len(lines)): + if lines[i].strip() == "---": + closing_idx = i + break + + if closing_idx: + # Skip front matter + original_content = "\n".join(lines[closing_idx + 1:]) + + # Append original content + readme_content += "\n" + original_content + + readme_path = staging_dir / "README.md" + readme_path.write_text(readme_content) diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py new file mode 100644 index 00000000..bc8f99b5 --- /dev/null +++ b/src/openenv_cli/core/space.py @@ -0,0 +1,77 @@ +# 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. + +"""Space management module for HuggingFace Spaces.""" + +from typing import Optional + +from huggingface_hub import HfApi + +from .auth import ensure_authenticated + + +def space_exists(api: HfApi, repo_id: str) -> bool: + """ + Check if a Docker Space exists. + + Args: + api: HfApi instance to use for API calls. + repo_id: Repository ID in format 'namespace/repo-name'. + + Returns: + True if space exists, False otherwise. + """ + try: + return api.repo_exists(repo_id=repo_id, repo_type="space") + except Exception: + return False + + +def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: + """ + Create a Docker Space on HuggingFace. + + Args: + api: HfApi instance to use for API calls. + repo_id: Repository ID in format 'namespace/repo-name'. + private: Whether the space should be private (default: False). + + Raises: + Exception: If space creation fails. + """ + try: + api.create_repo( + repo_id=repo_id, + repo_type="space", + space_sdk="docker", + private=private, + exist_ok=True # Don't fail if space already exists + ) + except Exception as e: + # If space already exists, that's okay + if "already exists" in str(e).lower() or "repository already exists" in str(e).lower(): + return + raise + + +def get_space_repo_id(env_name: str, namespace: Optional[str] = None) -> str: + """ + Get the full repository ID for a space. + + Args: + env_name: Environment name (e.g., "echo_env"). + namespace: Optional namespace (organization or user). If not provided, + uses the authenticated user's username. + + Returns: + Repository ID in format 'namespace/env-name'. + """ + if namespace: + return f"{namespace}/{env_name}" + + # Use authenticated user's username + username, _ = ensure_authenticated() + return f"{username}/{env_name}" diff --git a/src/openenv_cli/core/uploader.py b/src/openenv_cli/core/uploader.py new file mode 100644 index 00000000..001fc3fe --- /dev/null +++ b/src/openenv_cli/core/uploader.py @@ -0,0 +1,41 @@ +# 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. + +"""Uploader module for deploying to HuggingFace Spaces.""" + +from pathlib import Path + +from huggingface_hub import HfApi +from huggingface_hub import upload_folder + + +def upload_to_space( + api: HfApi, + repo_id: str, + staging_dir: Path, + token: str, +) -> None: + """ + Upload staging directory contents to HuggingFace Space. + + Args: + api: HfApi instance to use for API calls. + repo_id: Repository ID in format 'namespace/repo-name'. + staging_dir: Path to staging directory to upload. + token: HuggingFace token for authentication. + + Raises: + Exception: If upload fails. + """ + try: + upload_folder( + folder_path=str(staging_dir), + repo_id=repo_id, + repo_type="space", + token=token, + ) + except Exception as e: + raise Exception(f"Failed to upload to space {repo_id}: {str(e)}") diff --git a/src/openenv_cli/utils/__init__.py b/src/openenv_cli/utils/__init__.py new file mode 100644 index 00000000..df97cc42 --- /dev/null +++ b/src/openenv_cli/utils/__init__.py @@ -0,0 +1,7 @@ +# 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. + +"""OpenEnv CLI utility modules.""" diff --git a/src/openenv_cli/utils/env_loader.py b/src/openenv_cli/utils/env_loader.py new file mode 100644 index 00000000..11b1ef4c --- /dev/null +++ b/src/openenv_cli/utils/env_loader.py @@ -0,0 +1,91 @@ +# 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. + +"""Environment loader utilities.""" + +from pathlib import Path +from typing import Dict, Any, Optional + + +def validate_environment(env_name: str) -> Path: + """ + Validate that environment exists and return its path. + + Args: + env_name: Name of the environment to validate. + + Returns: + Path to the environment directory. + + Raises: + FileNotFoundError: If environment does not exist. + """ + env_path = Path("src/envs") / env_name + if not env_path.exists(): + raise FileNotFoundError( + f"Environment '{env_name}' not found under src/envs. " + f"Expected path: {env_path.absolute()}" + ) + if not env_path.is_dir(): + raise FileNotFoundError( + f"Environment '{env_name}' is not a directory. " + f"Path: {env_path.absolute()}" + ) + return env_path + + +def load_env_metadata(env_name: str) -> Dict[str, Any]: + """ + Load environment metadata. + + Args: + env_name: Name of the environment. + + Returns: + Dictionary with environment metadata. + """ + env_path = validate_environment(env_name) + + metadata: Dict[str, Any] = { + "name": env_name, + "path": str(env_path), + } + + # Load README if it exists + readme_path = env_path / "README.md" + if readme_path.exists(): + readme_content = readme_path.read_text() + metadata["readme"] = readme_content + + # Try to extract title from README + lines = readme_content.split("\n") + for line in lines: + if line.startswith("# "): + metadata["title"] = line[2:].strip() + break + + # Check for server directory + server_path = env_path / "server" + if server_path.exists(): + metadata["has_server"] = True + + # Check for Dockerfile + dockerfile_path = server_path / "Dockerfile" + if dockerfile_path.exists(): + metadata["has_dockerfile"] = True + metadata["dockerfile_path"] = str(dockerfile_path) + + # Check for models.py + models_path = env_path / "models.py" + if models_path.exists(): + metadata["has_models"] = True + + # Check for client.py + client_path = env_path / "client.py" + if client_path.exists(): + metadata["has_client"] = True + + return metadata diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..81c2b022 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,7 @@ +# 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.""" diff --git a/tests/test_cli/__init__.py b/tests/test_cli/__init__.py new file mode 100644 index 00000000..9b4cc691 --- /dev/null +++ b/tests/test_cli/__init__.py @@ -0,0 +1,7 @@ +# 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 CLI.""" diff --git a/tests/test_cli/test_auth.py b/tests/test_cli/test_auth.py new file mode 100644 index 00000000..8ca2157a --- /dev/null +++ b/tests/test_cli/test_auth.py @@ -0,0 +1,156 @@ +# 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 authentication module.""" + +import os +from unittest.mock import Mock, patch, MagicMock + +import pytest + +from openenv_cli.core.auth import check_authentication, ensure_authenticated, login_interactive + + +@pytest.fixture +def mock_hf_api(): + """Mock HfApi for testing.""" + return Mock() + + +@pytest.fixture +def mock_get_token(): + """Mock get_token for testing.""" + return Mock() + + +class TestCheckAuthentication: + """Tests for check_authentication function.""" + + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_check_authentication_with_valid_token(self, mock_get_token, mock_api_class): + """Test check_authentication with valid token.""" + mock_get_token.return_value = "test_token" + mock_api = Mock() + mock_api.whoami.return_value = {"name": "test_user"} + mock_api_class.return_value = mock_api + + result = check_authentication() + + assert result == "test_user" + mock_api.whoami.assert_called_once() + + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_check_authentication_no_token(self, mock_get_token, mock_api_class): + """Test check_authentication when no token exists.""" + mock_get_token.return_value = None + mock_api = Mock() + mock_api_class.return_value = mock_api + + result = check_authentication() + + assert result is None + + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_check_authentication_invalid_token(self, mock_get_token, mock_api_class): + """Test check_authentication with invalid token.""" + mock_get_token.return_value = "invalid_token" + mock_api = Mock() + mock_api.whoami.side_effect = Exception("Invalid token") + mock_api_class.return_value = mock_api + + result = check_authentication() + + assert result is None + + @patch("openenv_cli.core.auth.HfApi") + @patch.dict(os.environ, {"HF_TOKEN": "env_token"}) + def test_check_authentication_env_token(self, mock_api_class): + """Test check_authentication using HF_TOKEN environment variable.""" + mock_api = Mock() + mock_api.whoami.return_value = {"name": "env_user"} + mock_api_class.return_value = mock_api + + result = check_authentication() + + assert result == "env_user" + mock_api_class.assert_called_once_with(token="env_token") + mock_api.whoami.assert_called_once() + + +class TestEnsureAuthenticated: + """Tests for ensure_authenticated function.""" + + @patch("openenv_cli.core.auth.check_authentication") + @patch("openenv_cli.core.auth.get_token") + def test_ensure_authenticated_already_authenticated(self, mock_get_token, mock_check): + """Test ensure_authenticated when already authenticated.""" + mock_check.return_value = "test_user" + mock_get_token.return_value = "test_token" + + username, token = ensure_authenticated() + + assert username == "test_user" + assert token == "test_token" + mock_check.assert_called_once() + + @patch("openenv_cli.core.auth.check_authentication") + @patch("openenv_cli.core.auth.login_interactive") + @patch("openenv_cli.core.auth.get_token") + def test_ensure_authenticated_needs_login(self, mock_get_token, mock_login, mock_check): + """Test ensure_authenticated when login is needed.""" + mock_check.return_value = None + mock_login.return_value = "new_user" + mock_get_token.return_value = "new_token" + + username, token = ensure_authenticated() + + assert username == "new_user" + assert token == "new_token" + mock_login.assert_called_once() + + @patch.dict(os.environ, {"HF_TOKEN": "env_token"}) + @patch("openenv_cli.core.auth.HfApi") + def test_ensure_authenticated_env_token(self, mock_api_class): + """Test ensure_authenticated using HF_TOKEN environment variable.""" + mock_api = Mock() + mock_api.whoami.return_value = {"name": "env_user"} + mock_api_class.return_value = mock_api + + username, token = ensure_authenticated() + + assert username == "env_user" + assert token == "env_token" + + +class TestLoginInteractive: + """Tests for login_interactive function.""" + + @patch("openenv_cli.core.auth.login") + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_login_interactive_success(self, mock_get_token, mock_api_class, mock_login): + """Test successful interactive login.""" + mock_login.return_value = None # login doesn't return anything + mock_api = Mock() + mock_api.whoami.return_value = {"name": "logged_in_user"} + mock_api_class.return_value = mock_api + mock_get_token.return_value = "new_token" + + username = login_interactive() + + assert username == "logged_in_user" + mock_login.assert_called_once() + + @patch("openenv_cli.core.auth.login") + def test_login_interactive_failure(self, mock_login): + """Test interactive login failure.""" + mock_login.side_effect = Exception("Login failed") + + with pytest.raises(Exception, match="Login failed"): + login_interactive() diff --git a/tests/test_cli/test_builder.py b/tests/test_cli/test_builder.py new file mode 100644 index 00000000..2d22dd46 --- /dev/null +++ b/tests/test_cli/test_builder.py @@ -0,0 +1,251 @@ +# 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 builder module.""" + +import os +from pathlib import Path +from unittest.mock import patch, mock_open + +import pytest + +from openenv_cli.core.builder import ( + prepare_staging_directory, + copy_environment_files, + prepare_dockerfile, + prepare_readme, +) + + +@pytest.fixture +def test_env_path(tmp_path): + """Create a temporary test environment.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + server_dir = env_dir / "server" + server_dir.mkdir(parents=True) + + # Create basic environment files + (env_dir / "__init__.py").write_text("# Test env") + (env_dir / "models.py").write_text("# Test models") + (env_dir / "client.py").write_text("# Test client") + (env_dir / "README.md").write_text("# Test Environment\n\nTest description") + + # Create Dockerfile + (server_dir / "Dockerfile").write_text( + "ARG BASE_IMAGE=openenv-base:latest\n" + "FROM ${BASE_IMAGE}\n" + "COPY src/core/ /app/src/core/\n" + "COPY src/envs/test_env/ /app/src/envs/test_env/\n" + "CMD [\"uvicorn\", \"envs.test_env.server.app:app\", \"--host\", \"0.0.0.0\", \"--port\", \"8000\"]" + ) + + # Create app.py + (server_dir / "app.py").write_text("# Test app") + + return tmp_path + + +@pytest.fixture +def staging_dir(tmp_path): + """Create staging directory for tests.""" + staging = tmp_path / "staging" + staging.mkdir() + return staging + + +@pytest.fixture +def repo_root(tmp_path, test_env_path): + """Create a mock repo root structure.""" + # Create core directory (test_env_path already created src/envs) + core_dir = tmp_path / "src" / "core" + core_dir.mkdir(parents=True, exist_ok=True) + if not (core_dir / "__init__.py").exists(): + (core_dir / "__init__.py").write_text("# Core") + + # Create envs directory structure (may already exist from test_env_path) + envs_dir = tmp_path / "src" / "envs" + envs_dir.mkdir(parents=True, exist_ok=True) + + return tmp_path + + +class TestPrepareStagingDirectory: + """Tests for prepare_staging_directory function.""" + + @patch("openenv_cli.core.builder.Path") + def test_prepare_staging_directory_creates_structure(self, mock_path, staging_dir): + """Test that staging directory structure is created.""" + # This will be tested through integration with copy functions + pass + + +class TestCopyEnvironmentFiles: + """Tests for copy_environment_files function.""" + + def test_copy_environment_files_copies_files(self, repo_root, tmp_path, monkeypatch): + """Test that environment files are copied correctly.""" + from openenv_cli.core.builder import copy_environment_files, prepare_staging_directory + + # Set up test environment (test_env_path already created the directory) + test_env_dir = repo_root / "src" / "envs" / "test_env" + test_env_dir.mkdir(parents=True, exist_ok=True) + (test_env_dir / "models.py").write_text("# Test") + + # Create core directory (repo_root already created it) + core_dir = repo_root / "src" / "core" + core_dir.mkdir(parents=True, exist_ok=True) + if not (core_dir / "__init__.py").exists(): + (core_dir / "__init__.py").write_text("# Core") + + # Prepare staging directory (creates the structure) + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + # Set working directory + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + copy_environment_files("test_env", staging_dir) + + # Verify files were copied + assert (staging_dir / "src" / "core" / "__init__.py").exists() + assert (staging_dir / "src" / "envs" / "test_env" / "models.py").exists() + finally: + os.chdir(old_cwd) + + +class TestPrepareDockerfile: + """Tests for prepare_dockerfile function.""" + + def test_prepare_dockerfile_uses_existing_dockerfile(self, repo_root, tmp_path, monkeypatch): + """Test that existing Dockerfile is used and modified.""" + from openenv_cli.core.builder import prepare_dockerfile, prepare_staging_directory + + # Create test environment with Dockerfile (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" / "server" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "Dockerfile").write_text( + "ARG BASE_IMAGE=openenv-base:latest\n" + "FROM ${BASE_IMAGE}\n" + "COPY src/core/ /app/src/core/\n" + "CMD [\"uvicorn\", \"envs.test_env.server.app:app\", \"--host\", \"0.0.0.0\", \"--port\", \"8000\"]" + ) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + base_image = "ghcr.io/meta-pytorch/openenv-base:latest" + prepare_dockerfile("test_env", staging_dir, base_image) + + # Check Dockerfile was created + dockerfile_path = staging_dir / "Dockerfile" + assert dockerfile_path.exists() + + # Check base image was replaced + content = dockerfile_path.read_text() + assert base_image in content + assert "ENV ENABLE_WEB_INTERFACE=true" in content or "ENABLE_WEB_INTERFACE=true" in content + finally: + os.chdir(old_cwd) + + def test_prepare_dockerfile_creates_default_if_missing(self, repo_root, tmp_path, monkeypatch): + """Test that default Dockerfile is created if env doesn't have one.""" + from openenv_cli.core.builder import prepare_dockerfile, prepare_staging_directory + + # Create test environment without Dockerfile (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "models.py").write_text("# Test") + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + base_image = "ghcr.io/meta-pytorch/openenv-base:latest" + prepare_dockerfile("test_env", staging_dir, base_image) + + # Check Dockerfile was created + dockerfile_path = staging_dir / "Dockerfile" + assert dockerfile_path.exists() + + content = dockerfile_path.read_text() + assert base_image in content + assert "test_env" in content + finally: + os.chdir(old_cwd) + + +class TestPrepareReadme: + """Tests for prepare_readme function.""" + + def test_prepare_readme_adds_front_matter(self, repo_root, tmp_path, monkeypatch): + """Test that README gets HF front matter added.""" + from openenv_cli.core.builder import prepare_readme, prepare_staging_directory + + # Create test environment with README (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "README.md").write_text( + "# Test Environment\n\nThis is a test environment." + ) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Check for HF front matter + assert "---" in content + assert "sdk: docker" in content + assert "Test Environment" in content + finally: + os.chdir(old_cwd) + + def test_prepare_readme_handles_existing_front_matter(self, repo_root, tmp_path, monkeypatch): + """Test that README with existing front matter is handled correctly.""" + from openenv_cli.core.builder import prepare_readme, prepare_staging_directory + + # Create test environment with README that has front matter (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "README.md").write_text( + "---\n" + "title: Test\n" + "emoji: 🎮\n" + "---\n" + "# Test Environment\n\nThis is a test environment." + ) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should still have front matter + assert "---" in content + assert "Test Environment" in content + finally: + os.chdir(old_cwd) diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py new file mode 100644 index 00000000..cf3f811b --- /dev/null +++ b/tests/test_cli/test_push_command.py @@ -0,0 +1,235 @@ +# 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. + +"""End-to-end tests for push command.""" + +import os +from pathlib import Path +from unittest.mock import Mock, patch, MagicMock + +import pytest + +from openenv_cli.commands.push import push_environment + + +@pytest.fixture +def mock_environment(tmp_path, monkeypatch): + """Create a mock environment structure.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + server_dir = env_dir / "server" + server_dir.mkdir(parents=True) + + # Create basic files + (env_dir / "__init__.py").write_text("# Test") + (env_dir / "README.md").write_text("# Test Environment") + (server_dir / "Dockerfile").write_text("FROM test:latest") + (server_dir / "app.py").write_text("# App") + + # Create core directory + core_dir = tmp_path / "src" / "core" + core_dir.mkdir(parents=True) + (core_dir / "__init__.py").write_text("# Core") + + # Monkey patch Path to return our test paths + original_path = Path + monkeypatch.chdir(tmp_path) + + return tmp_path + + +class TestPushEnvironment: + """Tests for push_environment function.""" + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_full_workflow( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test full push workflow.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "test_user/test_env" + mock_space_exists.return_value = False + mock_prepare_staging.return_value = Path("staging") + + # Run push + push_environment("test_env", namespace=None, private=False) + + # Verify calls + mock_validate.assert_called_once_with("test_env") + mock_ensure_auth.assert_called_once() + mock_get_repo_id.assert_called_once_with("test_env", namespace=None) + mock_space_exists.assert_called_once_with(mock_api, "test_user/test_env") + mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) + mock_prepare_staging.assert_called_once() + mock_copy_files.assert_called_once() + mock_prepare_dockerfile.assert_called_once() + mock_prepare_readme.assert_called_once() + mock_upload.assert_called_once() + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_space_exists( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push when space already exists.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "test_user/test_env" + mock_space_exists.return_value = True # Space already exists + mock_prepare_staging.return_value = Path("staging") + + # Run push + push_environment("test_env") + + # Verify space was not created + mock_create_space.assert_not_called() + + @patch("openenv_cli.commands.push.validate_environment") + def test_push_environment_invalid_env(self, mock_validate, mock_environment): + """Test push with invalid environment name.""" + mock_validate.side_effect = FileNotFoundError("Environment not found") + + with pytest.raises(FileNotFoundError, match="Environment not found"): + push_environment("invalid_env") + + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + def test_push_environment_auth_failure(self, mock_ensure_auth, mock_validate, mock_environment): + """Test push when authentication fails.""" + mock_validate.return_value = Path("src/envs/test_env") + mock_ensure_auth.side_effect = Exception("Authentication failed") + + with pytest.raises(Exception, match="Authentication failed"): + push_environment("test_env") + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_with_namespace( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push with explicit namespace.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "my-org/test_env" + mock_space_exists.return_value = False + mock_prepare_staging.return_value = Path("staging") + + # Run push with namespace + push_environment("test_env", namespace="my-org") + + # Verify namespace was used + mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org") + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_private( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push with private space.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "test_user/test_env" + mock_space_exists.return_value = False + mock_prepare_staging.return_value = Path("staging") + + # Run push with private flag + push_environment("test_env", private=True) + + # Verify private space was created + mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=True) diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py new file mode 100644 index 00000000..0c2556cc --- /dev/null +++ b/tests/test_cli/test_space.py @@ -0,0 +1,135 @@ +# 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 space management module.""" + +from unittest.mock import Mock, patch + +import pytest + +from openenv_cli.core.space import space_exists, create_space, get_space_repo_id + + +@pytest.fixture +def mock_hf_api(): + """Mock HfApi for testing.""" + return Mock() + + +class TestSpaceExists: + """Tests for space_exists function.""" + + def test_space_exists_true(self, mock_hf_api): + """Test space_exists returns True when space exists.""" + mock_hf_api.repo_exists.return_value = True + + result = space_exists(mock_hf_api, "test_user/my-env") + + assert result is True + mock_hf_api.repo_exists.assert_called_once_with( + repo_id="test_user/my-env", + repo_type="space" + ) + + def test_space_exists_false(self, mock_hf_api): + """Test space_exists returns False when space doesn't exist.""" + mock_hf_api.repo_exists.return_value = False + + result = space_exists(mock_hf_api, "test_user/my-env") + + assert result is False + mock_hf_api.repo_exists.assert_called_once_with( + repo_id="test_user/my-env", + repo_type="space" + ) + + def test_space_exists_error(self, mock_hf_api): + """Test space_exists handles errors gracefully.""" + mock_hf_api.repo_exists.side_effect = Exception("API error") + + # Should return False on error (not raise) + result = space_exists(mock_hf_api, "test_user/my-env") + assert result is False + + +class TestCreateSpace: + """Tests for create_space function.""" + + def test_create_space_public(self, mock_hf_api): + """Test creating a public space.""" + mock_hf_api.create_repo.return_value = None + + create_space(mock_hf_api, "test_user/my-env", private=False) + + mock_hf_api.create_repo.assert_called_once_with( + repo_id="test_user/my-env", + repo_type="space", + space_sdk="docker", + private=False, + exist_ok=True + ) + + def test_create_space_private(self, mock_hf_api): + """Test creating a private space.""" + mock_hf_api.create_repo.return_value = None + + create_space(mock_hf_api, "test_user/my-env", private=True) + + mock_hf_api.create_repo.assert_called_once_with( + repo_id="test_user/my-env", + repo_type="space", + space_sdk="docker", + private=True, + exist_ok=True + ) + + def test_create_space_already_exists(self, mock_hf_api): + """Test creating a space that already exists.""" + from huggingface_hub.utils import RepositoryNotFoundError + + # RepositoryNotFoundError doesn't exist in all versions, use generic Exception + mock_hf_api.create_repo.side_effect = Exception("Repository already exists") + + # Should not raise, just silently continue + # This tests the function handles the case gracefully + try: + create_space(mock_hf_api, "test_user/my-env", private=False) + except Exception: + # If it raises, that's okay - we just test the call was made + pass + + mock_hf_api.create_repo.assert_called_once() + + +class TestGetSpaceRepoId: + """Tests for get_space_repo_id function.""" + + @patch("openenv_cli.core.space.ensure_authenticated") + def test_get_space_repo_id_with_namespace(self, mock_auth): + """Test get_space_repo_id with explicit namespace.""" + mock_auth.return_value = ("user", "token") + + result = get_space_repo_id("my-env", namespace="my-org") + + assert result == "my-org/my-env" + + @patch("openenv_cli.core.space.ensure_authenticated") + def test_get_space_repo_id_no_namespace(self, mock_auth): + """Test get_space_repo_id without namespace uses authenticated user.""" + mock_auth.return_value = ("test_user", "token") + + result = get_space_repo_id("my-env") + + assert result == "test_user/my-env" + + @patch("openenv_cli.core.space.ensure_authenticated") + def test_get_space_repo_id_env_name_with_underscore(self, mock_auth): + """Test get_space_repo_id handles env names with underscores.""" + mock_auth.return_value = ("test_user", "token") + + result = get_space_repo_id("my_env", namespace="my-org") + + assert result == "my-org/my_env" diff --git a/tests/test_cli/test_uploader.py b/tests/test_cli/test_uploader.py new file mode 100644 index 00000000..3d64b2e1 --- /dev/null +++ b/tests/test_cli/test_uploader.py @@ -0,0 +1,87 @@ +# 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 uploader module.""" + +from pathlib import Path +from unittest.mock import Mock, patch, MagicMock + +import pytest + +from openenv_cli.core.uploader import upload_to_space + + +@pytest.fixture +def mock_hf_api(): + """Mock HfApi for testing.""" + return Mock() + + +@pytest.fixture +def staging_dir(tmp_path): + """Create a staging directory with test files.""" + staging = tmp_path / "staging" + staging.mkdir() + + # Create some test files + (staging / "Dockerfile").write_text("FROM test:latest") + (staging / "README.md").write_text("# Test") + (staging / "src" / "core").mkdir(parents=True) + (staging / "src" / "core" / "__init__.py").write_text("# Core") + + return staging + + +class TestUploadToSpace: + """Tests for upload_to_space function.""" + + @patch("openenv_cli.core.uploader.upload_folder") + def test_upload_to_space_uses_upload_folder(self, mock_upload, mock_hf_api, staging_dir): + """Test that upload_to_space uses upload_folder for bulk upload.""" + mock_upload.return_value = None + + upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + + mock_upload.assert_called_once() + # Check that the call was made with correct parameters + call_args = mock_upload.call_args + assert call_args[1]["repo_id"] == "test_user/my-env" + assert call_args[1]["folder_path"] == str(staging_dir) + assert call_args[1]["repo_type"] == "space" + assert call_args[1]["token"] == "test_token" + + @patch("openenv_cli.core.uploader.upload_folder") + def test_upload_to_space_handles_errors(self, mock_upload, mock_hf_api, staging_dir): + """Test that upload_to_space handles upload errors.""" + mock_upload.side_effect = Exception("Upload failed") + + with pytest.raises(Exception, match="Upload failed"): + upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + + @patch("openenv_cli.core.uploader.upload_folder") + def test_upload_to_space_empty_directory(self, mock_upload, mock_hf_api, tmp_path): + """Test uploading an empty directory.""" + empty_dir = tmp_path / "empty" + empty_dir.mkdir() + + mock_upload.return_value = None + + upload_to_space(mock_hf_api, "test_user/my-env", empty_dir, "test_token") + + mock_upload.assert_called_once() + + @patch("openenv_cli.core.uploader.upload_folder") + def test_upload_to_space_nested_files(self, mock_upload, mock_hf_api, staging_dir): + """Test uploading directory with nested files.""" + # Create nested structure + (staging_dir / "src" / "envs" / "test_env").mkdir(parents=True) + (staging_dir / "src" / "envs" / "test_env" / "models.py").write_text("# Models") + + mock_upload.return_value = None + + upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + + mock_upload.assert_called_once() From 1a7a6019d40ee08e077dca857ef7ff52340b5f73 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 20:36:46 -0400 Subject: [PATCH 07/38] Update openspiel to reference BASE_IMAGE to support cli base-image override --- src/envs/openspiel_env/server/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/envs/openspiel_env/server/Dockerfile b/src/envs/openspiel_env/server/Dockerfile index 48ccff33..e1c65dee 100644 --- a/src/envs/openspiel_env/server/Dockerfile +++ b/src/envs/openspiel_env/server/Dockerfile @@ -7,8 +7,8 @@ # Use the pre-built OpenSpiel base image # Built from: docker build -t openspiel-base:latest -f src/envs/openspiel_env/server/Dockerfile.openspiel-base . # In GitHub Actions, this is overridden to use the GHCR base image -ARG OPENSPIEL_BASE_IMAGE=openspiel-base:latest -FROM ${OPENSPIEL_BASE_IMAGE} +ARG BASE_IMAGE=openspiel-base:latest +FROM ${BASE_IMAGE} # Copy OpenEnv core (base image already set WORKDIR=/app) WORKDIR /app From 7624f2483acb225b3e9d7ec9acbed206ea3dfcdc Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 20:38:17 -0400 Subject: [PATCH 08/38] Refactor push command to handle dry run more cleanly - Updated the `push_environment` function to conditionally upload to HuggingFace Spaces only if not in dry run mode. - Simplified the cleanup process for the staging directory, ensuring it is removed regardless of the upload status. --- src/openenv_cli/commands/push.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index 41ffa67e..b2abe420 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -79,16 +79,15 @@ def push_environment( if dry_run: print(f"Dry run: Files prepared in {staging_dir}") print(f"Would upload to: https://huggingface.co/spaces/{repo_id}") - return - # Upload to space - print(f"Uploading to space: {repo_id}") - upload_to_space(api, repo_id, staging_dir, token) - - print(f"✅ Successfully pushed {env_name} to https://huggingface.co/spaces/{repo_id}") + # Upload to space (skip if dry run) + if not dry_run: + print(f"Uploading to space: {repo_id}") + upload_to_space(api, repo_id, staging_dir, token) + print(f"✅ Successfully pushed {env_name} to https://huggingface.co/spaces/{repo_id}") finally: - # Cleanup staging directory after upload (or if upload fails) - if not dry_run and staging_dir.exists(): + # Cleanup staging directory after upload or dry run + if staging_dir.exists(): import shutil shutil.rmtree(staging_dir) From 4904e6731093a805ee2f5746cd3e71b98a6ee41b Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 20:52:34 -0400 Subject: [PATCH 09/38] Enhance README preparation logic in builder module - Added functionality to check for existing HuggingFace front matter in README files and use it as-is if present. - Implemented random generation of front matter with approved emojis and colors when no front matter exists. - Updated tests to verify correct handling of existing front matter and generation of new front matter. --- src/openenv_cli/core/builder.py | 83 +++++++++++++++++++++----- tests/test_cli/test_builder.py | 101 ++++++++++++++++++++++++++++++-- 2 files changed, 162 insertions(+), 22 deletions(-) diff --git a/src/openenv_cli/core/builder.py b/src/openenv_cli/core/builder.py index d3339b41..3fcd2cf7 100644 --- a/src/openenv_cli/core/builder.py +++ b/src/openenv_cli/core/builder.py @@ -7,6 +7,7 @@ """Builder module for preparing environments for deployment.""" import os +import random import re import shutil from pathlib import Path @@ -137,24 +138,74 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: """ Prepare README.md with HuggingFace front matter. + If the environment README already has HuggingFace front matter (starts and ends with `---`), + use it as-is. Otherwise, generate front matter with random emoji and colors. + Args: env_name: Name of the environment. staging_dir: Staging directory path. """ - # Capitalize first letter of environment name + # Check both src/envs/${ENV_NAME}/README.md and src/envs/${ENV_NAME}/server/README.md + readme_paths = [ + Path("src/envs") / env_name / "README.md", + Path("src/envs") / env_name / "server" / "README.md", + ] + + # Check if any README has HuggingFace front matter + existing_readme_content = None + for readme_path in readme_paths: + if readme_path.exists(): + content = readme_path.read_text() + # Check if it has front matter (starts with --- and has closing ---) + if content.startswith("---"): + lines = content.split("\n") + # Look for closing --- (must be at the top, within first 100 lines) + for i in range(1, min(100, len(lines))): + if lines[i].strip() == "---": + # Found front matter, use this README as-is + existing_readme_content = content + break + if existing_readme_content: + break + + # If we found an existing README with front matter, use it as-is + if existing_readme_content: + readme_path = staging_dir / "README.md" + readme_path.write_text(existing_readme_content) + return + + # Otherwise, generate front matter with random emoji and colors env_title = env_name[0].upper() + env_name[1:] if env_name else env_name - # Set environment-specific colors and emoji - emoji_map = { - "atari_env": ("đŸ•šī¸", "red", "yellow"), - "coding_env": ("đŸ’ģ", "blue", "gray"), - "openspiel_env": ("🎮", "purple", "indigo"), - "echo_env": ("🔊", "blue", "gray"), - "chat_env": ("đŸ’Ŧ", "blue", "green"), - "textarena_env": ("📜", "green", "blue"), - } + # Approved emojis from Spaces Configuration Reference + approved_emojis = [ + "🎮", "🚀", "đŸ’ģ", "đŸ”Ŧ", "đŸ§Ē", "đŸŽ¯", "🎨", "📊", "🤖", "🌟", + "⚡", "🔧", "📱", "💡", "🎲", "đŸŽĩ", "🎸", "🎭", "đŸŽŦ", "🏆", + "đŸ”Ĩ", "💎", "🌈", "🎁", "🎈", "🎊", "🎉", "đŸĻ„", "đŸŗ", "🐙", + "đŸĻ‹", "🐝", "🐞", "đŸĻ…", "đŸĻ‰", "đŸĻ‡", "🐉", "đŸĻ–", "đŸĻ•", "đŸĸ", + "🐍", "đŸĻŽ", "🐊", "🐋", "đŸĻˆ", "đŸŦ", "🐠", "🐟", "🐡", "đŸĻ‘", + "đŸĻ", "đŸĻž", "đŸĻ€", "đŸē", "🐗", "🐴", "🐛", "🐌", "🐜", "đŸĻŸ", + "đŸĻ—", "đŸ•ˇī¸", "đŸĻ‚", "🐅", "🐆", "đŸĻ“", "đŸĻ", "đŸĻ§", "🐘", "đŸĻ›", + "đŸĻ", "đŸĒ", "đŸĢ", "đŸĻ’", "đŸĻ˜", "đŸĻŦ", "🐃", "🐂", "🐄", "🐎", + "🐖", "🐏", "🐑", "đŸĻ™", "🐐", "đŸĻŒ", "🐕", "🐩", "đŸĻŽ", "🐈", + "đŸĒļ", "🐓", "đŸĻƒ", "đŸĻš", "đŸĻœ", "đŸĻĸ", "đŸĻŠ", "đŸ•Šī¸", "🐇", "đŸŋī¸", + "đŸĻĢ", "đŸĻ”", "đŸĻ", "đŸĻ¨", "đŸĻĄ", "đŸĻĻ", "đŸĻĨ", "🐁", "🐀", "🐾", + "🐲", "đŸŒĩ", "🎄", "🌲", "đŸŒŗ", "🌴", "🌱", "đŸŒŋ", "â˜˜ī¸", "🍀", + "🎍", "đŸĒ´", "🎋", "🍃", "🍂", "🍁", "🍄", "🐚", "đŸĒ¨", "🌾", + "💐", "🌷", "🌹", "đŸĨ€", "đŸŒē", "🌸", "đŸŒŧ", "đŸŒģ", "🌞", "🌝", + "🌛", "🌜", "🌕", "🌖", "🌗", "🌘", "🌑", "🌒", "🌓", "🌔", + "🌙", "🌚", "🌏", "đŸĒ", "đŸ’Ģ", "⭐", "✨", "â˜„ī¸", "đŸ’Ĩ", "â˜€ī¸", + "⛅", "â˜ī¸", "â›ˆī¸", "đŸŒ¤ī¸", "đŸŒĻī¸", "đŸŒ§ī¸", "đŸŒŠī¸", "â„ī¸", "â˜ƒī¸", "⛄", + "đŸŒŦī¸", "💨", "💧", "đŸ’Ļ", "☔", "â˜‚ī¸", "🌊", "đŸŒĢī¸", + ] + + # Approved colors from Spaces Configuration Reference + approved_colors = ["red", "yellow", "green", "blue", "indigo", "purple", "pink", "gray"] - emoji, color_from, color_to = emoji_map.get(env_name, ("đŸŗ", "blue", "green")) + # Randomly select emoji and colors + emoji = random.choice(approved_emojis) + color_from = random.choice(approved_colors) + color_to = random.choice(approved_colors) # Create README with front matter readme_content = f"""--- @@ -197,14 +248,13 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: The environment provides a health check endpoint at `/health`. """ - # Try to append content from original README if it exists + # Try to append content from original README if it exists (without front matter) original_readme = Path("src/envs") / env_name / "README.md" if original_readme.exists(): original_content = original_readme.read_text() # Skip front matter if present if original_content.startswith("---"): - # Find closing --- lines = original_content.split("\n") closing_idx = None for i in range(1, len(lines)): @@ -214,10 +264,11 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: if closing_idx: # Skip front matter - original_content = "\n".join(lines[closing_idx + 1:]) + original_content = "\n".join(lines[closing_idx + 1:]).strip() - # Append original content - readme_content += "\n" + original_content + # Append original content if there's any + if original_content: + readme_content += "\n\n" + original_content readme_path = staging_dir / "README.md" readme_path.write_text(readme_content) diff --git a/tests/test_cli/test_builder.py b/tests/test_cli/test_builder.py index 2d22dd46..3f3ed0de 100644 --- a/tests/test_cli/test_builder.py +++ b/tests/test_cli/test_builder.py @@ -217,19 +217,23 @@ def test_prepare_readme_adds_front_matter(self, repo_root, tmp_path, monkeypatch os.chdir(old_cwd) def test_prepare_readme_handles_existing_front_matter(self, repo_root, tmp_path, monkeypatch): - """Test that README with existing front matter is handled correctly.""" + """Test that README with existing front matter is used as-is.""" from openenv_cli.core.builder import prepare_readme, prepare_staging_directory # Create test environment with README that has front matter (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" env_dir.mkdir(parents=True, exist_ok=True) - (env_dir / "README.md").write_text( + original_content = ( "---\n" - "title: Test\n" + "title: Test Environment\n" "emoji: 🎮\n" + "colorFrom: red\n" + "colorTo: blue\n" + "sdk: docker\n" "---\n" "# Test Environment\n\nThis is a test environment." ) + (env_dir / "README.md").write_text(original_content) # Prepare staging directory staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) @@ -244,8 +248,93 @@ def test_prepare_readme_handles_existing_front_matter(self, repo_root, tmp_path, assert readme_path.exists() content = readme_path.read_text() - # Should still have front matter - assert "---" in content - assert "Test Environment" in content + # Should use original content as-is + assert content == original_content + assert "emoji: 🎮" in content + assert "colorFrom: red" in content + assert "colorTo: blue" in content + finally: + os.chdir(old_cwd) + + def test_prepare_readme_generates_front_matter_when_missing(self, repo_root, tmp_path, monkeypatch): + """Test that README without front matter gets generated front matter.""" + from openenv_cli.core.builder import prepare_readme, prepare_staging_directory + + # Create test environment with README without front matter (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "README.md").write_text( + "# Test Environment\n\nThis is a test environment." + ) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should have generated front matter + assert content.startswith("---") + assert "sdk: docker" in content + assert "emoji:" in content + assert "colorFrom:" in content + assert "colorTo:" in content + # Should include original content + assert "# Test Environment" in content + assert "This is a test environment." in content + finally: + os.chdir(old_cwd) + + def test_prepare_readme_uses_server_readme_front_matter(self, repo_root, tmp_path, monkeypatch): + """Test that server/README.md front matter is used when present.""" + from openenv_cli.core.builder import prepare_readme, prepare_staging_directory + + # Create test environment (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + server_dir = env_dir / "server" + server_dir.mkdir(parents=True, exist_ok=True) + + # Main README without front matter + (env_dir / "README.md").write_text("# Test Environment\n\nDescription.") + + # Server README with front matter + server_readme_content = ( + "---\n" + "title: Server Test\n" + "emoji: 🚀\n" + "colorFrom: purple\n" + "colorTo: indigo\n" + "sdk: docker\n" + "---\n" + "# Server README\n\nServer-specific content." + ) + (server_dir / "README.md").write_text(server_readme_content) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should use server README content as-is + assert content == server_readme_content + assert "emoji: 🚀" in content + assert "colorFrom: purple" in content + assert "colorTo: indigo" in content finally: os.chdir(old_cwd) From a326c9dc6498473a56c6acbf816089ddcc801c8b Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 20:55:05 -0400 Subject: [PATCH 10/38] Update color schemes to HF approved colors --- src/envs/atari_env/README.md | 4 ++-- src/envs/chat_env/README.md | 4 ++-- src/envs/coding_env/README.md | 4 ++-- src/envs/echo_env/README.md | 4 ++-- src/envs/openspiel_env/README.md | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/envs/atari_env/README.md b/src/envs/atari_env/README.md index d942f264..cc819e2a 100644 --- a/src/envs/atari_env/README.md +++ b/src/envs/atari_env/README.md @@ -1,8 +1,8 @@ --- title: Atari Environment Server emoji: đŸ•šī¸ -colorFrom: '#FF6200' -colorTo: '#D4151B' +colorFrom: yellow +colorTo: red sdk: docker pinned: false app_port: 8000 diff --git a/src/envs/chat_env/README.md b/src/envs/chat_env/README.md index 6cd11e27..47e340d8 100644 --- a/src/envs/chat_env/README.md +++ b/src/envs/chat_env/README.md @@ -1,8 +1,8 @@ --- title: Chat Environment Server emoji: đŸ’Ŧ -colorFrom: '#0084FF' -colorTo: '#25D366' +colorFrom: gray +colorTo: blue sdk: docker pinned: false app_port: 8000 diff --git a/src/envs/coding_env/README.md b/src/envs/coding_env/README.md index ab500002..718084f6 100644 --- a/src/envs/coding_env/README.md +++ b/src/envs/coding_env/README.md @@ -1,8 +1,8 @@ --- title: Coding Environment Server emoji: đŸ’ģ -colorFrom: '#007ACC' -colorTo: '#1E1E1E' +colorFrom: gray +colorTo: indigo sdk: docker pinned: false app_port: 8000 diff --git a/src/envs/echo_env/README.md b/src/envs/echo_env/README.md index c4b7af37..79855421 100644 --- a/src/envs/echo_env/README.md +++ b/src/envs/echo_env/README.md @@ -1,8 +1,8 @@ --- title: Echo Environment Server emoji: 🔊 -colorFrom: '#00C9FF' -colorTo: '#1B2845' +colorFrom: blue +colorTo: yellow sdk: docker pinned: false app_port: 8000 diff --git a/src/envs/openspiel_env/README.md b/src/envs/openspiel_env/README.md index 85acbecc..abf62cdc 100644 --- a/src/envs/openspiel_env/README.md +++ b/src/envs/openspiel_env/README.md @@ -1,8 +1,8 @@ --- title: OpenSpiel Environment Server emoji: 🎮 -colorFrom: '#9146FF' -colorTo: '#00FFA3' +colorFrom: pink +colorTo: purple sdk: docker pinned: false app_port: 8000 From d9b4b141dd036a768c9a2ccc5d6dc70da195e3a9 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 21:02:52 -0400 Subject: [PATCH 11/38] Add space name support in push command and repository ID retrieval - Introduced `--space-name` argument in the push command to allow users to specify a custom name for the HuggingFace Space. - Updated `push_environment` function to accept `space_name` parameter and modified repository ID retrieval to use it if provided. - Enhanced tests to cover scenarios with custom space names and ensure correct functionality with both namespace and space name. --- src/openenv_cli/__main__.py | 6 ++ src/openenv_cli/commands/push.py | 4 +- src/openenv_cli/core/space.py | 14 +++-- tests/test_cli/test_push_command.py | 90 ++++++++++++++++++++++++++++- tests/test_cli/test_space.py | 16 +++++ 5 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index 87d70a41..2924b856 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -35,6 +35,11 @@ def main(): help="HuggingFace namespace (organization or user). " "If not provided, uses authenticated user's username.", ) + push_parser.add_argument( + "--space-name", + help="Custom name for the HuggingFace Space. " + "If not provided, uses the environment name.", + ) push_parser.add_argument( "--private", action="store_true", @@ -58,6 +63,7 @@ def main(): push_environment( env_name=args.env_name, namespace=args.namespace, + space_name=args.space_name, private=args.private, base_image=args.base_image, dry_run=args.dry_run, diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index b2abe420..8a2702c8 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -26,6 +26,7 @@ def push_environment( env_name: str, namespace: Optional[str] = None, + space_name: Optional[str] = None, private: bool = False, base_image: Optional[str] = None, dry_run: bool = False, @@ -37,6 +38,7 @@ def push_environment( env_name: Name of the environment to push. namespace: Optional namespace (organization or user). If not provided, uses the authenticated user's username. + space_name: Optional custom space name. If not provided, uses env_name. private: Whether the space should be private (default: False). base_image: Base Docker image to use (default: ghcr.io/meta-pytorch/openenv-base:latest). dry_run: If True, prepare files but don't upload (default: False). @@ -48,7 +50,7 @@ def push_environment( username, token = ensure_authenticated() # Determine target space repo ID - repo_id = get_space_repo_id(env_name, namespace=namespace) + repo_id = get_space_repo_id(env_name, namespace=namespace, space_name=space_name) # Create HfApi instance api = HfApi(token=token) diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index bc8f99b5..a155f69d 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -57,21 +57,25 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: raise -def get_space_repo_id(env_name: str, namespace: Optional[str] = None) -> str: +def get_space_repo_id(env_name: str, namespace: Optional[str] = None, space_name: Optional[str] = None) -> str: """ Get the full repository ID for a space. Args: - env_name: Environment name (e.g., "echo_env"). + env_name: Environment name (e.g., "echo_env"). Used as space name if space_name is not provided. namespace: Optional namespace (organization or user). If not provided, uses the authenticated user's username. + space_name: Optional custom space name. If provided, used instead of env_name. Returns: - Repository ID in format 'namespace/env-name'. + Repository ID in format 'namespace/space-name'. """ + # Use space_name if provided, otherwise use env_name + repo_name = space_name if space_name else env_name + if namespace: - return f"{namespace}/{env_name}" + return f"{namespace}/{repo_name}" # Use authenticated user's username username, _ = ensure_authenticated() - return f"{username}/{env_name}" + return f"{username}/{repo_name}" diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index cf3f811b..b353b3ce 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -84,7 +84,7 @@ def test_push_environment_full_workflow( # Verify calls mock_validate.assert_called_once_with("test_env") mock_ensure_auth.assert_called_once() - mock_get_repo_id.assert_called_once_with("test_env", namespace=None) + mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name=None) mock_space_exists.assert_called_once_with(mock_api, "test_user/test_env") mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) mock_prepare_staging.assert_called_once() @@ -191,7 +191,7 @@ def test_push_environment_with_namespace( push_environment("test_env", namespace="my-org") # Verify namespace was used - mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org") + mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org", space_name=None) @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") @@ -233,3 +233,89 @@ def test_push_environment_private( # Verify private space was created mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=True) + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_with_space_name( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push with custom space name.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "test_user/custom-space" + mock_space_exists.return_value = False + mock_prepare_staging.return_value = Path("staging") + + # Run push with space_name + push_environment("test_env", space_name="custom-space") + + # Verify space_name was used + mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name="custom-space") + mock_space_exists.assert_called_once_with(mock_api, "test_user/custom-space") + mock_create_space.assert_called_once_with(mock_api, "test_user/custom-space", private=False) + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.space_exists") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.ensure_authenticated") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_with_namespace_and_space_name( + self, + mock_api_class, + mock_get_repo_id, + mock_ensure_auth, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_space_exists, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push with both namespace and space name.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_ensure_auth.return_value = ("test_user", "test_token") + mock_get_repo_id.return_value = "my-org/custom-space" + mock_space_exists.return_value = False + mock_prepare_staging.return_value = Path("staging") + + # Run push with namespace and space_name + push_environment("test_env", namespace="my-org", space_name="custom-space") + + # Verify both namespace and space_name were used + mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org", space_name="custom-space") + mock_space_exists.assert_called_once_with(mock_api, "my-org/custom-space") + mock_create_space.assert_called_once_with(mock_api, "my-org/custom-space", private=False) diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py index 0c2556cc..7fe542a7 100644 --- a/tests/test_cli/test_space.py +++ b/tests/test_cli/test_space.py @@ -133,3 +133,19 @@ def test_get_space_repo_id_env_name_with_underscore(self, mock_auth): result = get_space_repo_id("my_env", namespace="my-org") assert result == "my-org/my_env" + + @patch("openenv_cli.core.space.ensure_authenticated") + def test_get_space_repo_id_with_space_name(self, mock_auth): + """Test get_space_repo_id with custom space name.""" + mock_auth.return_value = ("test_user", "token") + + result = get_space_repo_id("my_env", space_name="custom-space") + + assert result == "test_user/custom-space" + + @patch("openenv_cli.core.space.ensure_authenticated") + def test_get_space_repo_id_with_namespace_and_space_name(self, mock_auth): + """Test get_space_repo_id with both namespace and space name.""" + result = get_space_repo_id("my_env", namespace="my-org", space_name="custom-space") + + assert result == "my-org/custom-space" From 3c1c6283c763368bceabbd9d9e6cc1ab8b088472 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 21:05:58 -0400 Subject: [PATCH 12/38] Update README documentation for `openenv push` command - Added `--space-name` option to specify a custom name for the HuggingFace Space. - Updated usage examples to demonstrate pushing with a custom space name and organization. - Clarified space provisioning logic in the README to reflect the new parameter handling. --- src/openenv_cli/README.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index b7cf31c2..cc37af41 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -35,6 +35,7 @@ openenv push [options] **Options:** - `--namespace `: HuggingFace namespace (organization or user). If not provided, uses authenticated user's username. +- `--space-name `: Custom name for the HuggingFace Space. If not provided, uses the environment name. - `--private`: Create a private space (default: public) - `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) - `--dry-run`: Prepare files but don't upload to HuggingFace @@ -48,6 +49,12 @@ openenv push echo_env # Push to a specific organization openenv push coding_env --namespace my-org +# Push with a custom space name +openenv push echo_env --space-name my-custom-space + +# Push to an organization with a custom space name +openenv push echo_env --namespace my-org --space-name my-custom-space + # Create a private space openenv push echo_env --private @@ -84,12 +91,12 @@ The `openenv push` command performs the following steps: 1. **Validation**: Checks that the environment exists in `src/envs//` 2. **Authentication**: Ensures you're authenticated with HuggingFace (prompts if needed) -3. **Space Provisioning**: Checks if a Docker Space exists, creates it if needed +3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Checks if a Docker Space exists, creates it if needed 4. **Build Process**: - Creates a staging directory - Copies core and environment files - Generates/modifies Dockerfile with web interface enabled - - Creates README with HuggingFace front matter + - Prepares README: If the environment's README already has HuggingFace front matter (starts and ends with `---`), uses it as-is. Otherwise, generates front matter with random emoji and colors from approved options 5. **Deployment**: Uploads all files to the HuggingFace Space 6. **Cleanup**: Removes staging directory after successful upload From 2b0741d94038c8839be4b51a3d259b75e9cf25d8 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sat, 1 Nov 2025 21:10:07 -0400 Subject: [PATCH 13/38] Remove outdated Phase 2 integration reference from README.md --- src/openenv_cli/README.md | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index cc37af41..2546d216 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -317,17 +317,6 @@ validate_parser.add_argument("env_name") # ... handle command ``` -### Integration with Phase 2 - -When HuggingFace native Environment primitives become available (Phase 2), the CLI will be updated to: - -- Use native Environment APIs instead of Docker Space orchestration -- Leverage HuggingFace's built-in build processes -- Simplify deployment workflow (less custom logic needed) -- Potentially deprecate or simplify the current `push` command - -The modular architecture ensures a smooth transition path. - ## Troubleshooting ### Authentication Issues From 94a94a254a89393874adc5f52ca39c72872a9d3d Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sun, 2 Nov 2025 07:39:53 +0100 Subject: [PATCH 14/38] locg with rich in main --- src/openenv_cli/__main__.py | 42 ++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index 2924b856..b7e03452 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -9,9 +9,16 @@ import argparse import sys +from rich.console import Console +from rich.traceback import install + from .commands.push import push_environment +console = Console() +install(show_locals=False) + + def main(): """Main entry point for OpenEnv CLI.""" parser = argparse.ArgumentParser( @@ -59,20 +66,35 @@ def main(): args = parser.parse_args() if args.command == "push": + if args.dry_run: + status_message = f"[bold yellow]Preparing dry run for '{args.env_name}'...[/bold yellow]" + else: + status_message = f"[bold cyan]Pushing environment '{args.env_name}'...[/bold cyan]" + try: - push_environment( - env_name=args.env_name, - namespace=args.namespace, - space_name=args.space_name, - private=args.private, - base_image=args.base_image, - dry_run=args.dry_run, - ) + with console.status(status_message): + push_environment( + env_name=args.env_name, + namespace=args.namespace, + space_name=args.space_name, + private=args.private, + base_image=args.base_image, + dry_run=args.dry_run, + ) + + if args.dry_run: + console.print( + f"[bold yellow]Dry run complete for '{args.env_name}'.[/bold yellow]" + ) + else: + console.print( + f"[bold green]Successfully pushed '{args.env_name}'.[/bold green]" + ) except Exception as e: - print(f"Error: {e}", file=sys.stderr) + console.print(f"[bold red]Error:[/bold red] {e}", highlight=False, soft_wrap=True) sys.exit(1) else: - parser.print_help() + console.print(parser.format_help()) sys.exit(1) From 690cfdfe2649db6b23aafb49b3931415f1b39bac Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sun, 2 Nov 2025 07:40:05 +0100 Subject: [PATCH 15/38] drop print statements in logic --- src/openenv_cli/commands/push.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index 8a2702c8..daa4bd51 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -6,7 +6,6 @@ """Push command for deploying environments to HuggingFace Spaces.""" -from pathlib import Path from typing import Optional from huggingface_hub import HfApi @@ -18,7 +17,7 @@ prepare_dockerfile, prepare_readme, ) -from ..core.space import space_exists, create_space, get_space_repo_id +from ..core.space import create_space, get_space_repo_id from ..core.uploader import upload_to_space from ..utils.env_loader import validate_environment @@ -47,7 +46,7 @@ def push_environment( validate_environment(env_name) # Authenticate with HuggingFace - username, token = ensure_authenticated() + _, token = ensure_authenticated() # Determine target space repo ID repo_id = get_space_repo_id(env_name, namespace=namespace, space_name=space_name) @@ -56,11 +55,7 @@ def push_environment( api = HfApi(token=token) # Check if space exists, create if needed - if not space_exists(api, repo_id): - create_space(api, repo_id, private=private) - else: - print(f"Space {repo_id} already exists, will update it") - + create_space(api, repo_id, private=private) # Set default base image if not provided if base_image is None: base_image = "ghcr.io/meta-pytorch/openenv-base:latest" @@ -78,15 +73,9 @@ def push_environment( # Prepare README prepare_readme(env_name, staging_dir) - if dry_run: - print(f"Dry run: Files prepared in {staging_dir}") - print(f"Would upload to: https://huggingface.co/spaces/{repo_id}") - # Upload to space (skip if dry run) if not dry_run: - print(f"Uploading to space: {repo_id}") upload_to_space(api, repo_id, staging_dir, token) - print(f"✅ Successfully pushed {env_name} to https://huggingface.co/spaces/{repo_id}") finally: # Cleanup staging directory after upload or dry run From 5e58273b1d70580b8a76a3f5f6f0cf37291aeaec Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Sun, 2 Nov 2025 07:40:14 +0100 Subject: [PATCH 16/38] add rich dependency --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 7b6fafe5..016d7025 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,8 @@ dependencies = [ "fastapi>=0.104.0", "uvicorn>=0.24.0", "smolagents>=1.22.0,<2", - "huggingface_hub>=0.20.0" + "huggingface_hub>=0.20.0", + "rich>=13.0.0", ] [tool.setuptools] From e873c362033dfa43722ad4cf74239c71bcda3465 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 12:18:54 -0500 Subject: [PATCH 17/38] Fix nits from Github Copilot --- src/openenv_cli/core/builder.py | 2 -- tests/test_cli/test_push_command.py | 4 +--- tests/test_cli/test_uploader.py | 3 +-- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/openenv_cli/core/builder.py b/src/openenv_cli/core/builder.py index 3fcd2cf7..9e3b1e8c 100644 --- a/src/openenv_cli/core/builder.py +++ b/src/openenv_cli/core/builder.py @@ -6,12 +6,10 @@ """Builder module for preparing environments for deployment.""" -import os import random import re import shutil from pathlib import Path -from typing import Optional def prepare_staging_directory(env_name: str, base_image: str, staging_root: str = "hf-staging") -> Path: diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index b353b3ce..fdf70283 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -6,9 +6,8 @@ """End-to-end tests for push command.""" -import os from pathlib import Path -from unittest.mock import Mock, patch, MagicMock +from unittest.mock import Mock, patch import pytest @@ -34,7 +33,6 @@ def mock_environment(tmp_path, monkeypatch): (core_dir / "__init__.py").write_text("# Core") # Monkey patch Path to return our test paths - original_path = Path monkeypatch.chdir(tmp_path) return tmp_path diff --git a/tests/test_cli/test_uploader.py b/tests/test_cli/test_uploader.py index 3d64b2e1..ea5e5df4 100644 --- a/tests/test_cli/test_uploader.py +++ b/tests/test_cli/test_uploader.py @@ -6,8 +6,7 @@ """Tests for uploader module.""" -from pathlib import Path -from unittest.mock import Mock, patch, MagicMock +from unittest.mock import Mock, patch import pytest From fc9ec5c5b7ecb571978c01d47a14d3027b76b98f Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 12:25:44 -0500 Subject: [PATCH 18/38] Fixed in-test imports in builder and no longer needed tests for merged push command logic update --- tests/test_cli/test_builder.py | 10 +--------- tests/test_cli/test_push_command.py | 27 +++------------------------ 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/tests/test_cli/test_builder.py b/tests/test_cli/test_builder.py index 3f3ed0de..d315f101 100644 --- a/tests/test_cli/test_builder.py +++ b/tests/test_cli/test_builder.py @@ -7,14 +7,12 @@ """Tests for builder module.""" import os -from pathlib import Path -from unittest.mock import patch, mock_open +from unittest.mock import patch import pytest from openenv_cli.core.builder import ( prepare_staging_directory, - copy_environment_files, prepare_dockerfile, prepare_readme, ) @@ -121,7 +119,6 @@ class TestPrepareDockerfile: def test_prepare_dockerfile_uses_existing_dockerfile(self, repo_root, tmp_path, monkeypatch): """Test that existing Dockerfile is used and modified.""" - from openenv_cli.core.builder import prepare_dockerfile, prepare_staging_directory # Create test environment with Dockerfile (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" / "server" @@ -155,7 +152,6 @@ def test_prepare_dockerfile_uses_existing_dockerfile(self, repo_root, tmp_path, def test_prepare_dockerfile_creates_default_if_missing(self, repo_root, tmp_path, monkeypatch): """Test that default Dockerfile is created if env doesn't have one.""" - from openenv_cli.core.builder import prepare_dockerfile, prepare_staging_directory # Create test environment without Dockerfile (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" @@ -187,7 +183,6 @@ class TestPrepareReadme: def test_prepare_readme_adds_front_matter(self, repo_root, tmp_path, monkeypatch): """Test that README gets HF front matter added.""" - from openenv_cli.core.builder import prepare_readme, prepare_staging_directory # Create test environment with README (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" @@ -218,7 +213,6 @@ def test_prepare_readme_adds_front_matter(self, repo_root, tmp_path, monkeypatch def test_prepare_readme_handles_existing_front_matter(self, repo_root, tmp_path, monkeypatch): """Test that README with existing front matter is used as-is.""" - from openenv_cli.core.builder import prepare_readme, prepare_staging_directory # Create test environment with README that has front matter (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" @@ -258,7 +252,6 @@ def test_prepare_readme_handles_existing_front_matter(self, repo_root, tmp_path, def test_prepare_readme_generates_front_matter_when_missing(self, repo_root, tmp_path, monkeypatch): """Test that README without front matter gets generated front matter.""" - from openenv_cli.core.builder import prepare_readme, prepare_staging_directory # Create test environment with README without front matter (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" @@ -294,7 +287,6 @@ def test_prepare_readme_generates_front_matter_when_missing(self, repo_root, tmp def test_prepare_readme_uses_server_readme_front_matter(self, repo_root, tmp_path, monkeypatch): """Test that server/README.md front matter is used when present.""" - from openenv_cli.core.builder import prepare_readme, prepare_staging_directory # Create test environment (test_env_path already created the directory) env_dir = repo_root / "src" / "envs" / "test_env" diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index fdf70283..d94c2a05 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -43,7 +43,6 @@ class TestPushEnvironment: @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -62,7 +61,6 @@ def test_push_environment_full_workflow( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, @@ -73,7 +71,6 @@ def test_push_environment_full_workflow( mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" - mock_space_exists.return_value = False mock_prepare_staging.return_value = Path("staging") # Run push @@ -83,7 +80,6 @@ def test_push_environment_full_workflow( mock_validate.assert_called_once_with("test_env") mock_ensure_auth.assert_called_once() mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name=None) - mock_space_exists.assert_called_once_with(mock_api, "test_user/test_env") mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) mock_prepare_staging.assert_called_once() mock_copy_files.assert_called_once() @@ -93,7 +89,6 @@ def test_push_environment_full_workflow( @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -112,25 +107,23 @@ def test_push_environment_space_exists( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, ): - """Test push when space already exists.""" + """Test push when space already exists (create_space handles it with exist_ok=True).""" # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" - mock_space_exists.return_value = True # Space already exists mock_prepare_staging.return_value = Path("staging") # Run push push_environment("test_env") - # Verify space was not created - mock_create_space.assert_not_called() + # Verify create_space was called (it handles existing spaces internally) + mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) @patch("openenv_cli.commands.push.validate_environment") def test_push_environment_invalid_env(self, mock_validate, mock_environment): @@ -152,7 +145,6 @@ def test_push_environment_auth_failure(self, mock_ensure_auth, mock_validate, mo @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -171,7 +163,6 @@ def test_push_environment_with_namespace( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, @@ -182,7 +173,6 @@ def test_push_environment_with_namespace( mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "my-org/test_env" - mock_space_exists.return_value = False mock_prepare_staging.return_value = Path("staging") # Run push with namespace @@ -193,7 +183,6 @@ def test_push_environment_with_namespace( @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -212,7 +201,6 @@ def test_push_environment_private( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, @@ -223,7 +211,6 @@ def test_push_environment_private( mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" - mock_space_exists.return_value = False mock_prepare_staging.return_value = Path("staging") # Run push with private flag @@ -234,7 +221,6 @@ def test_push_environment_private( @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -253,7 +239,6 @@ def test_push_environment_with_space_name( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, @@ -264,7 +249,6 @@ def test_push_environment_with_space_name( mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/custom-space" - mock_space_exists.return_value = False mock_prepare_staging.return_value = Path("staging") # Run push with space_name @@ -272,12 +256,10 @@ def test_push_environment_with_space_name( # Verify space_name was used mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name="custom-space") - mock_space_exists.assert_called_once_with(mock_api, "test_user/custom-space") mock_create_space.assert_called_once_with(mock_api, "test_user/custom-space", private=False) @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.space_exists") @patch("openenv_cli.commands.push.prepare_readme") @patch("openenv_cli.commands.push.prepare_dockerfile") @patch("openenv_cli.commands.push.copy_environment_files") @@ -296,7 +278,6 @@ def test_push_environment_with_namespace_and_space_name( mock_copy_files, mock_prepare_dockerfile, mock_prepare_readme, - mock_space_exists, mock_create_space, mock_upload, mock_environment, @@ -307,7 +288,6 @@ def test_push_environment_with_namespace_and_space_name( mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "my-org/custom-space" - mock_space_exists.return_value = False mock_prepare_staging.return_value = Path("staging") # Run push with namespace and space_name @@ -315,5 +295,4 @@ def test_push_environment_with_namespace_and_space_name( # Verify both namespace and space_name were used mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org", space_name="custom-space") - mock_space_exists.assert_called_once_with(mock_api, "my-org/custom-space") mock_create_space.assert_called_once_with(mock_api, "my-org/custom-space", private=False) From 6e118e703f98d244981c6fb248563ebb94737152 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 12:30:26 -0500 Subject: [PATCH 19/38] Enhance README generation in builder.py to handle empty environment names and avoid duplicates when appending original content. Added tests --- src/openenv_cli/core/builder.py | 32 +++++----- tests/test_cli/test_builder.py | 110 ++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 17 deletions(-) diff --git a/src/openenv_cli/core/builder.py b/src/openenv_cli/core/builder.py index 9e3b1e8c..60dbfc5d 100644 --- a/src/openenv_cli/core/builder.py +++ b/src/openenv_cli/core/builder.py @@ -173,7 +173,11 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: return # Otherwise, generate front matter with random emoji and colors - env_title = env_name[0].upper() + env_name[1:] if env_name else env_name + # Safely capitalize env_name - handle empty string and None cases + if env_name and len(env_name) > 0: + env_title = env_name[0].upper() + env_name[1:] + else: + env_title = "" # Approved emojis from Spaces Configuration Reference approved_emojis = [ @@ -247,26 +251,20 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: """ # Try to append content from original README if it exists (without front matter) + # Only append if the original README doesn't have front matter (if it has front matter, + # we've already used it as-is above, so we shouldn't append here to avoid duplicates) original_readme = Path("src/envs") / env_name / "README.md" if original_readme.exists(): original_content = original_readme.read_text() - # Skip front matter if present - if original_content.startswith("---"): - lines = original_content.split("\n") - closing_idx = None - for i in range(1, len(lines)): - if lines[i].strip() == "---": - closing_idx = i - break - - if closing_idx: - # Skip front matter - original_content = "\n".join(lines[closing_idx + 1:]).strip() - - # Append original content if there's any - if original_content: - readme_content += "\n\n" + original_content + # Only append if original README doesn't have front matter + # (if it has front matter, we already used it as-is earlier, so skip to avoid duplicates) + if not original_content.startswith("---"): + # Original README has no front matter, append its content after generated sections + # This preserves environment-specific documentation + original_content_stripped = original_content.strip() + if original_content_stripped: + readme_content += "\n\n" + original_content_stripped readme_path = staging_dir / "README.md" readme_path.write_text(readme_content) diff --git a/tests/test_cli/test_builder.py b/tests/test_cli/test_builder.py index d315f101..8b5ddd32 100644 --- a/tests/test_cli/test_builder.py +++ b/tests/test_cli/test_builder.py @@ -330,3 +330,113 @@ def test_prepare_readme_uses_server_readme_front_matter(self, repo_root, tmp_pat assert "colorTo: indigo" in content finally: os.chdir(old_cwd) + + def test_prepare_readme_handles_empty_env_name(self, repo_root, tmp_path, monkeypatch): + """Test that README generation handles empty env_name safely.""" + + # Create test environment (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "" + env_dir.mkdir(parents=True, exist_ok=True) + (env_dir / "README.md").write_text( + "# Test Environment\n\nThis is a test environment." + ) + + # Prepare staging directory + staging_dir = prepare_staging_directory("", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + # Should not raise IndexError + prepare_readme("", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should have generated front matter + assert content.startswith("---") + assert "sdk: docker" in content + # Title should handle empty string + assert "title:" in content + finally: + os.chdir(old_cwd) + + def test_prepare_readme_no_duplicate_when_original_has_front_matter(self, repo_root, tmp_path, monkeypatch): + """Test that original README with front matter is not appended (no duplicates).""" + + # Create test environment with README that has front matter (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + original_content = ( + "---\n" + "title: Test Environment\n" + "emoji: 🎮\n" + "colorFrom: red\n" + "colorTo: blue\n" + "sdk: docker\n" + "---\n" + "# Test Environment\n\nThis is a test environment.\n\n## About\n\nSome custom about content." + ) + (env_dir / "README.md").write_text(original_content) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should use original content as-is (not append duplicate sections) + assert content == original_content + # Should not have duplicate "About" sections + assert content.count("## About") == 1 + finally: + os.chdir(old_cwd) + + def test_prepare_readme_appends_original_without_front_matter(self, repo_root, tmp_path, monkeypatch): + """Test that original README without front matter is appended correctly.""" + + # Create test environment with README without front matter (test_env_path already created the directory) + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True, exist_ok=True) + original_content = ( + "# Test Environment\n\n" + "This is a test environment with custom documentation.\n\n" + "## Custom Section\n\n" + "This is environment-specific content that should be preserved." + ) + (env_dir / "README.md").write_text(original_content) + + # Prepare staging directory + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + prepare_readme("test_env", staging_dir) + + # Check README was created + readme_path = staging_dir / "README.md" + assert readme_path.exists() + + content = readme_path.read_text() + # Should have generated front matter + assert content.startswith("---") + # Should have generated standard sections + assert "## About" in content + assert "## Web Interface" in content + # Should append original content after generated sections + assert "Custom Section" in content + assert "environment-specific content" in content + # Should not duplicate the title - original "# Test Environment" appears once (from appended content) + assert content.count("# Test Environment") == 1 + finally: + os.chdir(old_cwd) From 475ca816d794fa8bc488328e0a00429f1aec1849 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 12:35:45 -0500 Subject: [PATCH 20/38] Refactor space creation error handling in create_space function - Removed the space_exists function as it was deemed unnecessary. - Enhanced error handling in create_space to provide clearer messages for authentication and permission errors. - Updated tests to validate the new error handling logic for create_space, ensuring informative exceptions are raised for various failure scenarios. --- src/openenv_cli/core/space.py | 45 +++++++++---------- tests/test_cli/test_space.py | 83 ++++++++++++++--------------------- 2 files changed, 57 insertions(+), 71 deletions(-) diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index a155f69d..6a5d09e2 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -13,23 +13,6 @@ from .auth import ensure_authenticated -def space_exists(api: HfApi, repo_id: str) -> bool: - """ - Check if a Docker Space exists. - - Args: - api: HfApi instance to use for API calls. - repo_id: Repository ID in format 'namespace/repo-name'. - - Returns: - True if space exists, False otherwise. - """ - try: - return api.repo_exists(repo_id=repo_id, repo_type="space") - except Exception: - return False - - def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: """ Create a Docker Space on HuggingFace. @@ -48,13 +31,31 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: repo_type="space", space_sdk="docker", private=private, - exist_ok=True # Don't fail if space already exists + exist_ok=True # Hub CLI handles existence checks and private/non-private re-creation ) except Exception as e: - # If space already exists, that's okay - if "already exists" in str(e).lower() or "repository already exists" in str(e).lower(): - return - raise + # Check for authentication-related errors and provide clearer messages + error_str = str(e).lower() + if any(keyword in error_str for keyword in ["unauthorized", "authentication", "401", "invalid token", "token"]): + raise Exception( + f"Authentication failed when creating space {repo_id}. " + f"Please check your HuggingFace token and ensure it has write permissions. " + f"Original error: {e}" + ) from e + + # Check for permission-related errors + if any(keyword in error_str for keyword in ["forbidden", "403", "permission", "not authorized"]): + raise Exception( + f"Permission denied when creating space {repo_id}. " + f"Please verify you have permission to create spaces in this namespace. " + f"Original error: {e}" + ) from e + + # Raise a clearer generic error for other cases + # Note: exist_ok=True handles space existence and will print warnings to the user + raise Exception( + f"Failed to create space {repo_id}: {e}" + ) from e def get_space_repo_id(env_name: str, namespace: Optional[str] = None, space_name: Optional[str] = None) -> str: diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py index 7fe542a7..b5e578cc 100644 --- a/tests/test_cli/test_space.py +++ b/tests/test_cli/test_space.py @@ -10,7 +10,7 @@ import pytest -from openenv_cli.core.space import space_exists, create_space, get_space_repo_id +from openenv_cli.core.space import create_space, get_space_repo_id @pytest.fixture @@ -19,42 +19,6 @@ def mock_hf_api(): return Mock() -class TestSpaceExists: - """Tests for space_exists function.""" - - def test_space_exists_true(self, mock_hf_api): - """Test space_exists returns True when space exists.""" - mock_hf_api.repo_exists.return_value = True - - result = space_exists(mock_hf_api, "test_user/my-env") - - assert result is True - mock_hf_api.repo_exists.assert_called_once_with( - repo_id="test_user/my-env", - repo_type="space" - ) - - def test_space_exists_false(self, mock_hf_api): - """Test space_exists returns False when space doesn't exist.""" - mock_hf_api.repo_exists.return_value = False - - result = space_exists(mock_hf_api, "test_user/my-env") - - assert result is False - mock_hf_api.repo_exists.assert_called_once_with( - repo_id="test_user/my-env", - repo_type="space" - ) - - def test_space_exists_error(self, mock_hf_api): - """Test space_exists handles errors gracefully.""" - mock_hf_api.repo_exists.side_effect = Exception("API error") - - # Should return False on error (not raise) - result = space_exists(mock_hf_api, "test_user/my-env") - assert result is False - - class TestCreateSpace: """Tests for create_space function.""" @@ -86,22 +50,43 @@ def test_create_space_private(self, mock_hf_api): exist_ok=True ) - def test_create_space_already_exists(self, mock_hf_api): - """Test creating a space that already exists.""" - from huggingface_hub.utils import RepositoryNotFoundError + + def test_create_space_authentication_error(self, mock_hf_api): + """Test that authentication errors are raised with clearer messages.""" + mock_hf_api.create_repo.side_effect = Exception("401 Unauthorized: Invalid token") + + with pytest.raises(Exception) as exc_info: + create_space(mock_hf_api, "test_user/my-env", private=False) - # RepositoryNotFoundError doesn't exist in all versions, use generic Exception - mock_hf_api.create_repo.side_effect = Exception("Repository already exists") + error_message = str(exc_info.value) + assert "Authentication failed" in error_message + assert "test_user/my-env" in error_message + assert "HuggingFace token" in error_message + assert "write permissions" in error_message + + def test_create_space_permission_error(self, mock_hf_api): + """Test that permission errors are raised with clearer messages.""" + mock_hf_api.create_repo.side_effect = Exception("403 Forbidden: Not authorized") + + with pytest.raises(Exception) as exc_info: + create_space(mock_hf_api, "test_user/my-env", private=False) + + error_message = str(exc_info.value) + assert "Permission denied" in error_message + assert "test_user/my-env" in error_message + assert "permission to create spaces" in error_message + + def test_create_space_generic_error(self, mock_hf_api): + """Test that generic errors are raised with clearer messages.""" + mock_hf_api.create_repo.side_effect = Exception("Network error occurred") - # Should not raise, just silently continue - # This tests the function handles the case gracefully - try: + with pytest.raises(Exception) as exc_info: create_space(mock_hf_api, "test_user/my-env", private=False) - except Exception: - # If it raises, that's okay - we just test the call was made - pass - mock_hf_api.create_repo.assert_called_once() + error_message = str(exc_info.value) + assert "Failed to create space" in error_message + assert "test_user/my-env" in error_message + assert "Network error occurred" in error_message class TestGetSpaceRepoId: From 0064c1cc1030e3dd44210ab001124c309e83b116 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 16:45:22 -0500 Subject: [PATCH 21/38] Update README.md Co-authored-by: burtenshaw --- src/openenv_cli/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index 2546d216..7ea96b9d 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -82,7 +82,7 @@ The CLI uses HuggingFace authentication. You can authenticate in two ways: To get a token: 1. Go to https://huggingface.co/settings/tokens -2. Create a new token with "write" permissions +2. Create a new token with "write" permissions on the namespace where you are pushing the environment. 3. Use it as `HF_TOKEN` or log in interactively ## How It Works From d30f42b5eb32056d199cca764bea15315c73b8d3 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 19:36:36 -0500 Subject: [PATCH 22/38] =?UTF-8?q?=F0=9F=A4=97?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openenv_cli/README.md | 40 ++++++++++++++++---------------- src/openenv_cli/__main__.py | 8 +++---- src/openenv_cli/commands/push.py | 6 ++--- src/openenv_cli/core/auth.py | 6 ++--- src/openenv_cli/core/builder.py | 6 ++--- src/openenv_cli/core/space.py | 6 ++--- src/openenv_cli/core/uploader.py | 6 ++--- tests/test_cli/test_space.py | 2 +- 8 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index 7ea96b9d..7af77847 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -1,10 +1,10 @@ # OpenEnv CLI -Command-line tool for managing and deploying OpenEnv environments to HuggingFace Spaces. +Command-line tool for managing and deploying OpenEnv environments to Hugging Face Spaces. ## Overview -The OpenEnv CLI provides a self-service workflow for publishing environments to HuggingFace Spaces, enabling community members to share environments without requiring GitHub PRs. The CLI handles authentication, space provisioning, building, and deployment automatically. +The OpenEnv CLI provides a self-service workflow for publishing environments to Hugging Face Spaces, enabling community members to share environments without requiring GitHub PRs. The CLI handles authentication, space provisioning, building, and deployment automatically. ## Installation @@ -24,7 +24,7 @@ pip install -e ".[dev]" ### Push Environment -Push an environment to HuggingFace Spaces: +Push an environment to Hugging Face Spaces: ```bash openenv push [options] @@ -34,11 +34,11 @@ openenv push [options] - `env_name`: Name of the environment to push (e.g., `echo_env`, `coding_env`) **Options:** -- `--namespace `: HuggingFace namespace (organization or user). If not provided, uses authenticated user's username. -- `--space-name `: Custom name for the HuggingFace Space. If not provided, uses the environment name. +- `--namespace `: Hugging Face namespace (organization or user). If not provided, uses authenticated user's username. +- `--space-name `: Custom name for the Hugging Face Space. If not provided, uses the environment name. - `--private`: Create a private space (default: public) - `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) -- `--dry-run`: Prepare files but don't upload to HuggingFace +- `--dry-run`: Prepare files but don't upload to Hugging Face **Examples:** @@ -67,9 +67,9 @@ openenv push echo_env --dry-run ### Authentication -The CLI uses HuggingFace authentication. You can authenticate in two ways: +The CLI uses Hugging Face authentication. You can authenticate in two ways: -1. **Environment Variable**: Set `HF_TOKEN` environment variable with your HuggingFace token +1. **Environment Variable**: Set `HF_TOKEN` environment variable with your Hugging Face token ```bash export HF_TOKEN=your_token_here ``` @@ -90,14 +90,14 @@ To get a token: The `openenv push` command performs the following steps: 1. **Validation**: Checks that the environment exists in `src/envs//` -2. **Authentication**: Ensures you're authenticated with HuggingFace (prompts if needed) +2. **Authentication**: Ensures you're authenticated with Hugging Face (prompts if needed) 3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Checks if a Docker Space exists, creates it if needed 4. **Build Process**: - Creates a staging directory - Copies core and environment files - Generates/modifies Dockerfile with web interface enabled - - Prepares README: If the environment's README already has HuggingFace front matter (starts and ends with `---`), uses it as-is. Otherwise, generates front matter with random emoji and colors from approved options -5. **Deployment**: Uploads all files to the HuggingFace Space + - Prepares README: If the environment's README already has Hugging Face front matter (starts and ends with `---`), uses it as-is. Otherwise, generates front matter with random emoji and colors from approved options +5. **Deployment**: Uploads all files to the Hugging Face Space 6. **Cleanup**: Removes staging directory after successful upload ## Testing @@ -132,7 +132,7 @@ pytest tests/test_cli/test_push_command.py ### Test Structure -Tests follow pytest conventions and use mocking to avoid requiring actual HuggingFace API access: +Tests follow pytest conventions and use mocking to avoid requiring actual Hugging Face API access: - **Unit Tests**: Each module (`auth`, `space`, `builder`, `uploader`) has dedicated tests - **Integration Tests**: `test_push_command.py` tests the full workflow with mocks @@ -175,20 +175,20 @@ src/openenv_cli/ ├── commands/ │ └── push.py # Push command implementation ├── core/ -│ ├── auth.py # HuggingFace authentication +│ ├── auth.py # Hugging Face authentication │ ├── space.py # Space management (create/check) │ ├── builder.py # Build staging directory and files -│ └── uploader.py # Upload to HuggingFace Spaces +│ └── uploader.py # Upload to Hugging Face Spaces └── utils/ └── env_loader.py # Environment validation and metadata ``` ### Module Responsibilities -- **`auth.py`**: Handles HuggingFace authentication using `huggingface_hub` +- **`auth.py`**: Handles Hugging Face authentication using `huggingface_hub` - **`space.py`**: Manages Docker Spaces (check existence, create) - **`builder.py`**: Prepares deployment package (copy files, generate Dockerfile/README) -- **`uploader.py`**: Uploads files to HuggingFace using `upload_folder` +- **`uploader.py`**: Uploads files to Hugging Face using `upload_folder` - **`env_loader.py`**: Validates environment structure and loads metadata ## Implementation Details @@ -202,7 +202,7 @@ The CLI uses `huggingface_hub` Python modules directly (not the CLI), as specifi - `huggingface_hub.upload_folder()` for file uploads - `huggingface_hub.utils.get_token` for token management -This ensures consistency with HuggingFace tooling and allows for better error handling and programmatic control. +This ensures consistency with Hugging Face tooling and allows for better error handling and programmatic control. ### Web Interface @@ -219,7 +219,7 @@ The builder creates a staging directory structure: ``` hf-staging// ├── Dockerfile # Generated/modified Dockerfile -├── README.md # With HuggingFace front matter +├── README.md # With Hugging Face front matter └── src/ ├── core/ # Copied from src/core/ └── envs/ @@ -361,12 +361,12 @@ validate_parser.add_argument("env_name") When adding new features: 1. **Write Tests First**: Follow TDD approach, write tests before implementation -2. **Use Mocks**: Mock external APIs (HuggingFace) to keep tests fast and isolated +2. **Use Mocks**: Mock external APIs (Hugging Face) to keep tests fast and isolated 3. **Follow Patterns**: Match existing code style and patterns 4. **Update Documentation**: Update this README and add docstrings ## References -- [HuggingFace Hub Python API](https://huggingface.co/docs/huggingface_hub/index) +- [Hugging Face Hub Python API](https://huggingface.co/docs/huggingface_hub/index) - [OpenEnv Environment Structure](../envs/README.md) - [OpenEnv RFCs](../../rfcs/) diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index b7e03452..a2bb9342 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -31,7 +31,7 @@ def main(): # Push command push_parser = subparsers.add_parser( "push", - help="Push an environment to HuggingFace Spaces", + help="Push an environment to Hugging Face Spaces", ) push_parser.add_argument( "env_name", @@ -39,12 +39,12 @@ def main(): ) push_parser.add_argument( "--namespace", - help="HuggingFace namespace (organization or user). " + help="Hugging Face namespace (organization or user). " "If not provided, uses authenticated user's username.", ) push_parser.add_argument( "--space-name", - help="Custom name for the HuggingFace Space. " + help="Custom name for the Hugging Face Space. " "If not provided, uses the environment name.", ) push_parser.add_argument( @@ -60,7 +60,7 @@ def main(): push_parser.add_argument( "--dry-run", action="store_true", - help="Prepare files but don't upload to HuggingFace", + help="Prepare files but don't upload to Hugging Face", ) args = parser.parse_args() diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index daa4bd51..cc9259f0 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -4,7 +4,7 @@ # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -"""Push command for deploying environments to HuggingFace Spaces.""" +"""Push command for deploying environments to Hugging Face Spaces.""" from typing import Optional @@ -31,7 +31,7 @@ def push_environment( dry_run: bool = False, ) -> None: """ - Push an environment to HuggingFace Spaces. + Push an environment to Hugging Face Spaces. Args: env_name: Name of the environment to push. @@ -45,7 +45,7 @@ def push_environment( # Validate environment exists validate_environment(env_name) - # Authenticate with HuggingFace + # Authenticate with Hugging Face _, token = ensure_authenticated() # Determine target space repo ID diff --git a/src/openenv_cli/core/auth.py b/src/openenv_cli/core/auth.py index e949b507..5776968e 100644 --- a/src/openenv_cli/core/auth.py +++ b/src/openenv_cli/core/auth.py @@ -4,7 +4,7 @@ # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -"""Authentication module for HuggingFace.""" +"""Authentication module for Hugging Face.""" import os from typing import Optional, Tuple @@ -15,7 +15,7 @@ def check_authentication() -> Optional[str]: """ - Check if user is authenticated with HuggingFace. + Check if user is authenticated with Hugging Face. Returns: Username if authenticated, None otherwise. @@ -78,7 +78,7 @@ def ensure_authenticated() -> Tuple[str, str]: def login_interactive() -> str: """ - Perform interactive login to HuggingFace. + Perform interactive login to Hugging Face. Returns: Username after successful login. diff --git a/src/openenv_cli/core/builder.py b/src/openenv_cli/core/builder.py index 60dbfc5d..4426dd86 100644 --- a/src/openenv_cli/core/builder.py +++ b/src/openenv_cli/core/builder.py @@ -134,9 +134,9 @@ def prepare_dockerfile(env_name: str, staging_dir: Path, base_image: str) -> Non def prepare_readme(env_name: str, staging_dir: Path) -> None: """ - Prepare README.md with HuggingFace front matter. + Prepare README.md with Hugging Face front matter. - If the environment README already has HuggingFace front matter (starts and ends with `---`), + If the environment README already has Hugging Face front matter (starts and ends with `---`), use it as-is. Otherwise, generate front matter with random emoji and colors. Args: @@ -149,7 +149,7 @@ def prepare_readme(env_name: str, staging_dir: Path) -> None: Path("src/envs") / env_name / "server" / "README.md", ] - # Check if any README has HuggingFace front matter + # Check if any README has Hugging Face front matter existing_readme_content = None for readme_path in readme_paths: if readme_path.exists(): diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index 6a5d09e2..d839f52b 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -4,7 +4,7 @@ # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -"""Space management module for HuggingFace Spaces.""" +"""Space management module for Hugging Face Spaces.""" from typing import Optional @@ -15,7 +15,7 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: """ - Create a Docker Space on HuggingFace. + Create a Docker Space on Hugging Face. Args: api: HfApi instance to use for API calls. @@ -39,7 +39,7 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: if any(keyword in error_str for keyword in ["unauthorized", "authentication", "401", "invalid token", "token"]): raise Exception( f"Authentication failed when creating space {repo_id}. " - f"Please check your HuggingFace token and ensure it has write permissions. " + f"Please check your Hugging Face token and ensure it has write permissions. " f"Original error: {e}" ) from e diff --git a/src/openenv_cli/core/uploader.py b/src/openenv_cli/core/uploader.py index 001fc3fe..28f7a4a7 100644 --- a/src/openenv_cli/core/uploader.py +++ b/src/openenv_cli/core/uploader.py @@ -4,7 +4,7 @@ # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -"""Uploader module for deploying to HuggingFace Spaces.""" +"""Uploader module for deploying to Hugging Face Spaces.""" from pathlib import Path @@ -19,13 +19,13 @@ def upload_to_space( token: str, ) -> None: """ - Upload staging directory contents to HuggingFace Space. + Upload staging directory contents to Hugging Face Space. Args: api: HfApi instance to use for API calls. repo_id: Repository ID in format 'namespace/repo-name'. staging_dir: Path to staging directory to upload. - token: HuggingFace token for authentication. + token: Hugging Face token for authentication. Raises: Exception: If upload fails. diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py index b5e578cc..eab47e18 100644 --- a/tests/test_cli/test_space.py +++ b/tests/test_cli/test_space.py @@ -61,7 +61,7 @@ def test_create_space_authentication_error(self, mock_hf_api): error_message = str(exc_info.value) assert "Authentication failed" in error_message assert "test_user/my-env" in error_message - assert "HuggingFace token" in error_message + assert "Hugging Face token" in error_message assert "write permissions" in error_message def test_create_space_permission_error(self, mock_hf_api): From 2131e415844f5333fdd770c56d0dacf206fdb780 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 19:45:35 -0500 Subject: [PATCH 23/38] Better devex by supporting interactive login only (no $HF_TOKEN) --- src/openenv_cli/README.md | 51 +++++++++++++++++------------------- src/openenv_cli/core/auth.py | 26 ++++-------------- tests/test_cli/test_auth.py | 30 +-------------------- 3 files changed, 30 insertions(+), 77 deletions(-) diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index 7af77847..f7024209 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -67,31 +67,25 @@ openenv push echo_env --dry-run ### Authentication -The CLI uses Hugging Face authentication. You can authenticate in two ways: +The CLI uses Hugging Face authentication via interactive login. When you run a command that requires authentication, the CLI will prompt you to log in: -1. **Environment Variable**: Set `HF_TOKEN` environment variable with your Hugging Face token - ```bash - export HF_TOKEN=your_token_here - ``` +```bash +# The CLI will automatically prompt for login when needed +openenv push echo_env +``` -2. **Interactive Login**: If no token is found, the CLI will prompt for interactive login: - ```bash - # The CLI will automatically prompt when needed - openenv push echo_env - ``` +The login process will: +1. Open your browser to authenticate with Hugging Face +2. Store your credentials for future use (from `huggingface_hub`) -To get a token: -1. Go to https://huggingface.co/settings/tokens -2. Create a new token with "write" permissions on the namespace where you are pushing the environment. -3. Use it as `HF_TOKEN` or log in interactively ## How It Works The `openenv push` command performs the following steps: 1. **Validation**: Checks that the environment exists in `src/envs//` -2. **Authentication**: Ensures you're authenticated with Hugging Face (prompts if needed) -3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Checks if a Docker Space exists, creates it if needed +2. **Authentication**: Ensures you're authenticated with Hugging Face via interactive login (prompts if needed) +3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Creates the Docker Space if needed (using `exist_ok=True` to handle existing spaces automatically) 4. **Build Process**: - Creates a staging directory - Copies core and environment files @@ -185,8 +179,8 @@ src/openenv_cli/ ### Module Responsibilities -- **`auth.py`**: Handles Hugging Face authentication using `huggingface_hub` -- **`space.py`**: Manages Docker Spaces (check existence, create) +- **`auth.py`**: Handles Hugging Face authentication via interactive login using `huggingface_hub` +- **`space.py`**: Manages Docker Spaces (creates with `exist_ok=True` to handle existing spaces) - **`builder.py`**: Prepares deployment package (copy files, generate Dockerfile/README) - **`uploader.py`**: Uploads files to Hugging Face using `upload_folder` - **`env_loader.py`**: Validates environment structure and loads metadata @@ -198,11 +192,11 @@ src/openenv_cli/ The CLI uses `huggingface_hub` Python modules directly (not the CLI), as specified: - `huggingface_hub.HfApi` for API calls -- `huggingface_hub.login()` for authentication +- `huggingface_hub.login()` for interactive authentication - `huggingface_hub.upload_folder()` for file uploads -- `huggingface_hub.utils.get_token` for token management +- `huggingface_hub.utils.get_token` for token management (reads stored credentials from previous login) -This ensures consistency with Hugging Face tooling and allows for better error handling and programmatic control. +This ensures consistency with Hugging Face tooling and allows for better error handling and programmatic control. Authentication is handled exclusively through interactive login via the browser, with credentials stored by `huggingface_hub` for future use. ### Web Interface @@ -321,21 +315,23 @@ validate_parser.add_argument("env_name") ### Authentication Issues -**Problem**: "Failed to retrieve token after login" +**Problem**: "Failed to retrieve token after login" or authentication errors **Solution**: - Check that `huggingface_hub` is properly installed: `pip install --upgrade huggingface_hub` -- Verify token permissions (needs "write" access) -- Try logging in via CLI: `huggingface-cli login` +- Try logging in via the Hugging Face CLI: `huggingface-cli login` +- Clear cached credentials if needed (credentials are stored by `huggingface_hub`) +- Ensure you have "write" permissions on the namespace where you're pushing ### Space Creation Fails -**Problem**: "Failed to create space" +**Problem**: "Failed to create space" or "Permission denied" **Solution**: - Check that namespace/username is correct - Verify you have permission to create spaces in that namespace -- Ensure space name doesn't already exist (check on huggingface.co) +- If the space already exists, `exist_ok=True` handles it automatically (you may see a warning from the Hub CLI) +- For authentication errors, see "Authentication Issues" above ### Upload Fails @@ -343,9 +339,10 @@ validate_parser.add_argument("env_name") **Solution**: - Check internet connection -- Verify token hasn't expired +- Verify you're still authenticated (may need to log in again) - Try `--dry-run` first to check file preparation - Check staging directory exists and has files +- Verify you have write permissions on the target space ### Environment Not Found diff --git a/src/openenv_cli/core/auth.py b/src/openenv_cli/core/auth.py index 5776968e..1552502a 100644 --- a/src/openenv_cli/core/auth.py +++ b/src/openenv_cli/core/auth.py @@ -6,7 +6,6 @@ """Authentication module for Hugging Face.""" -import os from typing import Optional, Tuple from huggingface_hub import HfApi, login @@ -20,12 +19,8 @@ def check_authentication() -> Optional[str]: Returns: Username if authenticated, None otherwise. """ - # Check for token in environment variable first - token = os.environ.get("HF_TOKEN") - - if not token: - # Try to get token from stored credentials - token = get_token() + # Get token from stored credentials (from previous login) + token = get_token() if not token: return None @@ -49,27 +44,16 @@ def ensure_authenticated() -> Tuple[str, str]: Raises: Exception: If authentication fails. """ - # Check for token in environment variable first - token = os.environ.get("HF_TOKEN") - - if token: - try: - api = HfApi(token=token) - user_info = api.whoami() - return user_info.get("name"), token - except Exception: - pass # Fall through to login - - # Check existing authentication + # Check existing authentication (from stored credentials) username = check_authentication() if username: - token = get_token() or os.environ.get("HF_TOKEN") + token = get_token() if token: return username, token # Need to login username = login_interactive() - token = get_token() or os.environ.get("HF_TOKEN") + token = get_token() if not token: raise Exception("Failed to retrieve token after login") diff --git a/tests/test_cli/test_auth.py b/tests/test_cli/test_auth.py index 8ca2157a..e0277629 100644 --- a/tests/test_cli/test_auth.py +++ b/tests/test_cli/test_auth.py @@ -6,8 +6,7 @@ """Tests for authentication module.""" -import os -from unittest.mock import Mock, patch, MagicMock +from unittest.mock import Mock, patch import pytest @@ -68,20 +67,6 @@ def test_check_authentication_invalid_token(self, mock_get_token, mock_api_class assert result is None - @patch("openenv_cli.core.auth.HfApi") - @patch.dict(os.environ, {"HF_TOKEN": "env_token"}) - def test_check_authentication_env_token(self, mock_api_class): - """Test check_authentication using HF_TOKEN environment variable.""" - mock_api = Mock() - mock_api.whoami.return_value = {"name": "env_user"} - mock_api_class.return_value = mock_api - - result = check_authentication() - - assert result == "env_user" - mock_api_class.assert_called_once_with(token="env_token") - mock_api.whoami.assert_called_once() - class TestEnsureAuthenticated: """Tests for ensure_authenticated function.""" @@ -114,19 +99,6 @@ def test_ensure_authenticated_needs_login(self, mock_get_token, mock_login, mock assert token == "new_token" mock_login.assert_called_once() - @patch.dict(os.environ, {"HF_TOKEN": "env_token"}) - @patch("openenv_cli.core.auth.HfApi") - def test_ensure_authenticated_env_token(self, mock_api_class): - """Test ensure_authenticated using HF_TOKEN environment variable.""" - mock_api = Mock() - mock_api.whoami.return_value = {"name": "env_user"} - mock_api_class.return_value = mock_api - - username, token = ensure_authenticated() - - assert username == "env_user" - assert token == "env_token" - class TestLoginInteractive: """Tests for login_interactive function.""" From 880ec6d2466c8629eefb3913b0e9dacacbb52744 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 19:49:12 -0500 Subject: [PATCH 24/38] Moved openenv cli usage instructions to main README --- README.md | 129 +++++++++++++++++++++++++++++++++++++ src/openenv_cli/README.md | 132 +------------------------------------- 2 files changed, 131 insertions(+), 130 deletions(-) diff --git a/README.md b/README.md index c4cd0a08..a2412c7c 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,135 @@ To use an environment: See example scripts in `examples/` directory. +### Deploying Environments to Hugging Face Spaces + +The OpenEnv CLI provides a self-service workflow for publishing environments to Hugging Face Spaces. This enables community members to share environments without requiring adding them as examples to this repo. + +#### Installation + +The CLI is installed as part of the OpenEnv package: + +```bash +pip install -e . +``` + +#### Push Environment + +Push an environment to Hugging Face Spaces: + +```bash +openenv push [options] +``` + +**Arguments:** +- `env_name`: Name of the environment to push (e.g., `echo_env`, `coding_env`) + +**Options:** +- `--namespace `: Hugging Face namespace (organization or user). If not provided, uses authenticated user's username. +- `--space-name `: Custom name for the Hugging Face Space. If not provided, uses the environment name. +- `--private`: Create a private space (default: public) +- `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) +- `--dry-run`: Prepare files but don't upload to Hugging Face + +**Examples:** + +```bash +# Push echo_env to your personal namespace +openenv push echo_env + +# Push to a specific organization +openenv push coding_env --namespace my-org + +# Push with a custom space name +openenv push echo_env --space-name my-custom-space + +# Push to an organization with a custom space name +openenv push echo_env --namespace my-org --space-name my-custom-space + +# Create a private space +openenv push echo_env --private + +# Use a custom base image +openenv push echo_env --base-image ghcr.io/my-org/custom-base:latest + +# Prepare files without uploading +openenv push echo_env --dry-run +``` + +#### Authentication + +The CLI uses Hugging Face authentication via interactive login. When you run a command that requires authentication, the CLI will prompt you to log in: + +```bash +# The CLI will automatically prompt for login when needed +openenv push echo_env +``` + +The login process will: +1. Open your browser to authenticate with Hugging Face +2. Store your credentials for future use (from `huggingface_hub`) + +#### How It Works + +The `openenv push` command performs the following steps: + +1. **Validation**: Checks that the environment exists in `src/envs//` +2. **Authentication**: Ensures you're authenticated with Hugging Face via interactive login (prompts if needed) +3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Creates the Docker Space if needed (using `exist_ok=True` to handle existing spaces automatically) +4. **Build Process**: + - Creates a staging directory + - Copies core and environment files + - Generates/modifies Dockerfile with web interface enabled + - Prepares README: If the environment's README already has Hugging Face front matter (starts and ends with `---`), uses it as-is. Otherwise, generates front matter with random emoji and colors from approved options +5. **Deployment**: Uploads all files to the Hugging Face Space +6. **Cleanup**: Removes staging directory after successful upload + +All pushed environments automatically include the web interface, available at `/web` on deployed spaces. + +For more details on the CLI architecture and development, see [`src/openenv_cli/README.md`](src/openenv_cli/README.md). + +## CLI Troubleshooting + +### Authentication Issues + +**Problem**: "Failed to retrieve token after login" or authentication errors + +**Solution**: +- Check that `huggingface_hub` is properly installed: `pip install --upgrade huggingface_hub` +- Try logging in via the Hugging Face CLI: `huggingface-cli login` +- Clear cached credentials if needed (credentials are stored by `huggingface_hub`) +- Ensure you have "write" permissions on the namespace where you're pushing + +### Space Creation Fails + +**Problem**: "Failed to create space" or "Permission denied" + +**Solution**: +- Check that namespace/username is correct +- Verify you have permission to create spaces in that namespace +- If the space already exists, `exist_ok=True` handles it automatically (you may see a warning from the Hub CLI) +- For authentication errors, see "Authentication Issues" above + +### Upload Fails + +**Problem**: "Failed to upload to space" + +**Solution**: +- Check internet connection +- Verify you're still authenticated (may need to log in again) +- Try `--dry-run` first to check file preparation +- Check staging directory exists and has files +- Verify you have write permissions on the target space + +### Environment Not Found + +**Problem**: "Environment 'xyz' not found" + +**Solution**: +- Verify environment exists in `src/envs//` +- Check spelling of environment name +- Ensure environment directory has required structure (models.py, server/, etc.) + ## Design Principles 1. **Separation of Concerns**: Clear client-server boundaries diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index f7024209..2ee50c36 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -2,98 +2,12 @@ Command-line tool for managing and deploying OpenEnv environments to Hugging Face Spaces. +> **Note**: For basic usage and examples, see the main [README.md](../../README.md#deploying-environments-to-hugging-face-spaces) in the project root. This document focuses on CLI development, testing, and architecture. + ## Overview The OpenEnv CLI provides a self-service workflow for publishing environments to Hugging Face Spaces, enabling community members to share environments without requiring GitHub PRs. The CLI handles authentication, space provisioning, building, and deployment automatically. -## Installation - -The CLI is installed as part of the OpenEnv package: - -```bash -pip install -e . -``` - -Or install with development dependencies: - -```bash -pip install -e ".[dev]" -``` - -## Usage - -### Push Environment - -Push an environment to Hugging Face Spaces: - -```bash -openenv push [options] -``` - -**Arguments:** -- `env_name`: Name of the environment to push (e.g., `echo_env`, `coding_env`) - -**Options:** -- `--namespace `: Hugging Face namespace (organization or user). If not provided, uses authenticated user's username. -- `--space-name `: Custom name for the Hugging Face Space. If not provided, uses the environment name. -- `--private`: Create a private space (default: public) -- `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) -- `--dry-run`: Prepare files but don't upload to Hugging Face - -**Examples:** - -```bash -# Push echo_env to your personal namespace -openenv push echo_env - -# Push to a specific organization -openenv push coding_env --namespace my-org - -# Push with a custom space name -openenv push echo_env --space-name my-custom-space - -# Push to an organization with a custom space name -openenv push echo_env --namespace my-org --space-name my-custom-space - -# Create a private space -openenv push echo_env --private - -# Use a custom base image -openenv push echo_env --base-image ghcr.io/my-org/custom-base:latest - -# Prepare files without uploading -openenv push echo_env --dry-run -``` - -### Authentication - -The CLI uses Hugging Face authentication via interactive login. When you run a command that requires authentication, the CLI will prompt you to log in: - -```bash -# The CLI will automatically prompt for login when needed -openenv push echo_env -``` - -The login process will: -1. Open your browser to authenticate with Hugging Face -2. Store your credentials for future use (from `huggingface_hub`) - - -## How It Works - -The `openenv push` command performs the following steps: - -1. **Validation**: Checks that the environment exists in `src/envs//` -2. **Authentication**: Ensures you're authenticated with Hugging Face via interactive login (prompts if needed) -3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Creates the Docker Space if needed (using `exist_ok=True` to handle existing spaces automatically) -4. **Build Process**: - - Creates a staging directory - - Copies core and environment files - - Generates/modifies Dockerfile with web interface enabled - - Prepares README: If the environment's README already has Hugging Face front matter (starts and ends with `---`), uses it as-is. Otherwise, generates front matter with random emoji and colors from approved options -5. **Deployment**: Uploads all files to the Hugging Face Space -6. **Cleanup**: Removes staging directory after successful upload - ## Testing ### Running Tests @@ -311,48 +225,6 @@ validate_parser.add_argument("env_name") # ... handle command ``` -## Troubleshooting - -### Authentication Issues - -**Problem**: "Failed to retrieve token after login" or authentication errors - -**Solution**: -- Check that `huggingface_hub` is properly installed: `pip install --upgrade huggingface_hub` -- Try logging in via the Hugging Face CLI: `huggingface-cli login` -- Clear cached credentials if needed (credentials are stored by `huggingface_hub`) -- Ensure you have "write" permissions on the namespace where you're pushing - -### Space Creation Fails - -**Problem**: "Failed to create space" or "Permission denied" - -**Solution**: -- Check that namespace/username is correct -- Verify you have permission to create spaces in that namespace -- If the space already exists, `exist_ok=True` handles it automatically (you may see a warning from the Hub CLI) -- For authentication errors, see "Authentication Issues" above - -### Upload Fails - -**Problem**: "Failed to upload to space" - -**Solution**: -- Check internet connection -- Verify you're still authenticated (may need to log in again) -- Try `--dry-run` first to check file preparation -- Check staging directory exists and has files -- Verify you have write permissions on the target space - -### Environment Not Found - -**Problem**: "Environment 'xyz' not found" - -**Solution**: -- Verify environment exists in `src/envs//` -- Check spelling of environment name -- Ensure environment directory has required structure (models.py, server/, etc.) - ## Contributing When adding new features: From 98d7b0a8c60e19f6582fe33235dde38e049e7c96 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 20:43:47 -0500 Subject: [PATCH 25/38] Refactor `openenv push` command to use `--repo-id` for specifying Hugging Face repository ID, replacing `--namespace` and `--space-name` options. - Fix interactive authentication bug. --- README.md | 12 ++--- src/openenv_cli/__main__.py | 29 ++++++------ src/openenv_cli/commands/push.py | 16 +++---- src/openenv_cli/core/space.py | 19 +++----- tests/test_cli/test_push_command.py | 70 +++++++---------------------- tests/test_cli/test_space.py | 34 +++----------- 6 files changed, 51 insertions(+), 129 deletions(-) diff --git a/README.md b/README.md index a2412c7c..cded2b56 100644 --- a/README.md +++ b/README.md @@ -167,8 +167,7 @@ openenv push [options] - `env_name`: Name of the environment to push (e.g., `echo_env`, `coding_env`) **Options:** -- `--namespace `: Hugging Face namespace (organization or user). If not provided, uses authenticated user's username. -- `--space-name `: Custom name for the Hugging Face Space. If not provided, uses the environment name. +- `--repo-id `: Hugging Face repository ID in format `namespace/space-name`. If not provided, uses `{username}/{env_name}`. - `--private`: Create a private space (default: public) - `--base-image `: Base Docker image to use (default: `ghcr.io/meta-pytorch/openenv-base:latest`) - `--dry-run`: Prepare files but don't upload to Hugging Face @@ -180,13 +179,10 @@ openenv push [options] openenv push echo_env # Push to a specific organization -openenv push coding_env --namespace my-org +openenv push coding_env --repo-id my-org/coding_env # Push with a custom space name -openenv push echo_env --space-name my-custom-space - -# Push to an organization with a custom space name -openenv push echo_env --namespace my-org --space-name my-custom-space +openenv push echo_env --repo-id my-org/my-custom-space # Create a private space openenv push echo_env --private @@ -217,7 +213,7 @@ The `openenv push` command performs the following steps: 1. **Validation**: Checks that the environment exists in `src/envs//` 2. **Authentication**: Ensures you're authenticated with Hugging Face via interactive login (prompts if needed) -3. **Space Provisioning**: Determines the target Space name (uses `--space-name` if provided, otherwise `env_name`) and namespace (`--namespace` if provided, otherwise authenticated user). Creates the Docker Space if needed (using `exist_ok=True` to handle existing spaces automatically) +3. **Space Provisioning**: Determines the target Space repository ID (uses `--repo-id` if provided, otherwise `{username}/{env_name}`). Creates the Docker Space if needed (using `exist_ok=True` to handle existing spaces automatically) 4. **Build Process**: - Creates a staging directory - Copies core and environment files diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index a2bb9342..5bef19a8 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -13,6 +13,7 @@ from rich.traceback import install from .commands.push import push_environment +from .core.auth import ensure_authenticated console = Console() @@ -38,14 +39,9 @@ def main(): help="Name of the environment to push (e.g., echo_env)", ) push_parser.add_argument( - "--namespace", - help="Hugging Face namespace (organization or user). " - "If not provided, uses authenticated user's username.", - ) - push_parser.add_argument( - "--space-name", - help="Custom name for the Hugging Face Space. " - "If not provided, uses the environment name.", + "--repo-id", + help="Hugging Face repository ID in format 'namespace/space-name'. " + "If not provided, uses '{username}/{env_name}'.", ) push_parser.add_argument( "--private", @@ -66,17 +62,20 @@ def main(): args = parser.parse_args() if args.command == "push": - if args.dry_run: - status_message = f"[bold yellow]Preparing dry run for '{args.env_name}'...[/bold yellow]" - else: - status_message = f"[bold cyan]Pushing environment '{args.env_name}'...[/bold cyan]" - try: + # Authenticate first (before status spinner) to allow interactive login if needed + console.print(f"[bold cyan]Authenticating...[/bold cyan]") + username, token = ensure_authenticated() + + if args.dry_run: + status_message = f"[bold yellow]Preparing dry run for '{args.env_name}'...[/bold yellow]" + else: + status_message = f"[bold cyan]Pushing environment '{args.env_name}'...[/bold cyan]" + with console.status(status_message): push_environment( env_name=args.env_name, - namespace=args.namespace, - space_name=args.space_name, + repo_id=args.repo_id, private=args.private, base_image=args.base_image, dry_run=args.dry_run, diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index cc9259f0..0e6dbcb7 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -24,8 +24,7 @@ def push_environment( env_name: str, - namespace: Optional[str] = None, - space_name: Optional[str] = None, + repo_id: Optional[str] = None, private: bool = False, base_image: Optional[str] = None, dry_run: bool = False, @@ -35,9 +34,8 @@ def push_environment( Args: env_name: Name of the environment to push. - namespace: Optional namespace (organization or user). If not provided, - uses the authenticated user's username. - space_name: Optional custom space name. If not provided, uses env_name. + repo_id: Optional repository ID in format 'namespace/space-name'. If not provided, + uses '{username}/{env_name}'. private: Whether the space should be private (default: False). base_image: Base Docker image to use (default: ghcr.io/meta-pytorch/openenv-base:latest). dry_run: If True, prepare files but don't upload (default: False). @@ -45,11 +43,13 @@ def push_environment( # Validate environment exists validate_environment(env_name) - # Authenticate with Hugging Face - _, token = ensure_authenticated() + # Get token (authentication should already be done in __main__, but get token for API) + # ensure_authenticated is idempotent - if already authenticated, it returns immediately + username, token = ensure_authenticated() # Determine target space repo ID - repo_id = get_space_repo_id(env_name, namespace=namespace, space_name=space_name) + if repo_id is None: + repo_id = get_space_repo_id(env_name) # Create HfApi instance api = HfApi(token=token) diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index d839f52b..4aaacc46 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -58,25 +58,16 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: ) from e -def get_space_repo_id(env_name: str, namespace: Optional[str] = None, space_name: Optional[str] = None) -> str: +def get_space_repo_id(env_name: str) -> str: """ - Get the full repository ID for a space. + Get the full repository ID for a space using the authenticated user's username. Args: - env_name: Environment name (e.g., "echo_env"). Used as space name if space_name is not provided. - namespace: Optional namespace (organization or user). If not provided, - uses the authenticated user's username. - space_name: Optional custom space name. If provided, used instead of env_name. + env_name: Environment name (e.g., "echo_env"). Used as space name. Returns: - Repository ID in format 'namespace/space-name'. + Repository ID in format 'username/env_name'. """ - # Use space_name if provided, otherwise use env_name - repo_name = space_name if space_name else env_name - - if namespace: - return f"{namespace}/{repo_name}" - # Use authenticated user's username username, _ = ensure_authenticated() - return f"{username}/{repo_name}" + return f"{username}/{env_name}" diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index d94c2a05..de2f67db 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -74,12 +74,12 @@ def test_push_environment_full_workflow( mock_prepare_staging.return_value = Path("staging") # Run push - push_environment("test_env", namespace=None, private=False) + push_environment("test_env", private=False) # Verify calls mock_validate.assert_called_once_with("test_env") mock_ensure_auth.assert_called_once() - mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name=None) + mock_get_repo_id.assert_called_once_with("test_env") mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) mock_prepare_staging.assert_called_once() mock_copy_files.assert_called_once() @@ -153,7 +153,7 @@ def test_push_environment_auth_failure(self, mock_ensure_auth, mock_validate, mo @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") - def test_push_environment_with_namespace( + def test_push_environment_with_repo_id( self, mock_api_class, mock_get_repo_id, @@ -167,19 +167,19 @@ def test_push_environment_with_namespace( mock_upload, mock_environment, ): - """Test push with explicit namespace.""" + """Test push with explicit repo_id.""" # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") - mock_get_repo_id.return_value = "my-org/test_env" mock_prepare_staging.return_value = Path("staging") - # Run push with namespace - push_environment("test_env", namespace="my-org") + # Run push with repo_id (should not call get_space_repo_id) + push_environment("test_env", repo_id="my-org/test_env") - # Verify namespace was used - mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org", space_name=None) + # Verify repo_id was used directly (get_space_repo_id should not be called) + mock_get_repo_id.assert_not_called() + mock_create_space.assert_called_once_with(mock_api, "my-org/test_env", private=False) @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") @@ -229,7 +229,7 @@ def test_push_environment_private( @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") - def test_push_environment_with_space_name( + def test_push_environment_with_repo_id_custom_space_name( self, mock_api_class, mock_get_repo_id, @@ -243,56 +243,16 @@ def test_push_environment_with_space_name( mock_upload, mock_environment, ): - """Test push with custom space name.""" + """Test push with repo_id that has custom space name.""" # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api mock_ensure_auth.return_value = ("test_user", "test_token") - mock_get_repo_id.return_value = "test_user/custom-space" mock_prepare_staging.return_value = Path("staging") - # Run push with space_name - push_environment("test_env", space_name="custom-space") + # Run push with repo_id containing custom space name + push_environment("test_env", repo_id="my-org/custom-space") - # Verify space_name was used - mock_get_repo_id.assert_called_once_with("test_env", namespace=None, space_name="custom-space") - mock_create_space.assert_called_once_with(mock_api, "test_user/custom-space", private=False) - - @patch("openenv_cli.commands.push.upload_to_space") - @patch("openenv_cli.commands.push.create_space") - @patch("openenv_cli.commands.push.prepare_readme") - @patch("openenv_cli.commands.push.prepare_dockerfile") - @patch("openenv_cli.commands.push.copy_environment_files") - @patch("openenv_cli.commands.push.prepare_staging_directory") - @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") - @patch("openenv_cli.commands.push.get_space_repo_id") - @patch("openenv_cli.commands.push.HfApi") - def test_push_environment_with_namespace_and_space_name( - self, - mock_api_class, - mock_get_repo_id, - mock_ensure_auth, - mock_validate, - mock_prepare_staging, - mock_copy_files, - mock_prepare_dockerfile, - mock_prepare_readme, - mock_create_space, - mock_upload, - mock_environment, - ): - """Test push with both namespace and space name.""" - # Setup mocks - mock_api = Mock() - mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") - mock_get_repo_id.return_value = "my-org/custom-space" - mock_prepare_staging.return_value = Path("staging") - - # Run push with namespace and space_name - push_environment("test_env", namespace="my-org", space_name="custom-space") - - # Verify both namespace and space_name were used - mock_get_repo_id.assert_called_once_with("test_env", namespace="my-org", space_name="custom-space") + # Verify repo_id was used directly (get_space_repo_id should not be called) + mock_get_repo_id.assert_not_called() mock_create_space.assert_called_once_with(mock_api, "my-org/custom-space", private=False) diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py index eab47e18..b4ab4947 100644 --- a/tests/test_cli/test_space.py +++ b/tests/test_cli/test_space.py @@ -93,44 +93,20 @@ class TestGetSpaceRepoId: """Tests for get_space_repo_id function.""" @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_with_namespace(self, mock_auth): - """Test get_space_repo_id with explicit namespace.""" - mock_auth.return_value = ("user", "token") - - result = get_space_repo_id("my-env", namespace="my-org") - - assert result == "my-org/my-env" - - @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_no_namespace(self, mock_auth): - """Test get_space_repo_id without namespace uses authenticated user.""" + def test_get_space_repo_id_uses_authenticated_user(self, mock_auth): + """Test get_space_repo_id uses authenticated user's username.""" mock_auth.return_value = ("test_user", "token") result = get_space_repo_id("my-env") assert result == "test_user/my-env" + mock_auth.assert_called_once() @patch("openenv_cli.core.space.ensure_authenticated") def test_get_space_repo_id_env_name_with_underscore(self, mock_auth): """Test get_space_repo_id handles env names with underscores.""" mock_auth.return_value = ("test_user", "token") - result = get_space_repo_id("my_env", namespace="my-org") - - assert result == "my-org/my_env" - - @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_with_space_name(self, mock_auth): - """Test get_space_repo_id with custom space name.""" - mock_auth.return_value = ("test_user", "token") - - result = get_space_repo_id("my_env", space_name="custom-space") - - assert result == "test_user/custom-space" - - @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_with_namespace_and_space_name(self, mock_auth): - """Test get_space_repo_id with both namespace and space name.""" - result = get_space_repo_id("my_env", namespace="my-org", space_name="custom-space") + result = get_space_repo_id("my_env") - assert result == "my-org/custom-space" + assert result == "test_user/my_env" From 02a417487e4cdffb83f01d0fa7ce2e93366371cf Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 20:56:44 -0500 Subject: [PATCH 26/38] Refactor `push` command to improve environment preparation and upload process. --- src/openenv_cli/__main__.py | 46 +++++++++++++++++---- src/openenv_cli/commands/push.py | 70 ++++++++++++++++++++++++++++++++ src/openenv_cli/core/auth.py | 3 ++ 3 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index 5bef19a8..c2a1c95c 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -64,21 +64,49 @@ def main(): if args.command == "push": try: # Authenticate first (before status spinner) to allow interactive login if needed - console.print(f"[bold cyan]Authenticating...[/bold cyan]") + # Note: login() may print ASCII art to stdout - we clean up after username, token = ensure_authenticated() + # Print a newline to separate login output from our status message + console.print() # Clean separator after login output if args.dry_run: status_message = f"[bold yellow]Preparing dry run for '{args.env_name}'...[/bold yellow]" + with console.status(status_message): + push_environment( + env_name=args.env_name, + repo_id=args.repo_id, + private=args.private, + base_image=args.base_image, + dry_run=args.dry_run, + ) else: - status_message = f"[bold cyan]Pushing environment '{args.env_name}'...[/bold cyan]" - - with console.status(status_message): - push_environment( + # Use status spinner for preparation steps + with console.status(f"[bold cyan]Preparing '{args.env_name}'...[/bold cyan]"): + from openenv_cli.commands.push import _prepare_environment + staging_dir = _prepare_environment( + env_name=args.env_name, + repo_id=args.repo_id, + private=args.private, + base_image=args.base_image, + username=username, + token=token, + ) + + # Determine repo_id for upload + if args.repo_id is None: + from openenv_cli.core.space import get_space_repo_id + repo_id = get_space_repo_id(args.env_name) + else: + repo_id = args.repo_id + + # Upload without spinner so messages from huggingface_hub appear cleanly + from openenv_cli.commands.push import _upload_environment + _upload_environment( env_name=args.env_name, - repo_id=args.repo_id, - private=args.private, - base_image=args.base_image, - dry_run=args.dry_run, + repo_id=repo_id, + staging_dir=staging_dir, + username=username, + token=token, ) if args.dry_run: diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index 0e6dbcb7..af47f32f 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -6,6 +6,7 @@ """Push command for deploying environments to Hugging Face Spaces.""" +from pathlib import Path from typing import Optional from huggingface_hub import HfApi @@ -82,3 +83,72 @@ def push_environment( if staging_dir.exists(): import shutil shutil.rmtree(staging_dir) + + +def _prepare_environment( + env_name: str, + repo_id: Optional[str], + private: bool, + base_image: Optional[str], + username: str, + token: str, +) -> Path: + """ + Internal function to prepare environment staging directory. + + Returns: + Path to staging directory (must be cleaned up by caller). + """ + # Validate environment exists + validate_environment(env_name) + + # Determine target space repo ID + if repo_id is None: + repo_id = get_space_repo_id(env_name) + + # Create HfApi instance + api = HfApi(token=token) + + # Check if space exists, create if needed + create_space(api, repo_id, private=private) + + # Set default base image if not provided + if base_image is None: + base_image = "ghcr.io/meta-pytorch/openenv-base:latest" + + # Prepare staging directory + staging_dir = prepare_staging_directory(env_name, base_image) + + # Copy files + copy_environment_files(env_name, staging_dir) + + # Prepare Dockerfile + prepare_dockerfile(env_name, staging_dir, base_image) + + # Prepare README + prepare_readme(env_name, staging_dir) + + return staging_dir + + +def _upload_environment( + env_name: str, + repo_id: str, + staging_dir: Path, + username: str, + token: str, +) -> None: + """ + Internal function to upload environment staging directory. + + The staging directory will be cleaned up after upload. + """ + api = HfApi(token=token) + + try: + upload_to_space(api, repo_id, staging_dir, token) + finally: + # Cleanup staging directory after upload + if staging_dir.exists(): + import shutil + shutil.rmtree(staging_dir) diff --git a/src/openenv_cli/core/auth.py b/src/openenv_cli/core/auth.py index 1552502a..06802d13 100644 --- a/src/openenv_cli/core/auth.py +++ b/src/openenv_cli/core/auth.py @@ -78,4 +78,7 @@ def login_interactive() -> str: raise Exception("Login failed: unable to verify authentication") return username except Exception as e: + # Re-raise our custom exceptions, wrap others + if isinstance(e, Exception) and str(e).startswith("Login failed"): + raise raise Exception(f"Login failed: {str(e)}") From 3c0c4323cf9c847aaf13f6ca118d88fe4a5131af Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 21:01:28 -0500 Subject: [PATCH 27/38] Add openenv_cli package and update build workflow - Added openenv_cli as a dependency in pyproject.toml. - Included openenv_cli scripts for easier access. - Updated package directory structure to include openenv_cli. - Added a verification step in the GitHub Actions workflow to check for the openenv_cli directory before building the package. --- .github/workflows/publish-pypi-core.yml | 2 ++ src/core/pyproject.toml | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish-pypi-core.yml b/.github/workflows/publish-pypi-core.yml index 4c1e8911..b11dd8e8 100644 --- a/.github/workflows/publish-pypi-core.yml +++ b/.github/workflows/publish-pypi-core.yml @@ -36,6 +36,8 @@ jobs: - name: Build package run: | cd src/core + # Verify openenv_cli is accessible + ls -la ../openenv_cli || echo "Warning: openenv_cli directory not found" python -m build - name: Check package diff --git a/src/core/pyproject.toml b/src/core/pyproject.toml index 32602f58..c0e2eb41 100644 --- a/src/core/pyproject.toml +++ b/src/core/pyproject.toml @@ -18,8 +18,13 @@ dependencies = [ "requests>=2.25.0", "fastapi>=0.104.0", "uvicorn>=0.24.0", + "huggingface_hub>=0.20.0", + "rich>=13.0.0", ] +[project.scripts] +openenv = "openenv_cli.__main__:main" + [project.optional-dependencies] dev = [ "pytest>=7.0.0", @@ -41,6 +46,10 @@ packages = [ "openenv_core.containers", "openenv_core.containers.runtime", "openenv_core.env_server", - "openenv_core.tools" + "openenv_core.tools", + "openenv_cli", + "openenv_cli.commands", + "openenv_cli.core", + "openenv_cli.utils", ] -package-dir = {"openenv_core" = "."} +package-dir = {"openenv_core" = ".", "openenv_cli" = "../openenv_cli"} From 9756cf9764ac3d1084540ec4205b94de48175fed Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 22:08:35 -0500 Subject: [PATCH 28/38] All user interaction happens in __main__ following HF hub approach - Introduced Typer for a more structured command-line interface, enhancing user experience. - Added `_cli_utils.py` for shared utilities, including a `typer_factory` for consistent app creation. - Updated `push` command to handle authentication and environment pushing more effectively. - Refactored authentication logic in `auth.py` to use an `AuthStatus` class for better state management. - Improved README to reflect new CLI structure and usage patterns. --- src/core/pyproject.toml | 1 + src/openenv_cli/README.md | 87 ++++++++++++++----- src/openenv_cli/__main__.py | 130 +++++----------------------- src/openenv_cli/_cli_utils.py | 36 ++++++++ src/openenv_cli/commands/push.py | 129 +++++++++++++++++++++++++-- src/openenv_cli/core/auth.py | 101 +++++++++++++-------- src/openenv_cli/core/space.py | 9 +- tests/test_cli/test_auth.py | 91 +++++++++++++------ tests/test_cli/test_push_command.py | 47 ++++------ tests/test_cli/test_space.py | 17 ++-- 10 files changed, 400 insertions(+), 248 deletions(-) create mode 100644 src/openenv_cli/_cli_utils.py diff --git a/src/core/pyproject.toml b/src/core/pyproject.toml index c0e2eb41..a7e48b7f 100644 --- a/src/core/pyproject.toml +++ b/src/core/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ "uvicorn>=0.24.0", "huggingface_hub>=0.20.0", "rich>=13.0.0", + "typer>=0.12.0", ] [project.scripts] diff --git a/src/openenv_cli/README.md b/src/openenv_cli/README.md index 2ee50c36..16b66fd5 100644 --- a/src/openenv_cli/README.md +++ b/src/openenv_cli/README.md @@ -75,32 +75,52 @@ pytest --cov=openenv_cli --cov-report=html tests/test_cli/ ## Architecture -The CLI is organized into modular components: +The CLI follows the HuggingFace Hub CLI pattern, using `typer` for command-line interface definition. All user interaction is centralized in command handlers, while business logic remains in pure functions. + +### Structure ``` src/openenv_cli/ -├── __main__.py # CLI entry point +├── __main__.py # CLI entry point (creates typer app, registers commands) +├── _cli_utils.py # CLI utilities (console, typer_factory) ├── commands/ -│ └── push.py # Push command implementation +│ └── push.py # Push command (typer command with all UI interaction) ├── core/ -│ ├── auth.py # Hugging Face authentication -│ ├── space.py # Space management (create/check) -│ ├── builder.py # Build staging directory and files -│ └── uploader.py # Upload to Hugging Face Spaces +│ ├── auth.py # Hugging Face authentication (pure functions) +│ ├── space.py # Space management (pure functions) +│ ├── builder.py # Build staging directory and files (pure functions) +│ └── uploader.py # Upload to Hugging Face Spaces (pure functions) └── utils/ - └── env_loader.py # Environment validation and metadata + └── env_loader.py # Environment validation and metadata (pure functions) ``` ### Module Responsibilities -- **`auth.py`**: Handles Hugging Face authentication via interactive login using `huggingface_hub` -- **`space.py`**: Manages Docker Spaces (creates with `exist_ok=True` to handle existing spaces) -- **`builder.py`**: Prepares deployment package (copy files, generate Dockerfile/README) -- **`uploader.py`**: Uploads files to Hugging Face using `upload_folder` -- **`env_loader.py`**: Validates environment structure and loads metadata +- **`__main__.py`**: Creates typer app and registers commands. Minimal logic, delegates to command handlers. +- **`_cli_utils.py`**: Provides `typer_factory()` for consistent app creation and shared `console` instance. +- **`commands/push.py`**: `push_command()` function decorated with `@app.command()` - handles all UI interaction (authentication prompts, status messages, error handling). +- **`core/auth.py`**: Pure authentication functions returning status objects (no UI). +- **`core/space.py`**: Pure functions for space management (no UI). +- **`core/builder.py`**: Pure functions for preparing deployment files (no UI). +- **`core/uploader.py`**: Pure function for uploading files (no UI). +- **`utils/env_loader.py`**: Pure functions for environment validation and metadata (no UI). ## Implementation Details +### CLI Framework: Typer + +The CLI uses [Typer](https://typer.timascio.com/) for command-line interface definition, following the same pattern as HuggingFace Hub's CLI: + +- **`typer_factory()`**: Creates typer apps with consistent settings +- **Command handlers**: Functions decorated with `@app.command()` handle all UI interaction +- **Type hints**: Uses `Annotated` types for better CLI argument definitions +- **Rich integration**: Typer integrates with Rich for beautiful terminal output + +This pattern ensures: +- Clean separation: All UI in command handlers, business logic in pure functions +- Better testability: Business logic can be tested without mocking UI +- Consistent experience: Same patterns as HuggingFace Hub CLI + ### Using huggingface_hub Modules The CLI uses `huggingface_hub` Python modules directly (not the CLI), as specified: @@ -205,24 +225,47 @@ openenv validate my_env The CLI architecture supports extension through: -1. **Command Registration**: Add new commands in `commands/` and register in `__main__.py` -2. **Core Modules**: Add new core functionality (e.g., `core/validator.py`) +1. **Command Registration**: Add new typer commands in `commands/` and register in `__main__.py` +2. **Core Modules**: Add new core functionality (e.g., `core/validator.py`) as pure functions 3. **Shared Utilities**: Extend `utils/` for reusable functions +4. **CLI Utilities**: Use `_cli_utils.py` for shared console and typer factory ### Example: Adding a New Command +Following the typer pattern: + ```python # src/openenv_cli/commands/validate.py -def validate_environment(env_name: str) -> None: +from typing import Annotated +import typer + +from .._cli_utils import console + +def validate_command( + env_name: Annotated[ + str, + typer.Argument(help="Name of the environment to validate"), + ], +) -> None: """Validate environment structure.""" - # Implementation - pass + # All UI interaction here + console.print(f"[bold cyan]Validating '{env_name}'...[/bold cyan]") + + # Call pure business logic functions + from ..core.validator import validate_environment + result = validate_environment(env_name) + + if result.is_valid: + console.print(f"[bold green]✓ Environment '{env_name}' is valid[/bold green]") + else: + console.print(f"[bold red]✗ Validation failed: {result.errors}[/bold red]") + sys.exit(1) # src/openenv_cli/__main__.py -# Add to subparsers: -validate_parser = subparsers.add_parser("validate", help="Validate environment") -validate_parser.add_argument("env_name") -# ... handle command +from .commands.validate import validate_command + +# Register command +app.command(name="validate", help="Validate environment structure")(validate_command) ``` ## Contributing diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index c2a1c95c..4561e136 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -6,122 +6,36 @@ """CLI entry point for OpenEnv.""" -import argparse import sys +from typing import Annotated, Optional -from rich.console import Console -from rich.traceback import install +import typer -from .commands.push import push_environment -from .core.auth import ensure_authenticated +from ._cli_utils import console, typer_factory +from .commands.push import push +# Create main typer app +app = typer_factory(help="OpenEnv CLI - Manage and deploy OpenEnv environments") -console = Console() -install(show_locals=False) +# Add callback to prevent single command from becoming implicit/default +@app.callback(invoke_without_command=True) +def main_callback() -> None: + """OpenEnv CLI - Manage and deploy OpenEnv environments.""" + pass +# Register top-level commands (following HF Hub pattern for simple commands) +app.command(name="push", help="Push an environment to Hugging Face Spaces.")(push) -def main(): - """Main entry point for OpenEnv CLI.""" - parser = argparse.ArgumentParser( - prog="openenv", - description="OpenEnv CLI - Manage and deploy OpenEnv environments", - ) - - subparsers = parser.add_subparsers(dest="command", help="Command to run") - - # Push command - push_parser = subparsers.add_parser( - "push", - help="Push an environment to Hugging Face Spaces", - ) - push_parser.add_argument( - "env_name", - help="Name of the environment to push (e.g., echo_env)", - ) - push_parser.add_argument( - "--repo-id", - help="Hugging Face repository ID in format 'namespace/space-name'. " - "If not provided, uses '{username}/{env_name}'.", - ) - push_parser.add_argument( - "--private", - action="store_true", - help="Create a private space (default: public)", - ) - push_parser.add_argument( - "--base-image", - help="Base Docker image to use " - "(default: ghcr.io/meta-pytorch/openenv-base:latest)", - ) - push_parser.add_argument( - "--dry-run", - action="store_true", - help="Prepare files but don't upload to Hugging Face", - ) - - args = parser.parse_args() - - if args.command == "push": - try: - # Authenticate first (before status spinner) to allow interactive login if needed - # Note: login() may print ASCII art to stdout - we clean up after - username, token = ensure_authenticated() - # Print a newline to separate login output from our status message - console.print() # Clean separator after login output - - if args.dry_run: - status_message = f"[bold yellow]Preparing dry run for '{args.env_name}'...[/bold yellow]" - with console.status(status_message): - push_environment( - env_name=args.env_name, - repo_id=args.repo_id, - private=args.private, - base_image=args.base_image, - dry_run=args.dry_run, - ) - else: - # Use status spinner for preparation steps - with console.status(f"[bold cyan]Preparing '{args.env_name}'...[/bold cyan]"): - from openenv_cli.commands.push import _prepare_environment - staging_dir = _prepare_environment( - env_name=args.env_name, - repo_id=args.repo_id, - private=args.private, - base_image=args.base_image, - username=username, - token=token, - ) - - # Determine repo_id for upload - if args.repo_id is None: - from openenv_cli.core.space import get_space_repo_id - repo_id = get_space_repo_id(args.env_name) - else: - repo_id = args.repo_id - - # Upload without spinner so messages from huggingface_hub appear cleanly - from openenv_cli.commands.push import _upload_environment - _upload_environment( - env_name=args.env_name, - repo_id=repo_id, - staging_dir=staging_dir, - username=username, - token=token, - ) - if args.dry_run: - console.print( - f"[bold yellow]Dry run complete for '{args.env_name}'.[/bold yellow]" - ) - else: - console.print( - f"[bold green]Successfully pushed '{args.env_name}'.[/bold green]" - ) - except Exception as e: - console.print(f"[bold red]Error:[/bold red] {e}", highlight=False, soft_wrap=True) - sys.exit(1) - else: - console.print(parser.format_help()) +def main() -> None: + """Main entry point for OpenEnv CLI.""" + try: + app() + except KeyboardInterrupt: + console.print("\n[bold yellow]Operation cancelled.[/bold yellow]") + sys.exit(130) # Standard exit code for SIGINT + except Exception as e: + console.print(f"[bold red]Error:[/bold red] {e}", highlight=False, soft_wrap=True) sys.exit(1) diff --git a/src/openenv_cli/_cli_utils.py b/src/openenv_cli/_cli_utils.py new file mode 100644 index 00000000..87f603d2 --- /dev/null +++ b/src/openenv_cli/_cli_utils.py @@ -0,0 +1,36 @@ +# 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. + +"""CLI utilities and helpers.""" + +import typer +from rich.console import Console +from rich.traceback import install + +# Initialize console and install rich traceback handler +console = Console() +install(show_locals=False) + + +def typer_factory(help: str) -> typer.Typer: + """ + Create a Typer app with consistent settings. + + Args: + help: Help text for the app. + + Returns: + Configured Typer app instance. + """ + return typer.Typer( + help=help, + add_completion=True, + no_args_is_help=True, + # Use rich for better formatting + rich_markup_mode="rich", + pretty_exceptions_show_locals=False, + ) + diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index af47f32f..637ca4e1 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -6,12 +6,16 @@ """Push command for deploying environments to Hugging Face Spaces.""" +import sys from pathlib import Path -from typing import Optional +from typing import Annotated, Optional + +import typer from huggingface_hub import HfApi -from ..core.auth import ensure_authenticated +from .._cli_utils import console, typer_factory +from ..core.auth import check_auth_status, perform_login from ..core.builder import ( prepare_staging_directory, copy_environment_files, @@ -23,8 +27,119 @@ from ..utils.env_loader import validate_environment +# Push command function (following HF Hub pattern for top-level commands like upload/download) +def push( + env_name: Annotated[ + str, + typer.Argument(help="Name of the environment to push (e.g., echo_env)"), + ], + repo_id: Annotated[ + Optional[str], + typer.Option( + "--repo-id", + help="Hugging Face repository ID in format 'namespace/space-name'. " + "If not provided, uses '{username}/{env_name}'.", + ), + ] = None, + private: Annotated[ + bool, + typer.Option("--private", help="Create a private space (default: public)"), + ] = False, + base_image: Annotated[ + Optional[str], + typer.Option( + "--base-image", + help="Base Docker image to use (default: ghcr.io/meta-pytorch/openenv-base:latest)", + ), + ] = None, + dry_run: Annotated[ + bool, + typer.Option("--dry-run", help="Prepare files but don't upload to Hugging Face"), + ] = False, +) -> None: + """ + Push an environment to Hugging Face Spaces. + + This command prepares and uploads an OpenEnv environment to Hugging Face Spaces, + handling authentication, space creation, file preparation, and deployment. + """ + try: + # Handle authentication (all UI here) + console.print("[bold cyan]Authenticating...[/bold cyan]", end=" ") + auth_status = check_auth_status() + + if not auth_status.is_authenticated: + # User needs to login - perform login (this will trigger prompts from huggingface_hub) + # Note: login() prints ASCII art and prompts - this is expected behavior + try: + auth_status = perform_login() + console.print(f"[bold green]✓ Authenticated as {auth_status.username}[/bold green]") + except Exception as e: + console.print(f"[bold red]Error:[/bold red] Login failed: {str(e)}") + sys.exit(1) + else: + # Already authenticated + console.print(f"[bold green]✓ Authenticated as {auth_status.username}[/bold green]") + + username, token = auth_status.get_credentials() + + # Print a newline to separate authentication from workflow messages + console.print() + + if dry_run: + status_message = f"[bold yellow]Preparing dry run for '{env_name}'...[/bold yellow]" + with console.status(status_message): + push_environment( + env_name=env_name, + username=username, + token=token, + repo_id=repo_id, + private=private, + base_image=base_image, + dry_run=dry_run, + ) + else: + # Use status spinner for preparation steps + with console.status(f"[bold cyan]Preparing '{env_name}'...[/bold cyan]"): + staging_dir = _prepare_environment( + env_name=env_name, + repo_id=repo_id, + private=private, + base_image=base_image, + username=username, + token=token, + ) + + # Determine repo_id for upload + if repo_id is None: + repo_id = get_space_repo_id(env_name, username) + + # Upload without spinner so messages from huggingface_hub appear cleanly + _upload_environment( + env_name=env_name, + repo_id=repo_id, + staging_dir=staging_dir, + username=username, + token=token, + ) + + if dry_run: + console.print( + f"[bold yellow]Dry run complete for '{env_name}'.[/bold yellow]" + ) + else: + console.print( + f"[bold green]Successfully pushed '{env_name}'.[/bold green]" + ) + except Exception as e: + console.print(f"[bold red]Error:[/bold red] {e}", highlight=False, soft_wrap=True) + sys.exit(1) + + def push_environment( env_name: str, + username: str, + token: str, repo_id: Optional[str] = None, private: bool = False, base_image: Optional[str] = None, @@ -35,6 +150,8 @@ def push_environment( Args: env_name: Name of the environment to push. + username: Authenticated username. + token: Authentication token. repo_id: Optional repository ID in format 'namespace/space-name'. If not provided, uses '{username}/{env_name}'. private: Whether the space should be private (default: False). @@ -44,13 +161,9 @@ def push_environment( # Validate environment exists validate_environment(env_name) - # Get token (authentication should already be done in __main__, but get token for API) - # ensure_authenticated is idempotent - if already authenticated, it returns immediately - username, token = ensure_authenticated() - # Determine target space repo ID if repo_id is None: - repo_id = get_space_repo_id(env_name) + repo_id = get_space_repo_id(env_name, username) # Create HfApi instance api = HfApi(token=token) @@ -104,7 +217,7 @@ def _prepare_environment( # Determine target space repo ID if repo_id is None: - repo_id = get_space_repo_id(env_name) + repo_id = get_space_repo_id(env_name, username) # Create HfApi instance api = HfApi(token=token) diff --git a/src/openenv_cli/core/auth.py b/src/openenv_cli/core/auth.py index 06802d13..433744a7 100644 --- a/src/openenv_cli/core/auth.py +++ b/src/openenv_cli/core/auth.py @@ -6,66 +6,61 @@ """Authentication module for Hugging Face.""" +from dataclasses import dataclass from typing import Optional, Tuple from huggingface_hub import HfApi, login from huggingface_hub.utils import get_token -def check_authentication() -> Optional[str]: +@dataclass +class AuthStatus: + """Authentication status.""" + is_authenticated: bool + username: Optional[str] = None + token: Optional[str] = None + + def get_credentials(self) -> Tuple[str, str]: + """Get username and token, raising if not authenticated.""" + if not self.is_authenticated or not self.username or not self.token: + raise Exception("Not authenticated") + return self.username, self.token + + +def check_auth_status() -> AuthStatus: """ Check if user is authenticated with Hugging Face. Returns: - Username if authenticated, None otherwise. + AuthStatus object indicating authentication state. """ # Get token from stored credentials (from previous login) token = get_token() if not token: - return None + return AuthStatus(is_authenticated=False) try: api = HfApi(token=token) user_info = api.whoami() - return user_info.get("name") + username = user_info.get("name") + if username: + return AuthStatus(is_authenticated=True, username=username, token=token) + return AuthStatus(is_authenticated=False) except Exception: # Invalid token or network error - return None - - -def ensure_authenticated() -> Tuple[str, str]: - """ - Ensure user is authenticated, prompting for login if needed. - - Returns: - Tuple of (username, token). - - Raises: - Exception: If authentication fails. - """ - # Check existing authentication (from stored credentials) - username = check_authentication() - if username: - token = get_token() - if token: - return username, token - - # Need to login - username = login_interactive() - token = get_token() - if not token: - raise Exception("Failed to retrieve token after login") - - return username, token + return AuthStatus(is_authenticated=False) -def login_interactive() -> str: +def perform_login() -> AuthStatus: """ Perform interactive login to Hugging Face. + Note: This function will trigger interactive prompts (handled by huggingface_hub.login()). + The caller should handle UI/prompts appropriately. + Returns: - Username after successful login. + AuthStatus after login attempt. Raises: Exception: If login fails. @@ -73,12 +68,46 @@ def login_interactive() -> str: try: login() # Verify login was successful - username = check_authentication() - if not username: + status = check_auth_status() + if not status.is_authenticated: raise Exception("Login failed: unable to verify authentication") - return username + return status except Exception as e: # Re-raise our custom exceptions, wrap others if isinstance(e, Exception) and str(e).startswith("Login failed"): raise raise Exception(f"Login failed: {str(e)}") + + +# Legacy functions for backward compatibility during refactoring +def check_authentication() -> Optional[str]: + """ + Check if user is authenticated with Hugging Face. + + Returns: + Username if authenticated, None otherwise. + """ + status = check_auth_status() + return status.username if status.is_authenticated else None + + +def ensure_authenticated() -> Tuple[str, str]: + """ + Ensure user is authenticated, prompting for login if needed. + + Note: This function triggers interactive prompts. For better separation of concerns, + consider using check_auth_status() and perform_login() separately. + + Returns: + Tuple of (username, token). + + Raises: + Exception: If authentication fails. + """ + status = check_auth_status() + if status.is_authenticated: + return status.get_credentials() + + # Need to login + status = perform_login() + return status.get_credentials() diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index 4aaacc46..cbd5ddc2 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -10,8 +10,6 @@ from huggingface_hub import HfApi -from .auth import ensure_authenticated - def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: """ @@ -58,16 +56,15 @@ def create_space(api: HfApi, repo_id: str, private: bool = False) -> None: ) from e -def get_space_repo_id(env_name: str) -> str: +def get_space_repo_id(env_name: str, username: str) -> str: """ - Get the full repository ID for a space using the authenticated user's username. + Get the full repository ID for a space using the provided username. Args: env_name: Environment name (e.g., "echo_env"). Used as space name. + username: Username to use for the repository ID. Returns: Repository ID in format 'username/env_name'. """ - # Use authenticated user's username - username, _ = ensure_authenticated() return f"{username}/{env_name}" diff --git a/tests/test_cli/test_auth.py b/tests/test_cli/test_auth.py index e0277629..41e70072 100644 --- a/tests/test_cli/test_auth.py +++ b/tests/test_cli/test_auth.py @@ -10,7 +10,13 @@ import pytest -from openenv_cli.core.auth import check_authentication, ensure_authenticated, login_interactive +from openenv_cli.core.auth import ( + check_authentication, + check_auth_status, + ensure_authenticated, + perform_login, + AuthStatus, +) @pytest.fixture @@ -69,60 +75,95 @@ def test_check_authentication_invalid_token(self, mock_get_token, mock_api_class class TestEnsureAuthenticated: - """Tests for ensure_authenticated function.""" + """Tests for ensure_authenticated function (legacy compatibility).""" - @patch("openenv_cli.core.auth.check_authentication") + @patch("openenv_cli.core.auth.check_auth_status") @patch("openenv_cli.core.auth.get_token") - def test_ensure_authenticated_already_authenticated(self, mock_get_token, mock_check): + def test_ensure_authenticated_already_authenticated(self, mock_get_token, mock_check_status): """Test ensure_authenticated when already authenticated.""" - mock_check.return_value = "test_user" + mock_check_status.return_value = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) mock_get_token.return_value = "test_token" username, token = ensure_authenticated() assert username == "test_user" assert token == "test_token" - mock_check.assert_called_once() + mock_check_status.assert_called_once() - @patch("openenv_cli.core.auth.check_authentication") - @patch("openenv_cli.core.auth.login_interactive") + @patch("openenv_cli.core.auth.perform_login") + @patch("openenv_cli.core.auth.check_auth_status") @patch("openenv_cli.core.auth.get_token") - def test_ensure_authenticated_needs_login(self, mock_get_token, mock_login, mock_check): + def test_ensure_authenticated_needs_login(self, mock_get_token, mock_check_status, mock_perform_login): """Test ensure_authenticated when login is needed.""" - mock_check.return_value = None - mock_login.return_value = "new_user" + mock_check_status.return_value = AuthStatus(is_authenticated=False) + mock_perform_login.return_value = AuthStatus( + is_authenticated=True, username="new_user", token="new_token" + ) mock_get_token.return_value = "new_token" username, token = ensure_authenticated() assert username == "new_user" assert token == "new_token" - mock_login.assert_called_once() + mock_perform_login.assert_called_once() -class TestLoginInteractive: - """Tests for login_interactive function.""" +class TestCheckAuthStatus: + """Tests for check_auth_status function.""" - @patch("openenv_cli.core.auth.login") @patch("openenv_cli.core.auth.HfApi") @patch("openenv_cli.core.auth.get_token") - def test_login_interactive_success(self, mock_get_token, mock_api_class, mock_login): - """Test successful interactive login.""" - mock_login.return_value = None # login doesn't return anything + def test_check_auth_status_authenticated(self, mock_get_token, mock_api_class): + """Test check_auth_status when authenticated.""" + mock_get_token.return_value = "test_token" mock_api = Mock() - mock_api.whoami.return_value = {"name": "logged_in_user"} + mock_api.whoami.return_value = {"name": "test_user"} mock_api_class.return_value = mock_api - mock_get_token.return_value = "new_token" - username = login_interactive() + status = check_auth_status() + + assert status.is_authenticated is True + assert status.username == "test_user" + assert status.token == "test_token" + mock_api.whoami.assert_called_once() + + @patch("openenv_cli.core.auth.get_token") + def test_check_auth_status_no_token(self, mock_get_token): + """Test check_auth_status when no token exists.""" + mock_get_token.return_value = None + + status = check_auth_status() + + assert status.is_authenticated is False + assert status.username is None + assert status.token is None + + +class TestPerformLogin: + """Tests for perform_login function.""" + + @patch("openenv_cli.core.auth.check_auth_status") + @patch("openenv_cli.core.auth.login") + def test_perform_login_success(self, mock_login, mock_check_auth): + """Test successful login.""" + mock_login.return_value = None # login doesn't return anything + mock_check_auth.return_value = AuthStatus( + is_authenticated=True, username="logged_in_user", token="new_token" + ) + + status = perform_login() - assert username == "logged_in_user" + assert status.is_authenticated is True + assert status.username == "logged_in_user" mock_login.assert_called_once() + mock_check_auth.assert_called_once() @patch("openenv_cli.core.auth.login") - def test_login_interactive_failure(self, mock_login): - """Test interactive login failure.""" + def test_perform_login_failure(self, mock_login): + """Test login failure.""" mock_login.side_effect = Exception("Login failed") with pytest.raises(Exception, match="Login failed"): - login_interactive() + perform_login() diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index de2f67db..e2497f87 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -48,14 +48,12 @@ class TestPushEnvironment: @patch("openenv_cli.commands.push.copy_environment_files") @patch("openenv_cli.commands.push.prepare_staging_directory") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") def test_push_environment_full_workflow( self, mock_api_class, mock_get_repo_id, - mock_ensure_auth, mock_validate, mock_prepare_staging, mock_copy_files, @@ -69,17 +67,15 @@ def test_push_environment_full_workflow( # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" mock_prepare_staging.return_value = Path("staging") - # Run push - push_environment("test_env", private=False) + # Run push with credentials + push_environment("test_env", username="test_user", token="test_token", private=False) # Verify calls mock_validate.assert_called_once_with("test_env") - mock_ensure_auth.assert_called_once() - mock_get_repo_id.assert_called_once_with("test_env") + mock_get_repo_id.assert_called_once_with("test_env", "test_user") mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) mock_prepare_staging.assert_called_once() mock_copy_files.assert_called_once() @@ -94,14 +90,12 @@ def test_push_environment_full_workflow( @patch("openenv_cli.commands.push.copy_environment_files") @patch("openenv_cli.commands.push.prepare_staging_directory") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") def test_push_environment_space_exists( self, mock_api_class, mock_get_repo_id, - mock_ensure_auth, mock_validate, mock_prepare_staging, mock_copy_files, @@ -115,12 +109,11 @@ def test_push_environment_space_exists( # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" mock_prepare_staging.return_value = Path("staging") - # Run push - push_environment("test_env") + # Run push with credentials + push_environment("test_env", username="test_user", token="test_token") # Verify create_space was called (it handles existing spaces internally) mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) @@ -131,17 +124,18 @@ def test_push_environment_invalid_env(self, mock_validate, mock_environment): mock_validate.side_effect = FileNotFoundError("Environment not found") with pytest.raises(FileNotFoundError, match="Environment not found"): - push_environment("invalid_env") + push_environment("invalid_env", username="test_user", token="test_token") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") - def test_push_environment_auth_failure(self, mock_ensure_auth, mock_validate, mock_environment): - """Test push when authentication fails.""" + def test_push_environment_auth_failure(self, mock_validate, mock_environment): + """Test push when authentication fails (now handled in __main__.py).""" + # Note: Authentication failures are now handled in __main__.py + # This test verifies validation works mock_validate.return_value = Path("src/envs/test_env") - mock_ensure_auth.side_effect = Exception("Authentication failed") - with pytest.raises(Exception, match="Authentication failed"): - push_environment("test_env") + # push_environment should succeed if credentials are provided + # Authentication failures would happen before calling push_environment + pass @patch("openenv_cli.commands.push.upload_to_space") @patch("openenv_cli.commands.push.create_space") @@ -150,14 +144,12 @@ def test_push_environment_auth_failure(self, mock_ensure_auth, mock_validate, mo @patch("openenv_cli.commands.push.copy_environment_files") @patch("openenv_cli.commands.push.prepare_staging_directory") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") def test_push_environment_with_repo_id( self, mock_api_class, mock_get_repo_id, - mock_ensure_auth, mock_validate, mock_prepare_staging, mock_copy_files, @@ -171,11 +163,10 @@ def test_push_environment_with_repo_id( # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") mock_prepare_staging.return_value = Path("staging") # Run push with repo_id (should not call get_space_repo_id) - push_environment("test_env", repo_id="my-org/test_env") + push_environment("test_env", username="test_user", token="test_token", repo_id="my-org/test_env") # Verify repo_id was used directly (get_space_repo_id should not be called) mock_get_repo_id.assert_not_called() @@ -188,14 +179,12 @@ def test_push_environment_with_repo_id( @patch("openenv_cli.commands.push.copy_environment_files") @patch("openenv_cli.commands.push.prepare_staging_directory") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") def test_push_environment_private( self, mock_api_class, mock_get_repo_id, - mock_ensure_auth, mock_validate, mock_prepare_staging, mock_copy_files, @@ -209,12 +198,11 @@ def test_push_environment_private( # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") mock_get_repo_id.return_value = "test_user/test_env" mock_prepare_staging.return_value = Path("staging") # Run push with private flag - push_environment("test_env", private=True) + push_environment("test_env", username="test_user", token="test_token", private=True) # Verify private space was created mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=True) @@ -226,14 +214,12 @@ def test_push_environment_private( @patch("openenv_cli.commands.push.copy_environment_files") @patch("openenv_cli.commands.push.prepare_staging_directory") @patch("openenv_cli.commands.push.validate_environment") - @patch("openenv_cli.commands.push.ensure_authenticated") @patch("openenv_cli.commands.push.get_space_repo_id") @patch("openenv_cli.commands.push.HfApi") def test_push_environment_with_repo_id_custom_space_name( self, mock_api_class, mock_get_repo_id, - mock_ensure_auth, mock_validate, mock_prepare_staging, mock_copy_files, @@ -247,11 +233,10 @@ def test_push_environment_with_repo_id_custom_space_name( # Setup mocks mock_api = Mock() mock_api_class.return_value = mock_api - mock_ensure_auth.return_value = ("test_user", "test_token") mock_prepare_staging.return_value = Path("staging") # Run push with repo_id containing custom space name - push_environment("test_env", repo_id="my-org/custom-space") + push_environment("test_env", username="test_user", token="test_token", repo_id="my-org/custom-space") # Verify repo_id was used directly (get_space_repo_id should not be called) mock_get_repo_id.assert_not_called() diff --git a/tests/test_cli/test_space.py b/tests/test_cli/test_space.py index b4ab4947..bd5ea2d0 100644 --- a/tests/test_cli/test_space.py +++ b/tests/test_cli/test_space.py @@ -92,21 +92,14 @@ def test_create_space_generic_error(self, mock_hf_api): class TestGetSpaceRepoId: """Tests for get_space_repo_id function.""" - @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_uses_authenticated_user(self, mock_auth): - """Test get_space_repo_id uses authenticated user's username.""" - mock_auth.return_value = ("test_user", "token") - - result = get_space_repo_id("my-env") + def test_get_space_repo_id_uses_provided_username(self): + """Test get_space_repo_id uses provided username.""" + result = get_space_repo_id("my-env", "test_user") assert result == "test_user/my-env" - mock_auth.assert_called_once() - @patch("openenv_cli.core.space.ensure_authenticated") - def test_get_space_repo_id_env_name_with_underscore(self, mock_auth): + def test_get_space_repo_id_env_name_with_underscore(self): """Test get_space_repo_id handles env names with underscores.""" - mock_auth.return_value = ("test_user", "token") - - result = get_space_repo_id("my_env") + result = get_space_repo_id("my_env", "test_user") assert result == "test_user/my_env" From 6d1ee8d686aa9c91bfe99bb910b47c215667a5af Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 22:21:35 -0500 Subject: [PATCH 29/38] Fixing nits, various refactors - Removed unused imports and type annotations in `__main__.py`, `_cli_utils.py`, and `space.py`. - Simplified the `upload_to_space` function by eliminating the `HfApi` parameter, streamlining the upload process. - Updated tests in `test_uploader.py` to reflect changes in the `upload_to_space` function, ensuring proper functionality without the `HfApi` dependency. --- src/openenv_cli/__main__.py | 3 --- src/openenv_cli/_cli_utils.py | 1 - src/openenv_cli/commands/push.py | 6 +++--- src/openenv_cli/core/space.py | 2 -- src/openenv_cli/core/uploader.py | 3 --- src/openenv_cli/utils/env_loader.py | 2 +- tests/test_cli/test_uploader.py | 24 +++++++++--------------- 7 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/openenv_cli/__main__.py b/src/openenv_cli/__main__.py index 4561e136..8b0e65c0 100644 --- a/src/openenv_cli/__main__.py +++ b/src/openenv_cli/__main__.py @@ -7,9 +7,6 @@ """CLI entry point for OpenEnv.""" import sys -from typing import Annotated, Optional - -import typer from ._cli_utils import console, typer_factory from .commands.push import push diff --git a/src/openenv_cli/_cli_utils.py b/src/openenv_cli/_cli_utils.py index 87f603d2..9c2de397 100644 --- a/src/openenv_cli/_cli_utils.py +++ b/src/openenv_cli/_cli_utils.py @@ -29,7 +29,6 @@ def typer_factory(help: str) -> typer.Typer: help=help, add_completion=True, no_args_is_help=True, - # Use rich for better formatting rich_markup_mode="rich", pretty_exceptions_show_locals=False, ) diff --git a/src/openenv_cli/commands/push.py b/src/openenv_cli/commands/push.py index 637ca4e1..e96a5815 100644 --- a/src/openenv_cli/commands/push.py +++ b/src/openenv_cli/commands/push.py @@ -14,7 +14,7 @@ from huggingface_hub import HfApi -from .._cli_utils import console, typer_factory +from .._cli_utils import console from ..core.auth import check_auth_status, perform_login from ..core.builder import ( prepare_staging_directory, @@ -189,7 +189,7 @@ def push_environment( # Upload to space (skip if dry run) if not dry_run: - upload_to_space(api, repo_id, staging_dir, token) + upload_to_space(repo_id, staging_dir, token) finally: # Cleanup staging directory after upload or dry run @@ -259,7 +259,7 @@ def _upload_environment( api = HfApi(token=token) try: - upload_to_space(api, repo_id, staging_dir, token) + upload_to_space(repo_id, staging_dir, token) finally: # Cleanup staging directory after upload if staging_dir.exists(): diff --git a/src/openenv_cli/core/space.py b/src/openenv_cli/core/space.py index cbd5ddc2..7daec5e9 100644 --- a/src/openenv_cli/core/space.py +++ b/src/openenv_cli/core/space.py @@ -6,8 +6,6 @@ """Space management module for Hugging Face Spaces.""" -from typing import Optional - from huggingface_hub import HfApi diff --git a/src/openenv_cli/core/uploader.py b/src/openenv_cli/core/uploader.py index 28f7a4a7..ff56510b 100644 --- a/src/openenv_cli/core/uploader.py +++ b/src/openenv_cli/core/uploader.py @@ -8,12 +8,10 @@ from pathlib import Path -from huggingface_hub import HfApi from huggingface_hub import upload_folder def upload_to_space( - api: HfApi, repo_id: str, staging_dir: Path, token: str, @@ -22,7 +20,6 @@ def upload_to_space( Upload staging directory contents to Hugging Face Space. Args: - api: HfApi instance to use for API calls. repo_id: Repository ID in format 'namespace/repo-name'. staging_dir: Path to staging directory to upload. token: Hugging Face token for authentication. diff --git a/src/openenv_cli/utils/env_loader.py b/src/openenv_cli/utils/env_loader.py index 11b1ef4c..d68bd9dc 100644 --- a/src/openenv_cli/utils/env_loader.py +++ b/src/openenv_cli/utils/env_loader.py @@ -7,7 +7,7 @@ """Environment loader utilities.""" from pathlib import Path -from typing import Dict, Any, Optional +from typing import Dict, Any def validate_environment(env_name: str) -> Path: diff --git a/tests/test_cli/test_uploader.py b/tests/test_cli/test_uploader.py index ea5e5df4..7afb60e7 100644 --- a/tests/test_cli/test_uploader.py +++ b/tests/test_cli/test_uploader.py @@ -6,19 +6,13 @@ """Tests for uploader module.""" -from unittest.mock import Mock, patch +from unittest.mock import patch import pytest from openenv_cli.core.uploader import upload_to_space -@pytest.fixture -def mock_hf_api(): - """Mock HfApi for testing.""" - return Mock() - - @pytest.fixture def staging_dir(tmp_path): """Create a staging directory with test files.""" @@ -38,11 +32,11 @@ class TestUploadToSpace: """Tests for upload_to_space function.""" @patch("openenv_cli.core.uploader.upload_folder") - def test_upload_to_space_uses_upload_folder(self, mock_upload, mock_hf_api, staging_dir): + def test_upload_to_space_uses_upload_folder(self, mock_upload, staging_dir): """Test that upload_to_space uses upload_folder for bulk upload.""" mock_upload.return_value = None - upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + upload_to_space("test_user/my-env", staging_dir, "test_token") mock_upload.assert_called_once() # Check that the call was made with correct parameters @@ -53,27 +47,27 @@ def test_upload_to_space_uses_upload_folder(self, mock_upload, mock_hf_api, stag assert call_args[1]["token"] == "test_token" @patch("openenv_cli.core.uploader.upload_folder") - def test_upload_to_space_handles_errors(self, mock_upload, mock_hf_api, staging_dir): + def test_upload_to_space_handles_errors(self, mock_upload, staging_dir): """Test that upload_to_space handles upload errors.""" mock_upload.side_effect = Exception("Upload failed") with pytest.raises(Exception, match="Upload failed"): - upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + upload_to_space("test_user/my-env", staging_dir, "test_token") @patch("openenv_cli.core.uploader.upload_folder") - def test_upload_to_space_empty_directory(self, mock_upload, mock_hf_api, tmp_path): + def test_upload_to_space_empty_directory(self, mock_upload, tmp_path): """Test uploading an empty directory.""" empty_dir = tmp_path / "empty" empty_dir.mkdir() mock_upload.return_value = None - upload_to_space(mock_hf_api, "test_user/my-env", empty_dir, "test_token") + upload_to_space("test_user/my-env", empty_dir, "test_token") mock_upload.assert_called_once() @patch("openenv_cli.core.uploader.upload_folder") - def test_upload_to_space_nested_files(self, mock_upload, mock_hf_api, staging_dir): + def test_upload_to_space_nested_files(self, mock_upload, staging_dir): """Test uploading directory with nested files.""" # Create nested structure (staging_dir / "src" / "envs" / "test_env").mkdir(parents=True) @@ -81,6 +75,6 @@ def test_upload_to_space_nested_files(self, mock_upload, mock_hf_api, staging_di mock_upload.return_value = None - upload_to_space(mock_hf_api, "test_user/my-env", staging_dir, "test_token") + upload_to_space("test_user/my-env", staging_dir, "test_token") mock_upload.assert_called_once() From 50240d3a9fdc8414726768f3269792371deaf8fe Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 22:24:45 -0500 Subject: [PATCH 30/38] Remove outdated authentication section from README.md --- README.md | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/README.md b/README.md index cded2b56..d8ec5a79 100644 --- a/README.md +++ b/README.md @@ -194,19 +194,6 @@ openenv push echo_env --base-image ghcr.io/my-org/custom-base:latest openenv push echo_env --dry-run ``` -#### Authentication - -The CLI uses Hugging Face authentication via interactive login. When you run a command that requires authentication, the CLI will prompt you to log in: - -```bash -# The CLI will automatically prompt for login when needed -openenv push echo_env -``` - -The login process will: -1. Open your browser to authenticate with Hugging Face -2. Store your credentials for future use (from `huggingface_hub`) - #### How It Works The `openenv push` command performs the following steps: From 37db23ff4d3bf7f0c94d47b53a197d43baa84185 Mon Sep 17 00:00:00 2001 From: Zach Wentz Date: Sun, 2 Nov 2025 22:39:10 -0500 Subject: [PATCH 31/38] Add comprehensive tests for CLI utilities and environment loader - Introduced tests for the `typer_factory` function in `test_cli_utils.py` to ensure proper Typer app creation and settings. - Added tests for environment loading functionalities in `test_env_loader.py`, covering validation and metadata loading for environments. - Enhanced authentication tests in `test_auth.py` to cover various failure scenarios during login and status checks. - Expanded tests for the `push` command in `test_push_command.py`, including error handling and dry run scenarios. - Implemented staging directory preparation tests in `test_builder.py` to verify cleanup and directory creation logic. --- tests/test_cli/test_auth.py | 84 +++++- tests/test_cli/test_builder.py | 85 ++++++ tests/test_cli/test_cli_utils.py | 32 +++ tests/test_cli/test_env_loader.py | 165 +++++++++++ tests/test_cli/test_main.py | 53 ++++ tests/test_cli/test_push_command.py | 432 +++++++++++++++++++++++++++- 6 files changed, 849 insertions(+), 2 deletions(-) create mode 100644 tests/test_cli/test_cli_utils.py create mode 100644 tests/test_cli/test_env_loader.py create mode 100644 tests/test_cli/test_main.py diff --git a/tests/test_cli/test_auth.py b/tests/test_cli/test_auth.py index 41e70072..1dd4f44b 100644 --- a/tests/test_cli/test_auth.py +++ b/tests/test_cli/test_auth.py @@ -164,6 +164,88 @@ def test_perform_login_success(self, mock_login, mock_check_auth): def test_perform_login_failure(self, mock_login): """Test login failure.""" mock_login.side_effect = Exception("Login failed") - + with pytest.raises(Exception, match="Login failed"): perform_login() + + @patch("openenv_cli.core.auth.check_auth_status") + @patch("openenv_cli.core.auth.login") + def test_perform_login_verification_fails(self, mock_login, mock_check_auth): + """Test login when verification fails.""" + mock_login.return_value = None + mock_check_auth.return_value = AuthStatus(is_authenticated=False) + + with pytest.raises(Exception, match="Login failed: unable to verify"): + perform_login() + + @patch("openenv_cli.core.auth.check_auth_status") + @patch("openenv_cli.core.auth.login") + def test_perform_login_verification_exception_wrapped(self, mock_login, mock_check_auth): + """Test login when verification raises exception.""" + mock_login.side_effect = ValueError("Network error") + + with pytest.raises(Exception, match="Login failed: Network error"): + perform_login() + + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_check_auth_status_no_username(self, mock_get_token, mock_api_class): + """Test check_auth_status when whoami returns no username.""" + mock_get_token.return_value = "test_token" + mock_api = Mock() + mock_api.whoami.return_value = {} # No "name" key + mock_api_class.return_value = mock_api + + status = check_auth_status() + + assert status.is_authenticated is False + assert status.username is None + + @patch("openenv_cli.core.auth.HfApi") + @patch("openenv_cli.core.auth.get_token") + def test_check_auth_status_api_exception(self, mock_get_token, mock_api_class): + """Test check_auth_status when API call raises exception.""" + mock_get_token.return_value = "test_token" + mock_api = Mock() + mock_api.whoami.side_effect = Exception("API error") + mock_api_class.return_value = mock_api + + status = check_auth_status() + + assert status.is_authenticated is False + + +class TestAuthStatus: + """Tests for AuthStatus dataclass.""" + + def test_get_credentials_success(self): + """Test get_credentials when authenticated.""" + status = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) + + username, token = status.get_credentials() + + assert username == "test_user" + assert token == "test_token" + + def test_get_credentials_not_authenticated(self): + """Test get_credentials when not authenticated.""" + status = AuthStatus(is_authenticated=False) + + with pytest.raises(Exception, match="Not authenticated"): + status.get_credentials() + + def test_get_credentials_missing_username(self): + """Test get_credentials when username is missing.""" + status = AuthStatus(is_authenticated=True, username=None, token="test_token") + + with pytest.raises(Exception, match="Not authenticated"): + status.get_credentials() + + def test_get_credentials_missing_token(self): + """Test get_credentials when token is missing.""" + status = AuthStatus(is_authenticated=True, username="test_user", token=None) + + with pytest.raises(Exception, match="Not authenticated"): + status.get_credentials() diff --git a/tests/test_cli/test_builder.py b/tests/test_cli/test_builder.py index 8b5ddd32..5d4af4e6 100644 --- a/tests/test_cli/test_builder.py +++ b/tests/test_cli/test_builder.py @@ -7,6 +7,7 @@ """Tests for builder module.""" import os +from pathlib import Path from unittest.mock import patch import pytest @@ -79,6 +80,22 @@ def test_prepare_staging_directory_creates_structure(self, mock_path, staging_di # This will be tested through integration with copy functions pass + def test_prepare_staging_directory_removes_existing(self, tmp_path, monkeypatch): + """Test that prepare_staging_directory removes existing directory.""" + from openenv_cli.core.builder import prepare_staging_directory + + staging_root = tmp_path / "hf-staging" + staging_dir = staging_root / "test_env" + staging_dir.mkdir(parents=True) + (staging_dir / "old_file.txt").write_text("old content") + + # Should remove and recreate + result = prepare_staging_directory("test_env", "test:latest", str(staging_root)) + + assert result.exists() + assert not (result / "old_file.txt").exists() + assert (result / "src" / "core").exists() + class TestCopyEnvironmentFiles: """Tests for copy_environment_files function.""" @@ -440,3 +457,71 @@ def test_prepare_readme_appends_original_without_front_matter(self, repo_root, t assert content.count("# Test Environment") == 1 finally: os.chdir(old_cwd) + + +class TestCopyEnvironmentFilesErrorCases: + """Tests for copy_environment_files error cases.""" + + def test_copy_environment_files_env_not_found(self, repo_root, tmp_path, monkeypatch): + """Test copy_environment_files when environment doesn't exist.""" + from openenv_cli.core.builder import copy_environment_files, prepare_staging_directory + + staging_dir = prepare_staging_directory("nonexistent", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + with pytest.raises(FileNotFoundError, match="Environment not found"): + copy_environment_files("nonexistent", staging_dir) + finally: + os.chdir(old_cwd) + + +class TestPrepareDockerfileDefault: + """Tests for prepare_dockerfile default creation.""" + + def test_prepare_dockerfile_creates_default(self, tmp_path, monkeypatch): + """Test that default Dockerfile is created when env has no Dockerfile.""" + from openenv_cli.core.builder import prepare_dockerfile, prepare_staging_directory + + # Create a fresh repo root without using repo_root fixture (which includes Dockerfile) + repo_root = tmp_path / "repo" + env_dir = repo_root / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + (env_dir / "models.py").write_text("# Test") + # Don't create server/Dockerfile - this should trigger default creation + + # Create core directory + core_dir = repo_root / "src" / "core" + core_dir.mkdir(parents=True) + (core_dir / "__init__.py").write_text("# Core") + + staging_dir = prepare_staging_directory("test_env", "test:latest", str(tmp_path / "hf-staging")) + + old_cwd = os.getcwd() + try: + os.chdir(repo_root) + base_image = "ghcr.io/meta-pytorch/openenv-base:latest" + + # Verify Dockerfile doesn't exist before + env_dockerfile = Path("src/envs") / "test_env" / "server" / "Dockerfile" + assert not env_dockerfile.exists(), "Dockerfile should not exist for this test" + + prepare_dockerfile("test_env", staging_dir, base_image) + + dockerfile_path = staging_dir / "Dockerfile" + assert dockerfile_path.exists() + + content = dockerfile_path.read_text() + # Check for default Dockerfile structure (lines 101-121) + # Default template includes HEALTHCHECK, FROM, COPY, CMD + assert f"FROM {base_image}" in content + assert "COPY src/core/" in content + assert "COPY src/envs/test_env/" in content + # HEALTHCHECK should be in default template + assert "HEALTHCHECK" in content + # ENABLE_WEB_INTERFACE is added after default template (line 129) + assert "ENV ENABLE_WEB_INTERFACE=true" in content + assert "CMD [\"uvicorn\", \"envs.test_env.server.app:app\"" in content + finally: + os.chdir(old_cwd) diff --git a/tests/test_cli/test_cli_utils.py b/tests/test_cli/test_cli_utils.py new file mode 100644 index 00000000..dca06095 --- /dev/null +++ b/tests/test_cli/test_cli_utils.py @@ -0,0 +1,32 @@ +# 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 CLI utilities.""" + +import typer + +from openenv_cli._cli_utils import typer_factory + + +class TestTyperFactory: + """Tests for typer_factory function.""" + + def test_typer_factory_returns_typer_app(self): + """Test that typer_factory returns a Typer app.""" + app = typer_factory("Test help text") + + assert isinstance(app, typer.Typer) + assert app.info.help == "Test help text" + + def test_typer_factory_creates_app_with_settings(self): + """Test that typer_factory creates app with correct settings.""" + app = typer_factory("Test help") + + # Verify it's a Typer instance with expected configuration + assert isinstance(app, typer.Typer) + # The app should have the help text + assert app.info.help == "Test help" + diff --git a/tests/test_cli/test_env_loader.py b/tests/test_cli/test_env_loader.py new file mode 100644 index 00000000..d0b75ec7 --- /dev/null +++ b/tests/test_cli/test_env_loader.py @@ -0,0 +1,165 @@ +# 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 environment loader utilities.""" + +from pathlib import Path + +import pytest + +from openenv_cli.utils.env_loader import load_env_metadata, validate_environment + + +class TestValidateEnvironment: + """Tests for validate_environment function.""" + + def test_validate_environment_success(self, tmp_path, monkeypatch): + """Test validate_environment with valid environment.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + + monkeypatch.chdir(tmp_path) + + result = validate_environment("test_env") + + # Result is relative path, but should exist and be a directory + assert result == Path("src/envs/test_env") + assert result.exists() + assert result.is_dir() + + def test_validate_environment_not_found(self, tmp_path, monkeypatch): + """Test validate_environment when environment doesn't exist.""" + monkeypatch.chdir(tmp_path) + + with pytest.raises(FileNotFoundError, match="Environment 'missing_env' not found"): + validate_environment("missing_env") + + def test_validate_environment_not_directory(self, tmp_path, monkeypatch): + """Test validate_environment when path exists but is not a directory.""" + env_file = tmp_path / "src" / "envs" / "test_env" + env_file.parent.mkdir(parents=True) + env_file.write_text("not a directory") + + monkeypatch.chdir(tmp_path) + + with pytest.raises(FileNotFoundError, match="Environment 'test_env' is not a directory"): + validate_environment("test_env") + + +class TestLoadEnvMetadata: + """Tests for load_env_metadata function.""" + + def test_load_env_metadata_basic(self, tmp_path, monkeypatch): + """Test load_env_metadata with basic environment.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["name"] == "test_env" + # Path is returned as relative string + assert metadata["path"] == "src/envs/test_env" + assert "readme" not in metadata + assert "has_server" not in metadata + + def test_load_env_metadata_with_readme(self, tmp_path, monkeypatch): + """Test load_env_metadata with README.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + readme = env_dir / "README.md" + readme.write_text("# Test Environment\n\nThis is a test.") + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["name"] == "test_env" + assert "readme" in metadata + assert metadata["readme"] == "# Test Environment\n\nThis is a test." + assert metadata["title"] == "Test Environment" + + def test_load_env_metadata_with_server(self, tmp_path, monkeypatch): + """Test load_env_metadata with server directory.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + server_dir = env_dir / "server" + server_dir.mkdir(parents=True) + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["has_server"] is True + assert metadata.get("has_dockerfile") is None + + def test_load_env_metadata_with_dockerfile(self, tmp_path, monkeypatch): + """Test load_env_metadata with Dockerfile.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + server_dir = env_dir / "server" + server_dir.mkdir(parents=True) + dockerfile = server_dir / "Dockerfile" + dockerfile.write_text("FROM test:latest") + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["has_server"] is True + assert metadata["has_dockerfile"] is True + # Path is returned as relative string + assert metadata["dockerfile_path"] == "src/envs/test_env/server/Dockerfile" + + def test_load_env_metadata_with_models(self, tmp_path, monkeypatch): + """Test load_env_metadata with models.py.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + models = env_dir / "models.py" + models.write_text("# Models") + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["has_models"] is True + + def test_load_env_metadata_with_client(self, tmp_path, monkeypatch): + """Test load_env_metadata with client.py.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + env_dir.mkdir(parents=True) + client = env_dir / "client.py" + client.write_text("# Client") + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["has_client"] is True + + def test_load_env_metadata_full(self, tmp_path, monkeypatch): + """Test load_env_metadata with all components.""" + env_dir = tmp_path / "src" / "envs" / "test_env" + server_dir = env_dir / "server" + server_dir.mkdir(parents=True) + + # Create all files + (env_dir / "README.md").write_text("# Full Environment\n\nComplete setup.") + (env_dir / "models.py").write_text("# Models") + (env_dir / "client.py").write_text("# Client") + (server_dir / "Dockerfile").write_text("FROM test:latest") + + monkeypatch.chdir(tmp_path) + + metadata = load_env_metadata("test_env") + + assert metadata["name"] == "test_env" + assert "readme" in metadata + assert metadata["title"] == "Full Environment" + assert metadata["has_server"] is True + assert metadata["has_dockerfile"] is True + assert metadata["has_models"] is True + assert metadata["has_client"] is True + diff --git a/tests/test_cli/test_main.py b/tests/test_cli/test_main.py new file mode 100644 index 00000000..2fde3093 --- /dev/null +++ b/tests/test_cli/test_main.py @@ -0,0 +1,53 @@ +# 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 __main__.py CLI entry point.""" + +import sys +from unittest.mock import patch + +import pytest + +from openenv_cli.__main__ import main + + +class TestMain: + """Tests for main() function.""" + + @patch("openenv_cli.__main__.app") + def test_main_success(self, mock_app): + """Test main() when app runs successfully.""" + mock_app.return_value = None + + # Should not raise + main() + + mock_app.assert_called_once() + + @patch("openenv_cli.__main__.console") + @patch("openenv_cli.__main__.app") + @patch("sys.exit") + def test_main_keyboard_interrupt(self, mock_exit, mock_app, mock_console): + """Test main() handling KeyboardInterrupt.""" + mock_app.side_effect = KeyboardInterrupt() + + main() + + mock_console.print.assert_called_once() + mock_exit.assert_called_once_with(130) + + @patch("openenv_cli.__main__.console") + @patch("openenv_cli.__main__.app") + @patch("sys.exit") + def test_main_exception(self, mock_exit, mock_app, mock_console): + """Test main() handling general Exception.""" + mock_app.side_effect = Exception("Test error") + + main() + + mock_console.print.assert_called_once() + mock_exit.assert_called_once_with(1) + diff --git a/tests/test_cli/test_push_command.py b/tests/test_cli/test_push_command.py index e2497f87..dbd47d83 100644 --- a/tests/test_cli/test_push_command.py +++ b/tests/test_cli/test_push_command.py @@ -11,7 +11,12 @@ import pytest -from openenv_cli.commands.push import push_environment +from openenv_cli.commands.push import ( + _prepare_environment, + _upload_environment, + push, + push_environment, +) @pytest.fixture @@ -241,3 +246,428 @@ def test_push_environment_with_repo_id_custom_space_name( # Verify repo_id was used directly (get_space_repo_id should not be called) mock_get_repo_id.assert_not_called() mock_create_space.assert_called_once_with(mock_api, "my-org/custom-space", private=False) + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_dry_run( + self, + mock_api_class, + mock_get_repo_id, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test push with dry_run=True (should not upload).""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_get_repo_id.return_value = "test_user/test_env" + staging_dir = mock_environment / "staging" + staging_dir.mkdir() + mock_prepare_staging.return_value = staging_dir + + # Run push with dry_run=True + push_environment( + "test_env", + username="test_user", + token="test_token", + dry_run=True, + ) + + # Verify upload was NOT called (dry run) + mock_upload.assert_not_called() + # Verify cleanup happened + assert not staging_dir.exists() + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_push_environment_cleanup_on_error( + self, + mock_api_class, + mock_get_repo_id, + mock_validate, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_create_space, + mock_upload, + mock_environment, + ): + """Test that staging directory is cleaned up even on upload error.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_get_repo_id.return_value = "test_user/test_env" + staging_dir = mock_environment / "staging" + staging_dir.mkdir() + mock_prepare_staging.return_value = staging_dir + mock_upload.side_effect = Exception("Upload failed") + + # Run push and expect error + with pytest.raises(Exception, match="Upload failed"): + push_environment("test_env", username="test_user", token="test_token") + + # Verify cleanup happened even after error + assert not staging_dir.exists() + + +class TestPrepareEnvironment: + """Tests for _prepare_environment function.""" + + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_prepare_environment_full( + self, + mock_api_class, + mock_get_repo_id, + mock_validate, + mock_create_space, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_environment, + ): + """Test _prepare_environment with all steps.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + mock_get_repo_id.return_value = "test_user/test_env" + staging_dir = Path("staging") + mock_prepare_staging.return_value = staging_dir + + # Run prepare + result = _prepare_environment( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + username="test_user", + token="test_token", + ) + + # Verify calls + mock_validate.assert_called_once_with("test_env") + mock_get_repo_id.assert_called_once_with("test_env", "test_user") + mock_create_space.assert_called_once_with(mock_api, "test_user/test_env", private=False) + mock_prepare_staging.assert_called_once() + mock_copy_files.assert_called_once() + mock_prepare_dockerfile.assert_called_once() + mock_prepare_readme.assert_called_once() + assert result == staging_dir + + @patch("openenv_cli.commands.push.prepare_readme") + @patch("openenv_cli.commands.push.prepare_dockerfile") + @patch("openenv_cli.commands.push.copy_environment_files") + @patch("openenv_cli.commands.push.prepare_staging_directory") + @patch("openenv_cli.commands.push.create_space") + @patch("openenv_cli.commands.push.validate_environment") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.HfApi") + def test_prepare_environment_with_repo_id( + self, + mock_api_class, + mock_get_repo_id, + mock_validate, + mock_create_space, + mock_prepare_staging, + mock_copy_files, + mock_prepare_dockerfile, + mock_prepare_readme, + mock_environment, + ): + """Test _prepare_environment with explicit repo_id.""" + # Setup mocks + mock_api = Mock() + mock_api_class.return_value = mock_api + staging_dir = Path("staging") + mock_prepare_staging.return_value = staging_dir + + # Run prepare with repo_id + result = _prepare_environment( + env_name="test_env", + repo_id="my-org/test_env", + private=True, + base_image="custom:latest", + username="test_user", + token="test_token", + ) + + # Verify get_space_repo_id was NOT called + mock_get_repo_id.assert_not_called() + mock_create_space.assert_called_once_with(mock_api, "my-org/test_env", private=True) + mock_prepare_staging.assert_called_once_with("test_env", "custom:latest") + assert result == staging_dir + + +class TestUploadEnvironment: + """Tests for _upload_environment function.""" + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.HfApi") + def test_upload_environment_success(self, mock_api_class, mock_upload, tmp_path): + """Test _upload_environment successfully uploads.""" + # Setup + mock_api = Mock() + mock_api_class.return_value = mock_api + staging_dir = tmp_path / "staging" + staging_dir.mkdir() + (staging_dir / "test.txt").write_text("test") + + # Run upload + _upload_environment( + env_name="test_env", + repo_id="test_user/test_env", + staging_dir=staging_dir, + username="test_user", + token="test_token", + ) + + # Verify upload was called and cleanup happened + mock_upload.assert_called_once_with("test_user/test_env", staging_dir, "test_token") + assert not staging_dir.exists() + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.HfApi") + def test_upload_environment_cleanup_on_error(self, mock_api_class, mock_upload, tmp_path): + """Test _upload_environment cleans up even on upload error.""" + # Setup + mock_api = Mock() + mock_api_class.return_value = mock_api + staging_dir = tmp_path / "staging" + staging_dir.mkdir() + (staging_dir / "test.txt").write_text("test") + mock_upload.side_effect = Exception("Upload failed") + + # Run upload and expect error + with pytest.raises(Exception, match="Upload failed"): + _upload_environment( + env_name="test_env", + repo_id="test_user/test_env", + staging_dir=staging_dir, + username="test_user", + token="test_token", + ) + + # Verify cleanup happened even after error + assert not staging_dir.exists() + + @patch("openenv_cli.commands.push.upload_to_space") + @patch("openenv_cli.commands.push.HfApi") + def test_upload_environment_nonexistent_staging_dir(self, mock_api_class, mock_upload, tmp_path): + """Test _upload_environment handles nonexistent staging directory.""" + # Setup + mock_api = Mock() + mock_api_class.return_value = mock_api + staging_dir = tmp_path / "nonexistent" + + # Run upload + _upload_environment( + env_name="test_env", + repo_id="test_user/test_env", + staging_dir=staging_dir, + username="test_user", + token="test_token", + ) + + # Verify upload was called (cleanup won't fail on nonexistent dir) + mock_upload.assert_called_once_with("test_user/test_env", staging_dir, "test_token") + + +class TestPushCommand: + """Tests for push() typer command function.""" + + @patch("openenv_cli.commands.push.push_environment") + @patch("openenv_cli.commands.push.check_auth_status") + def test_push_command_already_authenticated( + self, + mock_check_auth, + mock_push_env, + mock_environment, + ): + """Test push command when already authenticated.""" + from openenv_cli.core.auth import AuthStatus + + # Setup + mock_check_auth.return_value = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) + + # Run command + push( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + dry_run=True, + ) + + # Verify + mock_check_auth.assert_called_once() + mock_push_env.assert_called_once_with( + env_name="test_env", + username="test_user", + token="test_token", + repo_id=None, + private=False, + base_image=None, + dry_run=True, + ) + + @patch("openenv_cli.commands.push.perform_login") + @patch("openenv_cli.commands.push.check_auth_status") + @patch("sys.exit") + def test_push_command_needs_login( + self, + mock_exit, + mock_check_auth, + mock_perform_login, + mock_environment, + ): + """Test push command when login is needed.""" + from openenv_cli.core.auth import AuthStatus + + # Setup + mock_check_auth.return_value = AuthStatus(is_authenticated=False) + mock_perform_login.return_value = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) + + # Run command + push( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + dry_run=True, + ) + + # Verify login was called + mock_check_auth.assert_called_once() + mock_perform_login.assert_called_once() + + @patch("openenv_cli.commands.push.perform_login") + @patch("openenv_cli.commands.push.check_auth_status") + def test_push_command_login_failure( + self, + mock_check_auth, + mock_perform_login, + mock_environment, + ): + """Test push command when login fails.""" + from openenv_cli.core.auth import AuthStatus + + # Setup + mock_check_auth.return_value = AuthStatus(is_authenticated=False) + mock_perform_login.side_effect = Exception("Login failed") + + # Run command (should exit with SystemExit) + with pytest.raises(SystemExit) as exc_info: + push( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + dry_run=True, + ) + + # Verify exit code + assert exc_info.value.code == 1 + + @patch("openenv_cli.commands.push._upload_environment") + @patch("openenv_cli.commands.push._prepare_environment") + @patch("openenv_cli.commands.push.get_space_repo_id") + @patch("openenv_cli.commands.push.check_auth_status") + def test_push_command_non_dry_run( + self, + mock_check_auth, + mock_get_repo_id, + mock_prepare, + mock_upload, + mock_environment, + ): + """Test push command with dry_run=False (full workflow).""" + from openenv_cli.core.auth import AuthStatus + + # Setup + mock_check_auth.return_value = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) + mock_get_repo_id.return_value = "test_user/test_env" + staging_dir = Path("staging") + mock_prepare.return_value = staging_dir + + # Run command + push( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + dry_run=False, + ) + + # Verify + mock_prepare.assert_called_once() + mock_get_repo_id.assert_called_once_with("test_env", "test_user") + mock_upload.assert_called_once_with( + env_name="test_env", + repo_id="test_user/test_env", + staging_dir=staging_dir, + username="test_user", + token="test_token", + ) + + @patch("openenv_cli.commands.push.push_environment") + @patch("openenv_cli.commands.push.check_auth_status") + def test_push_command_error_handling( + self, + mock_check_auth, + mock_push_env, + mock_environment, + ): + """Test push command error handling.""" + from openenv_cli.core.auth import AuthStatus + + # Setup + mock_check_auth.return_value = AuthStatus( + is_authenticated=True, username="test_user", token="test_token" + ) + mock_push_env.side_effect = Exception("Test error") + + # Run command (should exit with error) + with pytest.raises(SystemExit) as exc_info: + push( + env_name="test_env", + repo_id=None, + private=False, + base_image=None, + dry_run=True, + ) + + assert exc_info.value.code == 1 From 4298bd6c19217fb4496b5d199952f5025d2012a6 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:39:34 +0100 Subject: [PATCH 32/38] use cli in action and update comment --- .github/workflows/pr-new-env.yml | 67 +++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pr-new-env.yml b/.github/workflows/pr-new-env.yml index 1374abf7..955f9913 100644 --- a/.github/workflows/pr-new-env.yml +++ b/.github/workflows/pr-new-env.yml @@ -103,6 +103,7 @@ jobs: environment: ${{ fromJSON(needs.detect-new-envs.outputs.new_envs_json) }} env: HF_TOKEN: ${{ secrets.HF_PR_TOKEN }} + HUGGINGFACEHUB_API_TOKEN: ${{ secrets.HF_PR_TOKEN }} HF_NAMESPACE: ${{ vars.HF_PR_NAMESPACE }} SPACE_SUFFIX: -pr-${{ github.event.number }} steps: @@ -127,18 +128,24 @@ jobs: exit 1 fi - - name: Install Hugging Face CLI + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Install OpenEnv package shell: bash run: | - curl -LsSf https://hf.co/cli/install.sh | bash - echo "$HOME/.local/bin" >> "$GITHUB_PATH" + set -euo pipefail + python -m pip install --upgrade pip + pip install . - - name: Deploy environment to Hugging Face + - name: Deploy environment with OpenEnv CLI shell: bash run: | set -euo pipefail - chmod +x scripts/deploy_to_hf.sh - ./scripts/deploy_to_hf.sh --env "${{ matrix.environment }}" --space-suffix "${SPACE_SUFFIX}" --hub-tag "openenv-pr" + repo_id="${HF_NAMESPACE}/${{ matrix.environment }}${SPACE_SUFFIX}" + openenv push "${{ matrix.environment }}" --repo-id "$repo_id" - name: Wait for deployment to stabilize shell: bash @@ -161,6 +168,7 @@ jobs: health_url="https://${namespace_slug}-${space_slug}.hf.space/health" live_url="https://${namespace_slug}-${space_slug}.hf.space" space_repo_url="https://huggingface.co/spaces/${HF_NAMESPACE}/${space_name}" + space_repo_id="${HF_NAMESPACE}/${space_name}" echo "namespace_slug=${namespace_slug}" >> "$GITHUB_OUTPUT" echo "space_name=${space_name}" >> "$GITHUB_OUTPUT" @@ -168,6 +176,7 @@ jobs: echo "health_url=${health_url}" >> "$GITHUB_OUTPUT" echo "live_url=${live_url}" >> "$GITHUB_OUTPUT" echo "space_repo_url=${space_repo_url}" >> "$GITHUB_OUTPUT" + echo "space_repo_id=${space_repo_id}" >> "$GITHUB_OUTPUT" - name: Perform environment health check id: health_check @@ -216,7 +225,9 @@ jobs: SPACE_NAME: ${{ steps.urls.outputs.space_name }} LIVE_URL: ${{ steps.urls.outputs.live_url }} SPACE_REPO_URL: ${{ steps.urls.outputs.space_repo_url }} + SPACE_REPO_ID: ${{ steps.urls.outputs.space_repo_id }} ENV_NAME: ${{ matrix.environment }} + COMMENT_TAG: "" with: github-token: ${{ secrets.GITHUB_TOKEN }} script: | @@ -224,7 +235,9 @@ jobs: const spaceName = process.env.SPACE_NAME; const liveUrl = process.env.LIVE_URL; const repoUrl = process.env.SPACE_REPO_URL; + const repoId = process.env.SPACE_REPO_ID; const envName = process.env.ENV_NAME; + const marker = process.env.COMMENT_TAG; const header = status === 'success' ? `✅ Deployment succeeded for \`${envName}\`` @@ -235,6 +248,8 @@ jobs: : 'Please resolve your environment.'; const body = [ + marker, + '', header, '', `- Space repo: [${repoUrl}](${repoUrl})`, @@ -242,15 +257,41 @@ jobs: '', summary, '', - 'You can iterate locally or validate fixes by running `scripts/deploy_to_hf.sh --env "' + envName + '"`.' + '- CLI: `openenv push ' + envName + ' --repo-id ' + repoId + '`', + '- Auth: export `HUGGINGFACEHUB_API_TOKEN` before running' ].join('\n'); - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number, - body - }); + const {owner, repo} = context.repo; + const issue_number = context.payload.pull_request.number; + + const existing = await github.paginate( + github.rest.issues.listComments, + { owner, repo, issue_number, per_page: 100 }, + (response, done) => { + const match = response.data.find(comment => comment.body && comment.body.includes(marker)); + if (match) { + done(); + return [match]; + } + return []; + } + ); + + if (existing.length > 0) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing[0].id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body, + }); + } - name: Fail job if health check failed if: steps.health_check.conclusion == 'failure' From a1a1b473f489a49058fefc3f09c56734d3fa2bb9 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:39:56 +0100 Subject: [PATCH 33/38] add delete space step --- .github/workflows/pr-new-env.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.github/workflows/pr-new-env.yml b/.github/workflows/pr-new-env.yml index 955f9913..e9854303 100644 --- a/.github/workflows/pr-new-env.yml +++ b/.github/workflows/pr-new-env.yml @@ -293,6 +293,36 @@ jobs: }); } + - name: Delete preview space on Hugging Face + if: always() + continue-on-error: true + shell: bash + env: + SPACE_REPO_ID: ${{ steps.urls.outputs.space_repo_id }} + run: | + set -euo pipefail + if [ -z "${SPACE_REPO_ID:-}" ]; then + echo "No space repo id; skipping deletion" + exit 0 + fi + + TOKEN="${HF_TOKEN:-${HUGGINGFACEHUB_API_TOKEN:-}}" + if [ -z "$TOKEN" ]; then + echo "HF token not available; cannot delete space" + exit 0 + fi + + set +e + hf repo delete "$SPACE_REPO_ID" --repo-type space --yes --token "$TOKEN" + status=$? + set -e + + if [ $status -eq 0 ]; then + echo "Deleted preview space $SPACE_REPO_ID" + else + echo "Failed to delete space $SPACE_REPO_ID (exit $status)" + fi + - name: Fail job if health check failed if: steps.health_check.conclusion == 'failure' run: exit 1 From 1a7612812f95cfe38238fd71057e589f0fb029da Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:45:52 +0100 Subject: [PATCH 34/38] streamline comment --- .github/workflows/pr-new-env.yml | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr-new-env.yml b/.github/workflows/pr-new-env.yml index e9854303..e5187221 100644 --- a/.github/workflows/pr-new-env.yml +++ b/.github/workflows/pr-new-env.yml @@ -240,25 +240,22 @@ jobs: const marker = process.env.COMMENT_TAG; const header = status === 'success' - ? `✅ Deployment succeeded for \`${envName}\`` - : `âš ī¸ Deployment failed for \`${envName}\``; + ? `✅ Deployment to Hugging Face succeeded for \`${envName}\`` + : `âš ī¸ Deployment Hugging Face failed for \`${envName}\``; const summary = status === 'success' - ? 'Nice work! Wait for a code review and we\'re ready to go.' - : 'Please resolve your environment.'; + ? 'Nice work! Wait for a code review and we\'re ready to go. You can test it with the CLI:' + : 'Please resolve your environment and test it with the CLI:'; const body = [ marker, '', header, '', - `- Space repo: [${repoUrl}](${repoUrl})`, - `- Live URL: [${liveUrl}](${liveUrl})`, '', summary, '', - '- CLI: `openenv push ' + envName + ' --repo-id ' + repoId + '`', - '- Auth: export `HUGGINGFACEHUB_API_TOKEN` before running' + '- `openenv push ' + envName + ' --repo-id ' + repoId + '`', ].join('\n'); const {owner, repo} = context.repo; From 7e77e72e807a3ff128965504f662e40e0b7419db Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:47:48 +0100 Subject: [PATCH 35/38] test change --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index c4cd0a08..5152f993 100644 --- a/README.md +++ b/README.md @@ -229,3 +229,5 @@ And we'd also like to acknowledge the team at Farama Foundation as the OpenEnv A ## License BSD 3-Clause License (see LICENSE file) + + From b8b6126c884e47a3569a174fd4919cd59376029d Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:56:27 +0100 Subject: [PATCH 36/38] hotfix coloring in readme --- src/envs/copy_env/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/envs/copy_env/README.md b/src/envs/copy_env/README.md index c4b7af37..1475d631 100644 --- a/src/envs/copy_env/README.md +++ b/src/envs/copy_env/README.md @@ -1,8 +1,8 @@ --- title: Echo Environment Server emoji: 🔊 -colorFrom: '#00C9FF' -colorTo: '#1B2845' +colorFrom: 'blue' +colorTo: 'gray' sdk: docker pinned: false app_port: 8000 From d82616136d5df5b8e51511188c3377dbbfc4542c Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 12:56:59 +0100 Subject: [PATCH 37/38] share github action url in comment --- .github/workflows/pr-new-env.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pr-new-env.yml b/.github/workflows/pr-new-env.yml index e5187221..4fe5d079 100644 --- a/.github/workflows/pr-new-env.yml +++ b/.github/workflows/pr-new-env.yml @@ -256,10 +256,18 @@ jobs: summary, '', '- `openenv push ' + envName + ' --repo-id ' + repoId + '`', - ].join('\n'); + ]; const {owner, repo} = context.repo; const issue_number = context.payload.pull_request.number; + const serverUrl = process.env.GITHUB_SERVER_URL || 'https://github.com'; + const runUrl = `${serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; + + if (status !== 'success') { + body.push(`- Failed run: ${runUrl}`); + } + + const bodyText = body.join('\n'); const existing = await github.paginate( github.rest.issues.listComments, @@ -279,14 +287,14 @@ jobs: owner, repo, comment_id: existing[0].id, - body, + body: bodyText, }); } else { await github.rest.issues.createComment({ owner, repo, issue_number, - body, + body: bodyText, }); } From 61857bfc55b6f8be20452cef634a0071aab22193 Mon Sep 17 00:00:00 2001 From: burtenshaw Date: Mon, 3 Nov 2025 13:09:04 +0100 Subject: [PATCH 38/38] test commit --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 5f484a90..f433ac5b 100644 --- a/README.md +++ b/README.md @@ -340,6 +340,4 @@ And we'd also like to acknowledge the team at Farama Foundation as the OpenEnv A ## License -BSD 3-Clause License (see LICENSE file) - - +BSD 3-Clause License (see LICENSE file) \ No newline at end of file