You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "collinmcnulty (via GitHub)" <gi...@apache.org> on 2026/10/13 13:44:46 UTC
[PR] Allow HITL assigned_users as identifiers resolved at task start (draft) [airflow]
collinmcnulty opened a new pull request, #74652:
URL: https://github.com/apache/airflow/pull/74652
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
**Draft for design feedback.** This is a first sketch to discuss the shape of the API before polishing it, not a finished proposal.
## Why
`HITLOperator(assigned_users=...)` needs `{"id": ..., "name": ...}`, where `id` is whatever the auth manager's `user.get_id()` returns (for example an opaque id such as a cuid, or the integer id on FAB). Dag authors usually know a person's email, not that id, so they have to look it up themselves, outside Airflow.
There is also a failure-mode question. Resolving the id inside the Dag file would put a network or user-store lookup in the parse path, so one bad entry could break parsing of the whole Dag. Resolving it when the task starts means a bad entry fails only that task instance.
This was motivated by a user (Collin at Astronomer) who generates one approval form per engineer at runtime and wants the form restricted to a specific person identified by email. A mistyped or unknown email should fail just that mapped task, not the Dag.
## What this changes
- `HITLOperator.assigned_users` also accepts string identifiers (alone or in a list, mixed with `{id, name}` entries). Existing usage is unchanged.
- The execution API `HITLDetailRequest` gets an optional `assigned_user_identifiers: list[str]` field (Cadwyn version change added to the unreleased `2026-10-30` version; stripped from responses for older clients).
- New optional `BaseAuthManager.resolve_hitl_user(identifier) -> {"id", "name"} | None`. Default returns `None`.
- `upsert_hitl_detail` resolves identifiers when the task starts. If one cannot be resolved it returns 422 with the identifier in the message, so only that task instance fails.
- `SimpleAuthManager` resolves configured usernames (any name in all-admins mode, where `assigned_users` is not enforced anyway).
- Task SDK plumbing (`upsert_hitl_detail`, supervisor handler, client, generated models/schema) and docs.
- The provider only sends the new field when identifiers are used, so DAGs that do not use them keep working with an older task SDK.
## Open design questions
1. **Where should resolution live?** I put it on the auth manager because it already owns `get_id()` and `is_authorized_hitl_task`, and the identifier-to-id mapping has to agree with the first. The alternative is a separate endpoint or a user-lookup interface on the auth manager.
2. **Other auth managers.** Only the default (resolves nothing) and `SimpleAuthManager` are implemented here. FAB (look up by email/username in `ab_user`, `id` as `str`), Keycloak, AWS and Astro's own auth manager would need implementations. What should the default be: fail (as here), or pass the string through as the id?
3. **Wire format.** A separate `assigned_user_identifiers` field keeps `{id, name}` unchanged. An alternative is making `HITLUser.name` optional or adding a discriminator.
4. **Provider version gate.** The new task SDK kwarg is only sent when identifiers are used. A proper `AIRFLOW_V_3_x_PLUS` gate and a clearer error on older cores depend on which release this lands in.
5. **Match semantics.** Exact match on one identifier type, or let each auth manager decide (email vs username)?
## Testing
- `airflow-core`: execution API HITL tests, base/simple auth manager tests: 289 passed.
- `providers/standard`: `test_hitl.py`: 87 passed, 1 skipped.
- `task-sdk`: HITL, client and supervisor test files: 494 passed, 3 failed, 6 errors. The failures and errors are in `TestWatchedSubprocess` (subprocess-based DAG-run log assertions, e.g. `test_run_simple_dag`); I did not compare against a clean `main` baseline, so I cannot say whether they pre-exist on this machine.
- `ruff format` / `ruff check` clean on the changed files. The full `prek` suite was not run.
- `_generated.py` and `schema.json` were edited by hand to match the generator output instead of regenerated.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (Claude Code)
Generated-by: Claude Code following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
No newsfragment: I was not certain this counts as a significant user-facing change; happy to add one if reviewers want it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Allow HITL assigned_users as identifiers resolved at task start (draft) [airflow]
Posted by "collinmcnulty (via GitHub)" <gi...@apache.org>.
collinmcnulty closed pull request #74652: Allow HITL assigned_users as identifiers resolved at task start (draft)
URL: https://github.com/apache/airflow/pull/74652
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org