villebro commented on PR #43768:
URL: https://github.com/apache/superset/pull/43768#issuecomment-5497647863
Thanks for the review — addressed in c1d1dac8eb.
**P2 (task-body failure + cleanup failure erasing the original error) —
fixed.** Root cause was exactly as you described: `error_properties()` built
the merged terminal-error blob but never updated
`TaskContext._properties_cache`, and the DAO writes a *complete* properties
column ("caller manages merging in their cache"), so the cleanup-failure
fallback rewrote a blob from the stale cache. Fix keeps the cache authoritative:
- `error_properties()` now writes the merged result back to
`_properties_cache`, so it reflects what the caller commits.
- `_write_handler_failures_to_db` reads the original error from that cache
(not the stale `self._task` snapshot), and the terminal-status-preserving
fallback records **only** the handler detail under distinct
`private.framework.cleanup_*` keys — so the body error's
`error_message`/`exception_type`/`stack_trace` survive.
- Added a regression test
(`test_cleanup_failure_after_body_failure_preserves_original_error`) for the
exact body-then-cleanup sequence.
**Follow-up: `WEBSOCKET_ENABLE` sufficient without a usable URL** —
addressed, since it directly undermines the new bounded-wait guarantee.
`wsEnabled` now requires `WEBSOCKET_URL` too: without a URL a socket can never
open (so no `unhealthy` ever fires and waiters would hang), so polling stays on
as the safe fallback. Test added. (The `typeof WebSocket === 'undefined'`
SSR/test case is left as-is.)
**Follow-up: inline DAG wait not bounded by `TaskOptions.timeout`** — kept
as a roadmap item per your note; the unbounded wait is intentional (it mirrors
the async path's unbounded defer-retry — wait until the prerequisite is
terminal). Added a code comment documenting that and the potential future bound.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
