no-hup commented on PR #43990: URL: https://github.com/apache/superset/pull/43990#issuecomment-5775346270
You're right, and thanks for pulling the prefetch out to check — that's the gap. The test mirrors the api.py sequence (dump → prefetch → read editors/viewers) rather than driving the handler, so it pins the ordering but not the call itself. Removing `prefetch_chart_access` from api.py leaves it green, exactly as you saw. The reason I stopped short of `inspect.unwrap(DashboardRestApi.get)` is that the real handler runs `schema.dump(dash)` first, and that serialization issues its own queries over the dashboard — so a statement count around the whole call isn't clean, it has to isolate the editors/viewers reads from the dump's own SQL. Doable, just more than the DAO-level test needed. I'll add the handler-level version: unwrap `get`, invoke it in a request context with `can_access_chart` stubbed to read editors/viewers, count only those reads, and confirm it fails when the prefetch line is removed. I don't want to push it until I've actually run it green both ways instead of eyeballing it — my local unit env is currently wedged on an unrelated flask-caching init error, so it'll take me a moment to sort that first. Happy to leave it as a follow-up if you'd rather not hold the PR on a non-blocking test. -- 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]
