codeant-ai-for-open-source[bot] commented on PR #36739: URL: https://github.com/apache/superset/pull/36739#issuecomment-3674746195
## 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/36739/files#diff-f86b52ad6b0cc7a6a20560346936133f084b6e29bf18c3f2ca17d363274c71b4R132-R139'><strong>Sensitive logging</strong></a><br>The except block logs `str(e)` through `ctx.error` which may include SQL text, parameters, or other sensitive information. Emitting raw exception messages to client-visible logs or contexts can leak secrets or internal details. The same applies to any detailed SQL or DB error messages forwarded un-sanitized.<br> - [ ] <a href='https://github.com/apache/superset/pull/36739/files#diff-f86b52ad6b0cc7a6a20560346936133f084b6e29bf18c3f2ca17d363274c71b4R170-R175'><strong>Large-memory conversion</strong></a><br>Converting a potentially large DataFrame to a list of dicts via `df.to_dict(orient="records")` may consume a lot of memory and cause OOM for large result sets. Consider streaming, pagination, or enforcing/validating limits before materializing full results.<br> - [ ] <a href='https://github.com/apache/superset/pull/36739/files#diff-5752907a34d06d9abfac36851fc41584cb5459c13265d13c21af2f39632519caR48-R50'><strong>Backwards compatibility: parameters -> template_params</strong></a><br>The request schema removed `parameters` and adds `template_params`. External callers (API clients, frontends, older integrations) may still send `parameters`. Consider maintaining an alias or migration path to avoid breaking external callers.<br> - [ ] <a href='https://github.com/apache/superset/pull/36739/files#diff-f86b52ad6b0cc7a6a20560346936133f084b6e29bf18c3f2ca17d363274c71b4R108-R110'><strong>Blocking call in async</strong></a><br>The code calls the synchronous Database.execute(...) API directly from an async FastMCP tool. If `database.execute` is CPU-bound or performs I/O (likely), this will block the event loop and degrade concurrency. Consider running the call in a thread executor or using an async database API.<br> - [ ] <a href='https://github.com/apache/superset/pull/36739/files#diff-5752907a34d06d9abfac36851fc41584cb5459c13265d13c21af2f39632519caR65-R70'><strong>Validator decorator correctness</strong></a><br>The field validator for `sql` is annotated with both `@field_validator("sql")` and `@classmethod`. The decorator ordering/combination can be incompatible with pydantic's expected callable signature and may cause the validator not to run or to raise at model creation time. Verify this is correct for the pydantic version in use and add tests to ensure the validator fires as expected.<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]
