zozo123 commented on PR #71676: URL: https://github.com/apache/airflow/pull/71676#issuecomment-5774112201
Pushed the round for the 2026-09-17 review; replies are inline on each thread. Behaviour changes, all confined to the OpenSandbox backend: the network policy is read back after create and a sandbox the server did not restrict is destroyed; a stalled command stream is abandoned 30s past its budget with the sandbox destroyed and `sandbox_terminated` set; an exit status the SDK could not parse is no longer a terminal error; oversized reads report the file's real size; listings are capped with guidance; a connection without a host is refused instead of falling back to `localhost:8080`; reconnect time is not charged to the command; `destroy` skips the readiness poll; and every sandbox carries `created-by: airflow` metadata. Two threads asked for evidence against a real server rather than fixtures. Both are now covered, against a local `opensandbox-server` on Docker: | case | result | | --- | --- | | server with **no egress sidecar**, default deny-all spec | server refuses the create -- `400 [SANDBOX::INVALID_PARAMETER] egress.image must be configured when networkPolicy is provided` -- surfaced as `SandboxTerminalError`. Fails closed at create; the read-back stays as the backstop for a build that would accept it silently. | | real timeout, 5s budget against `sleep 60` | `elapsed=6.0s exit_code=-1 timed_out=True sandbox_terminated=False`, stderr `signal: killed` | | signal kill, `sh -c 'kill -9 $$'` on a 30s budget | `elapsed=1.0s exit_code=-1 timed_out=False sandbox_terminated=False`, stderr `signal: killed` | The third is the one the `exit_code` thread was about: execd words `error.value` as the prose `signal: killed`, which `_infer_foreground_exit_code` cannot parse. It is now an ordinary failed command, and because it died well inside its budget it is not misreported as a timeout either. The system test also passed end to end against that server (`1 passed in 20.83s`), exercising create, two commands, write, read, mkdir, list, the egress policy read-back and teardown, with no containers left behind. Rebased onto current `main`, which reorganised the provider docs twice today, so the backend is documented in `docs/sandbox/backends.rst` beside Modal and `sbx` rather than in `toolsets.rst`. Also in this push: the SDK moves to `opensandbox>=1.1.0`, which I checked is API- and behaviour-identical for everything this backend touches, including the disabled SSE read timeout, `_infer_foreground_exit_code` and the unbounded frame buffer; it needs a temporary `exclude-newer-package` override until the rolling window passes 2026-09-21. Two packaging fixes came out of that review: the SDK had been listed under the `llamaindex` extra rather than `[dependency-groups].dev`, which both dragged it into `[llamaindex]` and made the unit tests `importorskip` away in the main lane, and the extras table in `docs/index.rst` had mismatched column rules with no `sandbox-opensandbox` row. 63 unit tests, ruff and the applicable prek hooks pass locally; mypy is clean on the changed files with the SDK installed. --- Drafted-by: Claude Code (Opus 5); reviewed by @zozo123 before posting -- 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]
