villebro commented on PR #43627: URL: https://github.com/apache/superset/pull/43627#issuecomment-5447541799
Thanks for the review — addressed in `58893a8a54`, plus a fix for the MySQL CI failure: - **Reaper aborts on one DB error (Major):** each candidate is now processed in its own `try/except` that rolls back and continues on failure (metric `gtf.task.reap_error`), so a transient error can neither abort the loop nor escape as a non-`CommandException` and skip the retention prune pass. - **Reaper stale-snapshot clobber (Major):** the FAILURE transition now re-reads the task's current `properties` immediately before the CAS (instead of reusing the possibly batch-stale `find_orphaned` snapshot), so a concurrently-written runtime property isn't overwritten. The status CAS remains the authority; for a genuine orphan the worker is dead and cannot race it. - **`None` query arg breaks Ocient/Impala (Major):** replaced `None` with a minimal `_CancellationQuery` stand-in exposing `database`, `id=None`, and the `extra` accessors. Impala/Ocient no longer `AttributeError`; an engine that cancels by query id sees `id=None` and declines gracefully via `validate_cancel_query_id` rather than raising. - **Regression test can pass without finalizing (Minor):** the executor abort/timeout tests now assert the `ABORTING → TIMED_OUT`/`ABORTED` transition was actually issued (correct `new_status` + `expected_status=ABORTING`), not just that the result echoes the fixture. **CI:** the `test-mysql` failure was `test_touch_heartbeat_does_not_advance_changed_on` comparing an in-memory `changed_on` (microseconds) against a post-`refresh()` value, which MySQL `DATETIME` truncates to whole seconds (Postgres/SQLite keep microseconds, so they passed). Fixed by reading the baseline back at column precision first. -- 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]
