kaxil commented on PR #71676: URL: https://github.com/apache/airflow/pull/71676#issuecomment-5774742637
I went back through each round-2 thread against the `opensandbox` 1.1.0 source rather than the fixtures, and the fixes hold: the dispatcher records `error` and `complete` regardless of `skip_accumulation`, so the exit-code path sees real data; a plain 404 on `connect` is not retried by the readiness budget, so `destroy` on a gone sandbox returns at once; `get_egress_policy`, the `metadata` kwarg and `skip_health_check` all exist with the shapes used here. Nothing blocking from me. Three non-blocking notes, here rather than as a review round: 1. `docs/sandbox/backends.rst` L228, the "Command timeouts" bullet says a timeout destroys an OpenSandbox sandbox and its files. Your own measurement above (`sleep 60` on a 5s budget gives `sandbox_terminated=False`) shows a server-enforced timeout leaves the sandbox and its files intact, and only a stalled stream destroys it. OpenSandbox belongs on the Modal side of that sentence, with the stall case as the exception. 2. `_get_network_policy` sends `NetworkPolicy(defaultAction="allow")` for `block_network=False`. The server refuses any create that carries a `networkPolicy` when `egress.image` is not configured (`ensure_egress_configured` in `server/opensandbox_server/services/validators.py` returns early only on a falsy policy), and the Docker runtime refuses one under `network_mode=host` or a user-defined network as well (`_ensure_network_policy_support` in `services/docker/networking.py`). So a deployment without the sidecar cannot run an open-network spec even though there is nothing to enforce. Returning `None` for the open case would let those deployments work, and `create` already skips the read-back for it. 3. The Modal backend's extra is `modal`; this one is `sandbox-opensandbox`. Extras are permanent public surface, so worth deciding now whether it should be `opensandbox` to match. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
