aminghadersohi commented on PR #44581: URL: https://github.com/apache/superset/pull/44581#issuecomment-5923708413
> `admission_counts` looks right, nice unifying the transport-thread bound in there too. Lifting the block. > > Put together a PR against your branch with a few smaller things from a deeper pass, rebased onto your latest since most of it had landed independently: [aminghadersohi#1](https://github.com/aminghadersohi/superset/pull/1). What's left: > > * `on_list_tools` and both search-transform `_get_visible_tools` overrides call `run_in_metadata_thread` with no try/except, so a setup failure there (pool exhausted, same scenario as above) propagates instead of failing open like the filter itself does. > * `QueryCancellation.capture()` doesn't reset `cancel_id` before a re-capture, so a failed second capture re-dispatches cancellation against the first, already-finished statement. > * `WorkerPool.cancel()` only releases its permit on `RuntimeError`; anything else leaks it. > * `run_in_worker`'s `pool.submit()` is outside the `try/finally` resetting `_mcp_user_id_var`, so a busy-pool rejection leaves a stale user id for audit logging. > * `run_in_worker`/`TransportContext.forward` both relabel any `TimeoutError` as our deadline, including one actually raised by the tool/driver. > > Take whatever's useful, happy to let the rest drop. i mergdd it in -- 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]
