aminghadersohi commented on PR #45283: URL: https://github.com/apache/superset/pull/45283#issuecomment-6190261374
The acceptance evidence records FAILED instead of STOPPED in 16/36 API and 22/36 browser Stops; the legacy error handler's status-read/unconditional-FAILED-commit sequence allowed a committed Stop and its end_time to be overwritten. Fixed the committed-Stop overwrite in `594710042339ce22ef7bf37c64fce54b00fdeb2b`; head `e5421d693dddcccac4edc90e486c27a0475c8dd1` also updates the shared-session mocks in the error-payload tests. Legacy error, backend-failure and success publication use the same conditional terminal-write guard as Celery; a losing error write returns STOPPED without failure fields. Regression tests: `test_get_sql_results_preserves_stop_during_postgres_cancellation`, `test_legacy_terminal_write_preserves_concurrent_stop`, and `test_legacy_cancel_error_preserves_committed_stop`. Running `python3 -m pytest -q tests/unit_tests/sql_lab_test.py tests/testcontainers/db_engine_specs/test_postgres.py -k 'test_get_sql_results_preserves_stop_during_postgres_cancellation or test_legacy_terminal_write_preserves_concurrent_stop or test_legacy_cancel_error_preserves_committed_stop' -m 'not manual and not llm_compliance' --tb=short` against the original legacy source at `ae40a754f3b05f1e25cb32d933ee57e733e84ae8` yielded **7 failed, 7 passed**; after the change, **14 passed**. The Postgres cases use a disposable Postgres 17 container, real pg_cancel_backend/pg_terminate_backend, independent metadata sessions, and return_results=True/False. They assert STOPPED, the Stop's end_time, and no failure payload. Validation: - `pytest -q tests/unit_tests/sql/ tests/unit_tests/daos/ tests/unit_tests/sql_lab_test.py -m 'not manual and not llm_compliance' --tb=short`: 2673 passed, 2 skipped. - `pytest -q tests/testcontainers/db_engine_specs/test_postgres.py -m testcontainers --tb=short`: 10 passed. - `pytest -q tests/unit_tests/test_sqllab_error_stacktrace.py tests/integration_tests/queries/api_tests.py tests/integration_tests/sqllab_tests.py -k 'test_stop_query or test_cancel_query_implicit or stacktrace' -m 'not manual and not llm_compliance' --tb=short`: 11 passed. - Staged-file `pre-commit run` and `pre-commit run mypy`: passed. Branch-wide `pre-commit run --files ...`: frontend type-check failed with TS6305 missing built workspace declarations; other applicable hooks passed. No frontend files changed in these commits. Siblings checked: - superset/sql_lab.py:799 — fixed in 594710042339ce22ef7bf37c64fce54b00fdeb2b - superset/sql_lab.py:849 — fixed in 594710042339ce22ef7bf37c64fce54b00fdeb2b - superset/sql_lab.py:395 — not affected: timeout stages local status and delegates terminal persistence to the guarded error handler - superset/sql/execution/celery_task.py — not affected: terminal writes retain the same conditional guard, moved unchanged into shared utilities; durable-target and timeout precedence are unchanged - superset/sql/execution/utils.py:135 — not affected: set_query_stopped already conditionally excludes completed outcomes - superset/sql/execution/utils.py:51 — not affected: result cleanup helpers roll back/read metadata and delete cache entries, without terminal Query writes Remaining: cancellation-before-Stop ordering is unchanged. The original full HTTP/browser acceptance matrix was not rerun. -- 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]
