potiuk commented on PR #67505:
URL: https://github.com/apache/airflow/pull/67505#issuecomment-4627287228
Good call on the granularity — collapsed it. `prepare_to_enrich_errors`
stays in its own guard (it runs `sentry_sdk.init()`), and `add_tagging` +
`add_breadcrumbs` now share a single try/except (one `sentry_enrichment_failed`
warning) instead of one each.
On "can these ever fail / all local operations" though — two of the three
aren't local:
- **`add_breadcrumbs`** isn't local at all: it calls
`RuntimeTaskInstance.get_task_breadcrumbs(...)`, which does
`SUPERVISOR_COMMS.send(GetTaskBreadcrumbs(...))` — a blocking Execution-API/IPC
round-trip to the supervisor. That can raise (IPC/API error), and today it
would surface as a task failure.
- **`prepare_to_enrich_errors`** runs `sentry_sdk.init(dsn=…)` plus
`conf.getimport("sentry", "before_send"/"transport")` — SDK/transport init and
importing a configured callable can fail on misconfiguration.
- `add_tagging` is the one that's genuinely local (`scope.set_tag` +
attribute reads), so you're right there — which is partly why it's now folded
into the shared guard rather than its own.
So this isn't purely defensive against the impossible — `add_breadcrumbs` in
particular is a remote call we don't want taking down a task. Mind another look?
---
Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk 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]
