codeant-ai-for-open-source[bot] commented on PR #36767: URL: https://github.com/apache/superset/pull/36767#issuecomment-3674995110
## 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/36767/files#diff-64900fd3f4e1f2c84ff1d1a922d9934cdc8ddefab4ef9626e45e3e426b0b45b1R567-R569'><strong>Attribute access</strong></a><br>The new code uses attribute access on `extension.manifest` and `backend` (e.g. `.backend`, `.entryPoints`). If `extension.manifest` or `backend` can still be plain dicts (or None) in some code paths, this will raise AttributeError at runtime. Ensure the manifest is always an object with attributes or add a safe fallback to support dicts.<br> - [ ] <a href='https://github.com/apache/superset/pull/36767/files#diff-15125851b69186fcdea89869d22e3b02e3e793c33306e81eb77274ac98cd0409R164-R184'><strong>Missing field (regression)</strong></a><br>The new build_extension_data no longer exposes the previous "extensionDependencies" field from the manifest. This is a behaviour change that may break callers / consumers that expect that key.<br> - [ ] <a href='https://github.com/apache/superset/pull/36767/files#diff-f6cf3b140848d761d2d23724c4b26e7d4c88ca5bbf16bfdcc9387c42440b364bR37-R55'><strong>Type change for exposes</strong></a><br>`ModuleFederationConfig.exposes` is typed as `list[str]` but the previous TypedDict used `dict[str, str]`. This is a semantic change that can break consumers expecting a mapping of exposed names to paths. Confirm intended shape and adjust accordingly.<br> - [ ] <a href='https://github.com/apache/superset/pull/36767/files#diff-15125851b69186fcdea89869d22e3b02e3e793c33306e81eb77274ac98cd0409R126-R140'><strong>Generic exceptions</strong></a><br>Validation and parsing failures raise plain Exception instances and propagate the raw ValidationError message. Use of broad Exception types makes it harder for callers to handle specific failure modes and may leak internal validation details.<br> - [ ] <a href='https://github.com/apache/superset/pull/36767/files#diff-d75ef7319112f6821fdf67eb488c07acd38df00ab55c200758bf3633e1e95e4aR61-R71'><strong>Broad exception handling / logging</strong></a><br>The try/except around extension loading catches all Exceptions and logs a short message. This can hide root causes when diagnosing failed loads. Consider narrowing exception types or including tracebacks (e.g., using `logger.exception` or `exc_info=True`) so failures are easier to debug.<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]
