rusackas commented on PR #43316: URL: https://github.com/apache/superset/pull/43316#issuecomment-5372791980
Went through this properly before approving, not just trusting the bot summaries. Good consolidation overall, and every open review thread turned out to already be fixed in code (moved `__init__.py`'s contents to `base.py`, `get_backend()` scoped to `DISTRIBUTED_COORDINATION_CONFIG` only, `wait_for_signal` checks before subscribing), so I replied and resolved those. One real blocker though: CI's actually red here, not flaky. `test-mysql`/`test-postgres`/`test-postgres-required`/`test-sqlite` all fail the same 6 tests in `tests/integration_tests/async_events/api_tests.py::TestAsyncEventApi`. Cause is in `run_test_with_cache_backend`: `self.login(ADMIN_USERNAME)` now runs before `async_query_manager_factory.init_app(app)`, so `login()`'s request trips Flask's `after_request` setup-method guard before `init_app` gets a chance to re-register it. Move `init_app(app)` (with the `_got_first_request` reset) back ahead of `login()` and it should go green. That also explains the scary-looking Codecov `-8.90%` line — mysql/postgres/sqlite just never uploaded a report because the run failed. One optional follow-up for later, not blocking: CodeAnt flagged that a mid-wait Redis outage in `wait_for_signal` propagates instead of falling back to DB polling. Checked it against the pre-refactor code, that's pre-existing (`_wait_via_pubsub` had the identical gap), so not something this PR needs to fix, but could be worth hardening separately sometime. -- 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]
