Merge pull request #313 from finos/feat/return-by-request-id

feat(machines): return machines by request-id

Author: fgogolli

pyproject.toml

@@ -208,22 +208,37 @@ test-k8s = [
 ]
 
 # ---------------------------------------------------------------------------
-# pip compatibility shims for [dependency-groups] (PEP 735)
+# pip compatibility shim for [dependency-groups] (PEP 735)
 #
 # pip does not yet support PEP 735 dependency-groups.  The authoritative dev
-# dependency lists live in [dependency-groups] below.  These optional-deps
-# mirror them so `pip install -e ".[dev]"` keeps working.  When pip gains
-# PEP 735 support these shims can be removed.
+# dependency list lives in [dependency-groups] below.  This optional-dep keeps
+# `pip install -e ".[dev]"` working by combining two things:
+#
+#   1. orb-py[all] — the full runtime surface (providers, api, ui, monitoring,
+#      sql, cli).  pip resolves extras, so runtime deps need NOT be re-listed
+#      here; referencing [all] avoids a hand-maintained mirror of them.
+#   2. The CI/dev TOOLCHAIN (ruff, pytest, mkdocs, bandit, …).  These live only
+#      in [dependency-groups] — which pip cannot read — so they must be mirrored
+#      here until pip gains PEP 735 support.
+#
+# tests/unit/test_pyproject_dev_extra.py guards that this shim still covers the
+# whole dev dependency-group, so the toolchain mirror can't silently drift.
 # ---------------------------------------------------------------------------
