codeant-ai-for-open-source[bot] commented on PR #36893: URL: https://github.com/apache/superset/pull/36893#issuecomment-3712625919
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36893/files#diff-40640739c760fc10b2c86514e363f5d2932d1c73b2388d5ecf58fbe10ca23aabR71-R81'><strong>Race condition</strong></a><br>The code calls `setupClient(...)` (which calls `SupersetClient.configure(...).init()`) but does not await the client's init promise before calling `SupersetClient.get(...)` to fetch the language pack. `setupClient` currently starts initialization asynchronously and returns immediately, so `SupersetClient.get` can run before the client is ready, causing failed requests or unpredictable behavior.<br> - [ ] <a href='https://github.com/apache/superset/pull/36893/files#diff-40640739c760fc10b2c86514e363f5d2932d1c73b2388d5ecf58fbe10ca23aabR47-R127'><strong>Cached rejected promise</strong></a><br>`initPromise` is a module-scoped cache of the initialization promise. If initialization fails, `initPromise` remains a rejected promise, and subsequent callers will receive that same rejected promise (no retry). This can prevent recovery or retries without a page reload.<br> - [ ] <a href='https://github.com/apache/superset/pull/36893/files#diff-61daded25bf82bf753850ef1808fd5f264d20a5fe8e614e91bfa09caf5dc4350R26-R36'><strong>Error handling</strong></a><br>The async IIFE awaits initPreamble() inside a try/finally but does not catch or log initialization errors. If initPreamble rejects, the promise will still reject (possibly creating an unhandled rejection) while the finally block renders the app. Consider explicit catch + logging so failures are visible and can be analyzed.<br> - [ ] <a href='https://github.com/apache/superset/pull/36893/files#diff-61daded25bf82bf753850ef1808fd5f264d20a5fe8e614e91bfa09caf5dc4350R28-R30'><strong>Blocking render</strong></a><br>initPreamble() waits for the language pack and other setup. While this fixes the race, it may delay initial render (e.g., slow network). Consider a bounded timeout or graceful fallback to avoid very long blank screens.<br> - [ ] <a href='https://github.com/apache/superset/pull/36893/files#diff-8a676fb002920d1d142fa9b19ded1ecb7b8b8d682cf9b9fe80f01b1a2314c86dR478-R480'><strong>Locale precision</strong></a><br>The new code uses `locale.language` when `locale` is present. `locale.language` only returns the language subtag (e.g. "zh") and drops territory/variant subtags (e.g. "zh_CN"). This can result in sending a less-specific locale to the frontend and cause incorrect or incomplete translations/pluralization when a territory is required.<br> </td></tr> </table> -- 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]
