geido commented on PR #43719:
URL: https://github.com/apache/superset/pull/43719#issuecomment-5540731887
Bito's run #a15641 caught a real one, so noting it here rather than letting
it sit as an unactioned bot suggestion.
The extension fallback I added for @EnxDev's finding routed a *handled*
failure through the shared `fetchDrillInfo` error path, which logs. In a
deployment that registers `load.drillby.options` because the REST endpoint is
unreachable — the exact case the fallback exists for — that meant
`logging.error('Failed to load dataset: ')` on every dashboard load, for a
failure the code recovers from by design.
```ts
// `bestEffort` callers recover from a failure themselves, so it is not worth
// logging: a deployment that registers the drill-by extension because this
// endpoint is unreachable would otherwise log on every dashboard load.
const fetchDrillInfo = async ({ bestEffort = false } = {}) => {
...
} catch (error) {
if (!bestEffort) {
logging.error('Failed to load dataset: ', error);
}
supersetGetCache.delete(endpoint);
throw error;
}
};
```
The cache eviction and the rethrow stay on both paths — only the log is
suppressed, and only for the caller that handles the error itself. Where the
API is the sole label source, a failure is still fatal and still logged.
Two tests, one per branch, so the distinction can't erode:
- `keeps extension drill-by options when the label fetch fails` now also
asserts `logging.error` was **not** called
- `logs when the API is the only source and it fails` asserts it **was**
The first fails against the previous commit. 282 passing across 33 suites,
pre-commit green.
For the record on the other bot item from run #4d4a23 — extracting a shared
base for `DatasetColumnDrillInfoSchema`/`DatasetMetricDrillInfoSchema` — still
declining. They share only `verbose_name`, and the field that would actually
diverge is the required identifier that differs between them.
--
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]