-ci = [
-    # Code Quality Tools
+dev = [
+    # Full runtime surface — including the UI, which [all-providers] omits.
+    "orb-py[all]",
+    # --- toolchain from dependency-groups.lint ---
     "ruff>=0.1.0",
     "pathspec>=0.11.0",
+    # --- toolchain from dependency-groups.typecheck ---
     "pyright>=1.1.408,<2.0.0",
-    "bandit>=1.7.5,<2.0.0",
-    "bandit-sarif-formatter>=1.1.1,<2.0.0",
+    "types-PyYAML>=6.0.12.12,<7.0.0",
+    "types-python-dateutil>=2.8.19.14,<3.0.0",
+    "kubernetes-stubs-elephant-fork>=36.0.2,<37.0.0",
+    "tomli>=2.0.0,<3.0.0",
+    # --- toolchain from dependency-groups.arch ---
     "import-linter>=2.0,<3.0.0",
-    # Testing Framework
+    # --- toolchain from dependency-groups.test (runtime test deps not in [all]) ---
     "pytest>=7.4.3,<10.0.0",
     "pytest-cov>=4.1.0,<8.0.0",
     "pytest-env>=1.1.1,<2.0.0",
@@ -234,39 +249,18 @@ ci = [
     "pytest-html>=4.1.1,<5.0.0",
     "pytest-benchmark>=5.1.0,<6.0.0",
     "coverage>=7.3.2,<8.0.0",
-    # Test dependencies for API and template tests
-    "fastapi>=0.128.0",
     "httpx>=0.27.0",
-    "jinja2>=3.1.0",
-    # AWS Testing
     "moto[all]>=5.1.19,<6.0.0",
     "responses>=0.24.0,<1.0.0",
     "requests-mock>=1.11.0,<2.0.0",
     "joserfc>=1.6.1",
     "werkzeug>=3.1.6",
     "nltk>=3.9.3",
-    # k8s-legacy extra deps — required for tests/integration/k8s_legacy/
-    "kubernetes",
     "watchdog",
     "tenacity",
     "pg8000",
-    "uvicorn>=0.24.0",
-    "prometheus-client>=0.17.0",
-    "alembic>=1.13",
-    # Essential Type Stubs
-    "types-PyYAML>=6.0.12.12,<7.0.0",
-    "types-python-dateutil>=2.8.19.14,<3.0.0",
-    "kubernetes-stubs-elephant-fork>=36.0.2,<37.0.0",
-    # Build Tools
-    "tomli>=2.0.0,<3.0.0",
-    # Security Scanning
-    "safety>=3.7.0,<4.0.0",
-    "pip-audit>=2.6.1,<3.0.0",
-    "semgrep>=1.45.0,<2.0.0",
-    "cyclonedx-bom>=4.0.0,<8.0.0",
-    "peewee>=3.18.3",
-    "marshmallow>=4.1.2",
-    # Documentation
+    "kmock>=0.7",
+    # --- toolchain from dependency-groups.docs ---
     "mkdocs>=1.5.0,<2.0.0",
     "mkdocs-material>=9.1.0,<10.0.0",
     "mkdocstrings>=0.22.0,<2.0.0",
@@ -275,12 +269,21 @@ ci = [
     "mkdocs-literate-nav>=0.6.0,<1.0.0",
     "mkdocs-section-index>=0.3.0,<1.0.0",
     "mike>=1.1.0,<3.0.0",
-    # Build and Packaging
+    # --- inlined from dependency-groups.security ---
+    "bandit>=1.7.5,<2.0.0",
+    "bandit-sarif-formatter>=1.1.1,<2.0.0",
+    "safety>=3.7.0,<4.0.0",
+    "pip-audit>=2.6.1,<3.0.0",
+    "cyclonedx-bom>=4.0.0,<8.0.0",
+    "peewee>=3.18.3",
+    "marshmallow>=4.1.2",
+    "lxml>=6.1.0",
+    # semgrep is intentionally excluded from this shim (as in the security group):
+    # workflows pin and install their own version to avoid version drift.
+    # --- inlined from dependency-groups.build ---
     "build>=1.0.3,<2.0.0",
     "wheel>=0.41.3,<1.0.0",
-]
-dev = [
-    "orb-py[ci,all-providers]",
+    # --- dev-only tools (not in any CI group) ---
     "virtualenv>=20.26.6",
     "pre-commit>=3.5.0,<5.0.0",
     "bump2version>=1.0.1,<2.0.0",
@@ -304,7 +307,7 @@ dev = [
 # The [project.optional-dependencies] ci/dev sections above are pip shims
 # that mirror these lists for `pip install -e ".[dev]"` compatibility.
 #
-# Per-job groups (Phase 2 scoping):
+# Per-job groups (each maps to one CI job):
 #   lint      — ruff + pathspec (lint-ruff, auto-format jobs)
 #   typecheck — pyright + type stubs (lint-pyright job)
 #   arch      — import-linter (arch-validation job; pyyaml via project deps)

src/orb/api/routers/machines.py

@@ -5,7 +5,7 @@
 try:
     from fastapi import APIRouter, Depends, Query, Request
     from fastapi.responses import JSONResponse
-    from pydantic import AliasChoices, Field
+    from pydantic import AliasChoices, Field, model_validator
 except ImportError:
     raise ImportError("FastAPI routing requires: pip install orb-py[api]") from None
 
@@ -68,14 +68,30 @@ class ReturnMachinesRequest(APIRequest):
     """Request for machine return.
 
     Accepts both camelCase (machineIds) and snake_case field names.
+    Exactly one of machine_ids (non-empty), request_id, or all_machines=True must be provided.
     """
 
-    machine_ids: list[str]
+    machine_ids: list[str] = Field(default_factory=list)
+    request_id: Optional[str] = None
     all_machines: bool = False
     force: bool = False
     provider_name: Optional[str] = None
     provider_type: Optional[str] = None
 
+    @model_validator(mode="after")
+    def validate_target_selection(self) -> "ReturnMachinesRequest":
+        has_machine_ids = bool(self.machine_ids)
+        has_request_id = bool(self.request_id)
+        has_all = self.all_machines
+        # Targeting modes are mutually exclusive, but zero is allowed: an empty
+        # request is handled downstream as a no-op (preserves existing contract).
+        if sum([has_machine_ids, has_request_id, has_all]) > 1:
+            raise ValueError(
+                "machine_ids, request_id, and all_machines are mutually exclusive; "
+                "provide at most one."
+            )
+        return self
+
 
 @router.post(
     "/request",
@@ -138,6 +154,7 @@ async def return_machines(
     result = await orchestrator.execute(
         ReturnMachinesInput(
             machine_ids=request_data.machine_ids,
+            request_id=request_data.request_id,
             all_machines=request_data.all_machines,
             force=request_data.force,
             provider_name=request_data.provider_name,

src/orb/application/services/orchestration/dtos.py

@@ -128,6 +128,7 @@ class ListRequestsOutput:
 class ReturnMachinesInput:
     machine_ids: list[str] = dataclasses.field(default_factory=list)
     all_machines: bool = False
+    request_id: Optional[str] = None
     force: bool = False
     wait: bool = False
     timeout_seconds: int = 300

src/orb/application/services/orchestration/return_machines.py

@@ -46,9 +46,10 @@ def __init__(
 
     async def execute(self, input: ReturnMachinesInput) -> ReturnMachinesOutput:  # type: ignore[return]
         self._logger.info(
-            "ReturnMachinesOrchestrator: machines=%s all=%s force=%s",
+            "ReturnMachinesOrchestrator: machines=%s all=%s request_id=%s force=%s",
             input.machine_ids,
             input.all_machines,
+            input.request_id,
             input.force,
         )
 
@@ -71,6 +72,26 @@ async def execute(self, input: ReturnMachinesInput) -> ReturnMachinesOutput:  #
                     status="no_machines",
                     message="No active machines found",
                 )
+        elif input.request_id:
+            result = await self._query_bus.execute(
+                ListMachinesQuery(
+                    request_id=input.request_id,
+                    provider_name=input.provider_name,
+                    provider_type=input.provider_type,
+                )
+            )
+            machine_dtos = result.items if isinstance(result, Paginated) else (result or [])
+            machine_ids = [dto.machine_id for dto in machine_dtos]
+            if not machine_ids:
+                self._logger.warning(
+                    "ReturnMachinesOrchestrator: request_id=%s requested but no active machines found",
+                    input.request_id,
+                )
+                return ReturnMachinesOutput(
+                    request_id=None,
+                    status="no_machines",
+                    message="No active machines found for request",
+                )
         else:
             machine_ids = list(input.machine_ids)
 

src/orb/cli/args.py

@@ -153,6 +153,15 @@ def add_machine_actions(subparsers):
     machines_return.add_argument(
         "--machine-id", "-m", action="append", dest="machine_ids_flag", help="Machine ID to return"
     )
+    machines_return.add_argument(
+        "--request-id",
+        "-r",
+        dest="request_id",
+        help=(
+            "Return all machines belonging to this acquisition request ID. "
+            "Mutually exclusive with machine IDs and --all."
+        ),
+    )
     machines_return.add_argument(
         "--wait", action="store_true", help="Wait for return request to complete"
     )

src/orb/interface/request_command_handlers.py

@@ -190,6 +190,7 @@ async def handle_request_return_machines(
     formatter = container.get(ResponseFormattingService)
 
     has_all = getattr(args, "all", False)
+    request_id = getattr(args, "request_id", None)
     machine_ids: list[str] = []
 
     if hasattr(args, "input_data") and args.input_data:
@@ -205,6 +206,18 @@ async def handle_request_return_machines(
 
     has_specific_ids = bool(machine_ids)
 
+    if request_id and has_specific_ids:
+        return {
+            "error": "Cannot use --request-id with specific machine IDs",
+            "message": "Use either --request-id or specific machine IDs, not both",
+        }
+
+    if request_id and has_all:
+        return {
+            "error": "Cannot use --request-id with --all",
+            "message": "Use either --request-id or --all, not both",
+        }
+
     if has_all and has_specific_ids:
         return {
             "error": "Cannot use --all with specific machine IDs",
@@ -219,7 +232,7 @@ async def handle_request_return_machines(
                 "message": "Use --force to confirm returning all machines",
             }
 
-    if not has_all and not machine_ids:
+    if not has_all and not machine_ids and not request_id:
         return {
             "error": "Machine IDs are required",
             "message": "Machine IDs must be provided either as arguments or in JSON file",
@@ -228,6 +241,7 @@ async def handle_request_return_machines(
     result = await orchestrator.execute(
         ReturnMachinesInput(
             machine_ids=machine_ids,
+            request_id=request_id,
             all_machines=has_all,
             force=getattr(args, "force", False),
             wait=getattr(args, "wait", False),