mikebridge commented on PR #42760: URL: https://github.com/apache/superset/pull/42760#issuecomment-5376395791
Rebased and hardened — ready for a second look. Head is now `56a0cf02ec` (three commits over the approved `4b1166afb0`... rather, the same five commits rebased onto current master plus two fix commits): **Rebase** (@rusackas's ask): onto master `09bd8c98de` — 430 commits of drift, conflicts were three additive collisions in `query_object.py`/`query_context_factory.py` (master's `extra_cache_keys` sorting + `datasource_model_instance` beside this PR's `force_query`), all resolved keep-both. **@sadpandajoe's three findings**: all confirmed real and fixed in `e5d5077568` with regression tests that fail against the previous code — details in each thread. Thank you for the careful pass; the `results=None` one was worse than reported (the AttributeError escaped the transformation boundary, so cache hits 500'd). **@rusackas's CI nit**: done in `e5d5077568` — new `semantic-layers` change-detector group; `test-semantic-cache-coordination` now runs only when `superset/semantic_layers/`, its tests, superset-core's semantic-layer types, or the workflow/detector themselves change. **Additional hardening** (`56a0cf02ec`) from a data-systems review pass over the full branch before pushing: - `SemanticCacheService.execute` now degrades on *any* exception at each cache stage instead of only its typed errors — cached pickles outlive the code that wrote them, and a shape-incompatible entry from an older release previously failed every request on the view until TTL expiry. Unexpected failures log + count against the stage metrics and fall through to the provider. - `initialize_semantic_cache` fails closed on Sentinel-backed data caches (new `UNSUPPORTED_BACKEND` reason): flask-caching's `RedisSentinelCache` reads via `slave_for` replicas while writing the master, so the lease-guarded bucket read-modify-write loses read-your-writes under replication lag. Routing repository reads through the write client (which would re-enable Sentinel deployments) is tracked as follow-up work, along with the review's lower-severity items (lease write fencing, capability-flag collation contract documentation, payload size caps, guest-token bucket churn). **Verification**: 607 unit tests green, the `semantic_layers` 100% coverage gate holds, and every fix's regression test was control-run red against the unfixed code. *Rebase and fixes authored with AI assistance (Claude); all changes reviewed, tested, and control-verified as described.* -- 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]
