kaxil opened a new pull request, #74302: URL: https://github.com/apache/airflow/pull/74302
`ModalSandboxBackend` could only authenticate with ambient Modal credentials (`MODAL_TOKEN_ID` / `MODAL_TOKEN_SECRET` in the worker environment, or `~/.modal.toml`). A deployment that keeps secrets in a secrets backend, or wants a different Modal workspace per Dag, had to put a token in the worker environment to use the sandbox toolset on Modal. The backend now takes `modal_conn_id` (default `modal_default`) and builds its Modal client through the Modal provider's `ModalHook` (#73418), whose docstring already names this backend as a caller: ```python SandboxToolset(ModalSandboxBackend(modal_conn_id="team_modal")) ``` The connection's token and optional `environment` are used for creating, reattaching to and destroying sandboxes. Without a `modal_default` connection the hook falls back to the ambient credentials, so an Airflow 3 deployment that relies on them today keeps working with no configuration. ## Design rationale **The Modal backend now needs Airflow 3.** The `modal` extra adds `apache-airflow-providers-modal`, which requires Airflow 3.0, while Common AI supports 2.11. This follows the precedent of the `skills` and `git` extras, and `installation.rst` now lists `modal` beside them. Making the provider optional would have kept the backend on Airflow 2.11, but the connection is what this backend offers over pydantic-ai's own `ModalSandbox` capability, so a Modal backend without it has little reason to exist. Common AI's Airflow 2 support is unreleased (0.10.0 floors Airflow 3), so no released user loses anything. **Credential problems fail the task, never the model.** A named connection that does not exist, or one with only half of the token, raises `SandboxTerminalError` naming the connection. Nothing the model does can fix it. `destroy` raises too, so a cleanup task with a misspelled connection fails instead of leaving the sandbox billing until its lifetime ends; the toolset's own teardown already catches and logs this, so a finished agent run is never failed by teardown. When Modal rejects a token that came from the connection, the error says to update the connection rather than to set worker environment variables, which would change nothing. **Nothing is read at Dag-parse time.** The hook is constructed in `__init__`, which reads no connection and opens no client; both wait for the first sandbox. Verified live against Modal: with no `MODAL_TOKEN_*` in the environment and no `~/.modal.toml`, a backend with `modal_conn_id` created a sandbox, ran a command, and a second backend instance reached the same sandbox by handle and destroyed it. The default path in the same environment failed with Modal's "Token missing", confirming the run used only the connection. That run was on the revision before review fixes, which moved the hook's construction into `__init__` and changed error reporting but not the credential path; the unit suite covers both. ## Gotchas - `apache-airflow-providers-modal` is unreleased (0.1.0), so the Common AI release carrying this needs to ship in the same wave as the Modal provider or after it. - A user who upgrades Common AI without the `modal` extra gets an optional-feature error importing `ModalSandboxBackend` until the Modal provider is installed. - `ModalHook` treats a missing `modal_default` as permission to use ambient credentials, and the Task SDK reports a secrets-backend outage as the same not-found error. With `modal_default` in a secrets backend and Modal credentials also on the worker, an outage would provision in the worker's Modal account. That is the hook's behavior for every caller, so it is left for a follow-up in the Modal provider rather than special-cased here. --- * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged. * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- 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]
