diff --git a/docs/guides/registration.md b/docs/guides/registration.md index 08d943c..d61fa63 100644 --- a/docs/guides/registration.md +++ b/docs/guides/registration.md @@ -12,7 +12,7 @@ The simplest approach with auto-detected hostname: from servicekit.api import BaseServiceBuilder, ServiceInfo app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration() # Reads SERVICEKIT_ORCHESTRATOR_URL from environment .build() ) @@ -61,6 +61,7 @@ class CustomServiceInfo(ServiceInfo): app = ( BaseServiceBuilder( info=CustomServiceInfo( + id="my-service", display_name="My Service", version="1.0.0", deployment_env="staging", @@ -99,6 +100,28 @@ The `.with_registration()` method accepts these parameters: ) ``` +### ServiceInfo ID Field + +The `id` field is **required** on `ServiceInfo` and must follow slug format: + +- Lowercase letters, numbers, and hyphens only +- Must start with a letter +- No consecutive hyphens +- No trailing or leading hyphens + +**Valid examples:** `my-service`, `chap-ewars`, `prediction-service`, `service1` + +**Invalid examples:** `My-Service` (uppercase), `my_service` (underscore), `1-service` (starts with number) + +```python +# Valid +ServiceInfo(id="my-service", display_name="My Service") + +# Invalid - will raise ValidationError +ServiceInfo(id="My-Service", display_name="My Service") # uppercase +ServiceInfo(id="my_service", display_name="My Service") # underscore +``` + ### Parameters - **orchestrator_url** (`str | None`): Orchestrator registration endpoint URL. If None, reads from environment variable. @@ -182,8 +205,10 @@ The service sends this payload to the orchestrator: ```json { + "id": "my-service", "url": "http://my-service:8000", "info": { + "id": "my-service", "display_name": "My Service", "version": "1.0.0", "summary": "Service description", @@ -196,8 +221,10 @@ For custom ServiceInfo subclasses: ```json { + "id": "ml-service", "url": "http://ml-service:8000", "info": { + "id": "ml-service", "display_name": "ML Service", "version": "2.0.0", "deployment_env": "production", @@ -214,21 +241,21 @@ The orchestrator responds with registration details, including the ping endpoint ```json { - "id": "01K83B5V85PQZ1HTH4DQ7NC9JM", + "id": "my-service", "status": "registered", "service_url": "http://my-service:8000", "message": "Service registered successfully", "ttl_seconds": 30, - "ping_url": "http://orchestrator:9000/services/01K83B5V85PQZ1HTH4DQ7NC9JM/$ping" + "ping_url": "http://orchestrator:9000/services/my-service/$ping" } ``` **Key fields:** -- `id`: Unique ULID identifier assigned by orchestrator +- `id`: Service identifier (matches the `id` field from ServiceInfo) - `ttl_seconds`: Time-to-live in seconds (service must ping within this window) - `ping_url`: Endpoint for keepalive pings (automatically used by the service) -**Important**: The `ping_url` is provided by the orchestrator - services don't need to configure it. The service automatically uses this URL for keepalive pings when `enable_keepalive=True`. +**Important**: The service ID is defined by the service itself via `ServiceInfo.id`, not assigned by the orchestrator. This makes registration idempotent - re-registering the same service updates the existing entry rather than creating a new one. ### Hostname Resolution @@ -261,7 +288,7 @@ Priority order: from servicekit.api import BaseServiceBuilder, ServiceInfo app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Production Service")) + BaseServiceBuilder(info=ServiceInfo(id="production-service", display_name="Production Service")) .with_logging() .with_health() .with_registration() # Reads from environment @@ -284,7 +311,7 @@ services: ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_registration( orchestrator_url="http://localhost:9000/services/$register", host="test-service", @@ -298,7 +325,7 @@ app = ( ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( orchestrator_url_env="MY_APP_ORCHESTRATOR_URL", host_env="MY_APP_HOST", @@ -321,7 +348,7 @@ For critical services that must register: ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Critical Service")) + BaseServiceBuilder(info=ServiceInfo(id="critical-service", display_name="Critical Service")) .with_registration( fail_on_error=True, # Abort startup if registration fails max_retries=10, @@ -335,7 +362,7 @@ app = ( ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( max_retries=10, # More attempts retry_delay=1.0, # Faster retries diff --git a/examples/app_hosting/main.py b/examples/app_hosting/main.py index 78f380e..e0025e7 100644 --- a/examples/app_hosting/main.py +++ b/examples/app_hosting/main.py @@ -6,6 +6,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="app-hosting-demo", display_name="App Hosting Demo", version="1.0.0", summary="Demonstrates hosting static web apps with servicekit", diff --git a/examples/auth_basic/main.py b/examples/auth_basic/main.py index ea24cc0..a9374ec 100644 --- a/examples/auth_basic/main.py +++ b/examples/auth_basic/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="auth-basic-example", display_name="Authenticated API Example", version="1.0.0", summary="Basic API key authentication example", diff --git a/examples/auth_custom_header/main.py b/examples/auth_custom_header/main.py index cbb5a4a..e878bcb 100644 --- a/examples/auth_custom_header/main.py +++ b/examples/auth_custom_header/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="auth-custom-header-example", display_name="API with Custom Authentication Header", version="1.0.0", summary="API using custom header name for authentication", diff --git a/examples/auth_docker_secrets/main.py b/examples/auth_docker_secrets/main.py index b162375..5cf424b 100644 --- a/examples/auth_docker_secrets/main.py +++ b/examples/auth_docker_secrets/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="auth-docker-secrets-example", display_name="Secure API with Docker Secrets", version="2.0.0", summary="Production API using Docker secrets file for authentication", diff --git a/examples/auth_envvar/main.py b/examples/auth_envvar/main.py index 1b60850..0d33f69 100644 --- a/examples/auth_envvar/main.py +++ b/examples/auth_envvar/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="auth-envvar-example", display_name="Production API with Environment Variable Auth", version="2.0.0", summary="Production-ready API using environment variables for authentication", diff --git a/examples/core_api/main.py b/examples/core_api/main.py index 2ba0549..f23925c 100644 --- a/examples/core_api/main.py +++ b/examples/core_api/main.py @@ -161,6 +161,7 @@ async def seed_users(app: FastAPI) -> None: app = ( BaseServiceBuilder( info=ServiceInfo( + id="core-user-service", display_name="Core User Service", version="1.0.0", summary="User management API using core-only features", diff --git a/examples/job_scheduler/main.py b/examples/job_scheduler/main.py index 4af864e..97b8273 100644 --- a/examples/job_scheduler/main.py +++ b/examples/job_scheduler/main.py @@ -119,6 +119,7 @@ async def get_computation_result( # pyright: ignore[reportUnusedFunction] info = ServiceInfo( + id="job-scheduler-demo", display_name="Job Scheduler Demo", summary="Demonstrates async job scheduling with polling and SSE streaming", version="1.0.0", diff --git a/examples/monitoring/main.py b/examples/monitoring/main.py index 01271c5..cad4821 100644 --- a/examples/monitoring/main.py +++ b/examples/monitoring/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="monitoring-example", display_name="Monitoring Example Service", version="1.0.0", summary="Service with OpenTelemetry monitoring and Prometheus metrics", diff --git a/examples/registration/README.md b/examples/registration/README.md index 08891a7..f3958eb 100644 --- a/examples/registration/README.md +++ b/examples/registration/README.md @@ -115,23 +115,25 @@ docker compose down 5. **Registration Attempt**: Sends POST to orchestrator with payload: ```json { + "id": "registration-example", "url": "http://svca:8000", "info": { + "id": "registration-example", "display_name": "Registration Example Service", "version": "1.0.0", ... } } ``` -6. **ID Assignment**: Orchestrator assigns ULID and returns: +6. **Registration Confirmed**: Orchestrator returns: ```json { - "id": "01K83B5V85PQZ1HTH4DQ7NC9JM", + "id": "registration-example", "status": "registered", "service_url": "http://svca:8000", "message": "...", "ttl_seconds": 30, - "ping_url": "http://orchestrator:9000/services/01K83B5V85PQZ1HTH4DQ7NC9JM/$ping" + "ping_url": "http://orchestrator:9000/services/registration-example/$ping" } ``` 7. **Keepalive Started**: Background task starts sending pings every 10 seconds to `ping_url` @@ -243,7 +245,7 @@ services: ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( orchestrator_url="http://orchestrator:9000/services/$register", service_key="my-secret-key", @@ -256,7 +258,7 @@ app = ( ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( service_key_env="MY_APP_REGISTRATION_KEY", ) @@ -277,7 +279,7 @@ The service key is included in: from servicekit.api import BaseServiceBuilder, ServiceInfo app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_logging() .with_health() .with_registration() # Auto-detect hostname @@ -297,6 +299,7 @@ class CustomServiceInfo(ServiceInfo): app = ( BaseServiceBuilder( info=CustomServiceInfo( + id="custom-service", display_name="Custom Service", deployment_env="staging", team="data-science", @@ -313,7 +316,7 @@ app = ( ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( orchestrator_url_env="MY_APP_ORCHESTRATOR_URL", host_env="MY_APP_HOST", @@ -327,7 +330,7 @@ app = ( ```python app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="My Service")) + BaseServiceBuilder(info=ServiceInfo(id="my-service", display_name="My Service")) .with_registration( orchestrator_url="http://orchestrator:9000/register", host="my-service", diff --git a/examples/registration/main.py b/examples/registration/main.py index ca2e5ac..6a90058 100644 --- a/examples/registration/main.py +++ b/examples/registration/main.py @@ -5,6 +5,7 @@ app = ( BaseServiceBuilder( info=ServiceInfo( + id="registration-example", display_name="Registration Example Service", version="1.0.0", summary="Demonstrates automatic service registration with orchestrator", diff --git a/examples/registration/main_custom.py b/examples/registration/main_custom.py index 1d68534..fe955f1 100644 --- a/examples/registration/main_custom.py +++ b/examples/registration/main_custom.py @@ -15,6 +15,7 @@ class CustomServiceInfo(ServiceInfo): app = ( BaseServiceBuilder( info=CustomServiceInfo( + id="custom-metadata-service", display_name="Custom Metadata Service", version="2.0.0", summary="Demonstrates custom ServiceInfo with additional metadata", diff --git a/examples/registration/orchestrator.py b/examples/registration/orchestrator.py index b83f952..9d1a521 100644 --- a/examples/registration/orchestrator.py +++ b/examples/registration/orchestrator.py @@ -262,6 +262,7 @@ async def lifespan(app: FastAPI): app = ( BaseServiceBuilder( info=ServiceInfo( + id="mock-orchestrator", display_name="Mock Orchestrator", version="1.0.0", summary="Simple orchestrator for testing service registration", diff --git a/pyproject.toml b/pyproject.toml index 99b80b3..9603fbc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "servicekit" -version = "0.7.0" +version = "0.8.0" description = "Async SQLAlchemy framework with FastAPI integration - reusable foundation for building data services" readme = "README.md" authors = [{ name = "Morten Hansen", email = "morten@winterop.com" }] diff --git a/src/servicekit/api/registration.py b/src/servicekit/api/registration.py index 5c8fc53..2421ebc 100644 --- a/src/servicekit/api/registration.py +++ b/src/servicekit/api/registration.py @@ -113,8 +113,19 @@ async def register_service( # Build service URL service_url = f"http://{resolved_host}:{resolved_port}" + # Extract service ID from info (requires id attribute) + service_id = getattr(info, "id", None) + if not service_id: + error_msg = "ServiceInfo must have an 'id' attribute for registration" + logger.error("registration.missing_service_id") + if fail_on_error: + raise ValueError(error_msg) + logger.warning("registration.skipped", reason="missing service ID") + return None + # Build registration payload payload: dict[str, Any] = { + "id": service_id, "url": service_url, "info": info.model_dump(mode="json"), } @@ -146,18 +157,16 @@ async def register_service( ) response.raise_for_status() - # Parse response to extract service ID + # Parse response for additional info (ping_url, ttl) response_data = response.json() - service_id = response_data.get("id") log_context = { "orchestrator_url": resolved_orchestrator_url, "service_url": service_url, + "service_id": service_id, "attempt": attempt, "status_code": response.status_code, } - if service_id: - log_context["service_id"] = service_id # Store global references for keepalive global _service_id, _ping_url diff --git a/src/servicekit/api/service_builder.py b/src/servicekit/api/service_builder.py index f9a45ff..b50ac8e 100644 --- a/src/servicekit/api/service_builder.py +++ b/src/servicekit/api/service_builder.py @@ -9,7 +9,7 @@ from typing import Any, AsyncContextManager, AsyncIterator, Awaitable, Callable, Dict, List, Self from fastapi import APIRouter, FastAPI -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, field_validator from sqlalchemy import text from servicekit import Database, SqliteDatabase @@ -100,6 +100,7 @@ class _RegistrationOptions: class ServiceInfo(BaseModel): """Service metadata for FastAPI application.""" + id: str display_name: str version: str = "1.0.0" summary: str | None = None @@ -109,6 +110,17 @@ class ServiceInfo(BaseModel): model_config = ConfigDict(extra="forbid") + @field_validator("id") + @classmethod + def validate_id(cls, v: str) -> str: + """Validate service ID follows slug format.""" + if not re.match(r"^[a-z][a-z0-9]*(-[a-z0-9]+)*$", v): + raise ValueError( + "Service ID must be slug format: lowercase letters, numbers, " + "and hyphens (e.g., 'my-service', 'chap-ewars')" + ) + return v + class BaseServiceBuilder: """Base service builder providing core FastAPI functionality without module dependencies.""" diff --git a/tests/test_api_registration.py b/tests/test_api_registration.py index 7e9c7b6..4938a0f 100644 --- a/tests/test_api_registration.py +++ b/tests/test_api_registration.py @@ -18,6 +18,114 @@ class CustomServiceInfo(ServiceInfo): priority: int = 1 +# --- ServiceInfo ID Validation Tests --- + + +def test_service_info_valid_id(): + """Test valid slug format IDs are accepted.""" + valid_ids = [ + "my-service", + "chap-ewars", + "prediction-service", + "service1", + "a", + "a1", + "my-service-v2", + "test123", + ] + for service_id in valid_ids: + info = ServiceInfo(id=service_id, display_name="Test") + assert info.id == service_id + + +def test_service_info_invalid_id_uppercase(): + """Test uppercase letters are rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="My-Service", display_name="Test") + + +def test_service_info_invalid_id_spaces(): + """Test spaces are rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="my service", display_name="Test") + + +def test_service_info_invalid_id_special_chars(): + """Test special characters are rejected.""" + invalid_ids = ["my_service", "my.service", "my@service", "my/service"] + for service_id in invalid_ids: + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id=service_id, display_name="Test") + + +def test_service_info_invalid_id_starts_with_number(): + """Test IDs starting with number are rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="1-service", display_name="Test") + + +def test_service_info_invalid_id_consecutive_hyphens(): + """Test consecutive hyphens are rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="my--service", display_name="Test") + + +def test_service_info_invalid_id_trailing_hyphen(): + """Test trailing hyphen is rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="my-service-", display_name="Test") + + +def test_service_info_invalid_id_leading_hyphen(): + """Test leading hyphen is rejected.""" + with pytest.raises(ValueError, match="slug format"): + ServiceInfo(id="-my-service", display_name="Test") + + +def test_service_info_id_required(): + """Test missing ID raises validation error.""" + with pytest.raises(ValueError): + ServiceInfo(display_name="Test") # type: ignore[call-arg] + + +# --- Registration Tests --- + + +@pytest.mark.asyncio +async def test_registration_missing_service_id_fail_on_error(): + """Test registration fails when info has no id attribute and fail_on_error=True.""" + # Create a mock info object without 'id' attribute + mock_info = MagicMock(spec=[]) # Empty spec means no 'id' attribute + mock_info.model_dump = MagicMock(return_value={"display_name": "Test"}) + + with pytest.raises(ValueError, match="must have an 'id' attribute"): + await register_service( + orchestrator_url="http://orchestrator:9000/services/$register", + host="test-service", + port=8000, + info=mock_info, + fail_on_error=True, + ) + + +@pytest.mark.asyncio +async def test_registration_missing_service_id_returns_none(): + """Test registration returns None when info has no id attribute and fail_on_error=False.""" + # Create a mock info object without 'id' attribute + mock_info = MagicMock(spec=[]) # Empty spec means no 'id' attribute + mock_info.model_dump = MagicMock(return_value={"display_name": "Test"}) + + result = await register_service( + orchestrator_url="http://orchestrator:9000/services/$register", + host="test-service", + port=8000, + info=mock_info, + fail_on_error=False, + ) + + assert result is None + + @pytest.mark.asyncio async def test_successful_registration(): """Test successful service registration.""" @@ -36,7 +144,7 @@ async def test_successful_registration(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -49,6 +157,7 @@ async def test_successful_registration(): call_args = mock_client.return_value.__aenter__.return_value.post.call_args assert call_args[0][0] == "http://orchestrator:9000/services/$register" payload = call_args[1]["json"] + assert payload["id"] == "test-service" assert payload["url"] == "http://test-service:8000" assert payload["info"]["display_name"] == "Test Service" @@ -78,7 +187,7 @@ async def test_retry_logic_success_on_second_attempt(): side_effect=[mock_response_fail, mock_response_success] ) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -104,7 +213,7 @@ async def test_fail_on_error_true_raises_exception(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") with pytest.raises(RuntimeError, match="Failed to register service"): await register_service( @@ -129,7 +238,7 @@ async def test_fail_on_error_false_continues(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") # Should not raise exception await register_service( @@ -153,7 +262,7 @@ async def test_hostname_resolution_parameter(): with patch("httpx.AsyncClient") as mock_client, patch.dict(os.environ, {}, clear=True): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -180,7 +289,7 @@ async def test_hostname_resolution_auto_detect(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -207,7 +316,7 @@ async def test_hostname_resolution_env_var(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -224,7 +333,7 @@ async def test_hostname_resolution_env_var(): async def test_hostname_missing_raises_with_fail_on_error(): """Test missing hostname raises exception when fail_on_error=True.""" with patch("socket.gethostname", side_effect=Exception("No hostname")), patch.dict(os.environ, {}, clear=True): - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") # Should log warning and return early (no exception with fail_on_error=False) await register_service( @@ -256,7 +365,7 @@ async def test_port_resolution_parameter(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -279,7 +388,7 @@ async def test_port_resolution_env_var(): with patch("httpx.AsyncClient") as mock_client, patch.dict(os.environ, {"SERVICEKIT_PORT": "7777"}): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -302,7 +411,7 @@ async def test_port_resolution_default(): with patch("httpx.AsyncClient") as mock_client, patch.dict(os.environ, {}, clear=True): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -336,7 +445,7 @@ async def test_custom_env_var_names(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url=None, @@ -365,6 +474,7 @@ async def test_custom_serviceinfo_serialization(): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) info = CustomServiceInfo( + id="custom-service", display_name="Custom Service", version="2.0.0", team="data-science", @@ -389,7 +499,7 @@ async def test_custom_serviceinfo_serialization(): async def test_missing_orchestrator_url(): """Test missing orchestrator URL.""" with patch.dict(os.environ, {}, clear=True): - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") # Should log warning and return early (no exception with fail_on_error=False) await register_service( @@ -421,7 +531,7 @@ async def test_url_construction(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -444,7 +554,7 @@ async def test_timeout_parameter(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -478,7 +588,7 @@ async def test_registration_returns_info(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") result = await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -488,7 +598,7 @@ async def test_registration_returns_info(): ) assert result is not None - assert result["service_id"] == "01K83B5V85PQZ1HTH4DQ7NC9JM" + assert result["service_id"] == "test-service" assert result["service_url"] == "http://test-service:8000" assert result["ttl_seconds"] == 30 assert result["ping_url"] == "http://orchestrator:9000/services/01K83B5V85PQZ1HTH4DQ7NC9JM/$ping" @@ -498,7 +608,7 @@ async def test_registration_returns_info(): async def test_registration_returns_none_on_failure(): """Test that failed registration returns None.""" with patch.dict(os.environ, {}, clear=True): - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") result = await register_service( orchestrator_url=None, # Missing URL @@ -655,7 +765,7 @@ async def test_registration_with_service_key_parameter(): with patch("httpx.AsyncClient") as mock_client: mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -686,7 +796,7 @@ async def test_registration_with_service_key_from_env(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -716,7 +826,7 @@ async def test_registration_with_custom_service_key_env(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -744,7 +854,7 @@ async def test_registration_without_service_key(): with patch("httpx.AsyncClient") as mock_client, patch.dict(os.environ, {}, clear=True): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", @@ -773,7 +883,7 @@ async def test_service_key_parameter_overrides_env(): ): mock_client.return_value.__aenter__.return_value.post = AsyncMock(return_value=mock_response) - info = ServiceInfo(display_name="Test Service", version="1.0.0") + info = ServiceInfo(id="test-service", display_name="Test Service", version="1.0.0") await register_service( orchestrator_url="http://orchestrator:9000/services/$register", diff --git a/tests/test_auth.py b/tests/test_auth.py index 75d1dc0..36615f5 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -303,7 +303,7 @@ def test_service_builder_auth_logging_no_duplicates(capsys: pytest.CaptureFixtur from servicekit.api import BaseServiceBuilder, ServiceInfo # Build app with direct API keys (triggers warning) - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") app = BaseServiceBuilder(info=info, include_logging=True).with_auth(api_keys=["sk_dev_test123"]).build() # Create test client (triggers startup) diff --git a/tests/test_job_router.py b/tests/test_job_router.py index b2fb9c3..374219d 100644 --- a/tests/test_job_router.py +++ b/tests/test_job_router.py @@ -16,7 +16,7 @@ @pytest.fixture async def app() -> AsyncGenerator[FastAPI, None]: """Create FastAPI app with job router and trigger lifespan.""" - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") app_instance = BaseServiceBuilder(info=info).with_jobs().build() # Manually trigger lifespan @@ -400,7 +400,7 @@ class TestJobRouterIntegration: @pytest.mark.asyncio async def test_service_builder_with_jobs(self) -> None: """Test BaseServiceBuilder.with_jobs() creates functional job endpoints.""" - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") app = BaseServiceBuilder(info=info).with_jobs(prefix="/jobs", tags=["background"]).build() # Trigger lifespan to initialize scheduler @@ -416,7 +416,7 @@ async def test_service_builder_with_jobs(self) -> None: @pytest.mark.asyncio async def test_service_builder_with_max_concurrency(self) -> None: """Test BaseServiceBuilder.with_jobs(max_concurrency=N) configures scheduler.""" - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") app = BaseServiceBuilder(info=info).with_jobs(max_concurrency=2).build() # Trigger lifespan to initialize scheduler @@ -428,7 +428,7 @@ async def test_service_builder_with_max_concurrency(self) -> None: @pytest.mark.asyncio async def test_job_endpoints_in_openapi_schema(self) -> None: """Test job endpoints appear in OpenAPI schema.""" - info = ServiceInfo(display_name="Test Service") + info = ServiceInfo(id="test-service", display_name="Test Service") app = BaseServiceBuilder(info=info).with_jobs().build() async with AsyncClient( diff --git a/tests/test_service_builder_apps.py b/tests/test_service_builder_apps.py index 80baa05..36b9c9f 100644 --- a/tests/test_service_builder_apps.py +++ b/tests/test_service_builder_apps.py @@ -36,7 +36,7 @@ def app_directory(tmp_path: Path) -> Path: def test_service_builder_with_single_app(app_directory: Path): """Test mounting a single app.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard")) .build() ) @@ -56,7 +56,7 @@ def test_service_builder_with_single_app(app_directory: Path): def test_service_builder_with_prefix_override(app_directory: Path): """Test mounting app with custom prefix.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard"), prefix="/custom") .build() ) @@ -75,7 +75,7 @@ def test_service_builder_with_prefix_override(app_directory: Path): def test_service_builder_with_multiple_apps(app_directory: Path): """Test mounting multiple apps.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard")) .with_app(str(app_directory / "admin")) .build() @@ -94,7 +94,11 @@ def test_service_builder_with_multiple_apps(app_directory: Path): def test_service_builder_with_apps_autodiscovery(app_directory: Path): """Test auto-discovering apps from directory.""" - app = BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")).with_apps(str(app_directory)).build() + app = ( + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) + .with_apps(str(app_directory)) + .build() + ) with TestClient(app) as client: # Both discovered apps should be accessible @@ -110,7 +114,7 @@ def test_service_builder_with_apps_autodiscovery(app_directory: Path): def test_service_builder_apps_with_api_routes(app_directory: Path): """Test that apps don't interfere with API routes.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_health() .with_system() .with_app(str(app_directory / "dashboard")) @@ -135,7 +139,7 @@ def test_service_builder_apps_override_semantics(app_directory: Path): """Test that duplicate prefixes use last-wins semantics.""" # Mount dashboard twice - second should override first app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard")) .with_app(str(app_directory / "admin"), prefix="/dashboard") # Override with admin .build() @@ -156,7 +160,9 @@ def test_service_builder_apps_api_prefix_blocked(app_directory: Path): (bad_app_dir / "index.html").write_text("Bad") with pytest.raises(ValueError, match="prefix cannot be '/api'"): - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")).with_app(str(bad_app_dir)).build() + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")).with_app( + str(bad_app_dir) + ).build() def test_service_builder_apps_root_mount_works(app_directory: Path): @@ -173,7 +179,7 @@ def test_service_builder_apps_root_mount_works(app_directory: Path): (about_dir / "index.html").write_text("About Page") app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_health() .with_system() .with_app(str(root_app_dir)) @@ -210,7 +216,7 @@ def test_service_builder_apps_root_override_landing_page(app_directory: Path): (root_app_dir / "index.html").write_text("Custom Root App") app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_landing_page() # Mount built-in landing page first .with_app(str(root_app_dir)) # Override with custom root app .build() @@ -231,7 +237,11 @@ def test_service_builder_html_mode_serves_index_for_subdirs(app_directory: Path) subdir.mkdir() (subdir / "index.html").write_text("Subdir Index") - app = BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")).with_app(str(dashboard_dir)).build() + app = ( + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) + .with_app(str(dashboard_dir)) + .build() + ) with TestClient(app) as client: # Requesting /dashboard/subdir/ should serve subdir/index.html @@ -243,7 +253,7 @@ def test_service_builder_html_mode_serves_index_for_subdirs(app_directory: Path) def test_service_builder_apps_404_for_missing_files(app_directory: Path): """Test that missing files return 404.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard")) .build() ) @@ -264,7 +274,7 @@ def test_service_builder_apps_mount_order(app_directory: Path): (api_like_app / "index.html").write_text("App Status") app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_health(prefix="/status") # Mount health at /status .with_app(str(api_like_app)) # Try to mount app at /status (should fail) .build() @@ -289,7 +299,7 @@ async def custom_endpoint() -> dict[str, str]: return {"message": "custom"} app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_app(str(app_directory / "dashboard")) .include_router(custom_router) .build() @@ -309,7 +319,7 @@ async def custom_endpoint() -> dict[str, str]: def test_system_apps_endpoint_empty(): """Test /api/v1/system/apps with no apps.""" - app = BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")).with_system().build() + app = BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")).with_system().build() with TestClient(app) as client: response = client.get("/api/v1/system/apps") @@ -320,7 +330,7 @@ def test_system_apps_endpoint_empty(): def test_system_apps_endpoint_with_apps(app_directory: Path): """Test /api/v1/system/apps lists installed apps.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_system() .with_app(str(app_directory / "dashboard")) .with_app(str(app_directory / "admin")) @@ -350,7 +360,7 @@ def test_system_apps_endpoint_with_apps(app_directory: Path): def test_system_apps_endpoint_returns_correct_fields(app_directory: Path): """Test AppInfo fields match manifest.""" app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_system() .with_app(str(app_directory / "dashboard")) .build() @@ -381,7 +391,7 @@ def test_system_apps_endpoint_returns_correct_fields(app_directory: Path): def test_system_apps_schema_endpoint(): """Test /api/v1/system/apps/$schema returns JSON schema.""" - app = BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")).with_system().build() + app = BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")).with_system().build() with TestClient(app) as client: response = client.get("/api/v1/system/apps/$schema") @@ -448,7 +458,7 @@ def test_service_builder_with_apps_package_discovery(tmp_path: Path, monkeypatch # Build service with package app discovery app = ( - BaseServiceBuilder(info=ServiceInfo(display_name="Test Service")) + BaseServiceBuilder(info=ServiceInfo(id="test-service", display_name="Test Service")) .with_apps(("test_app_package", "bundled_apps")) .build() ) diff --git a/uv.lock b/uv.lock index 8bbbf39..6d384b6 100644 --- a/uv.lock +++ b/uv.lock @@ -1129,7 +1129,7 @@ wheels = [ [[package]] name = "servicekit" -version = "0.7.0" +version = "0.8.0" source = { editable = "." } dependencies = [ { name = "aiosqlite" },