codeant-ai-for-open-source[bot] commented on PR #36677: URL: https://github.com/apache/superset/pull/36677#issuecomment-3661309816
## 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/36677/files#diff-83f266461d3ba6c4fb9ecd019c9a219a10583e7d4525d44eaa41278fc5643427R83-R96'><strong>Metadata type validation</strong></a><br>The code assumes the value returned by load_yaml(...) is a mapping and directly iterates over metadata.items(). If the YAML content is not a dict (e.g. a string, list, or None), this will raise an AttributeError. Validate that metadata is a dict before using .items().<br> - [ ] <a href='https://github.com/apache/superset/pull/36677/files#diff-83f266461d3ba6c4fb9ecd019c9a219a10583e7d4525d44eaa41278fc5643427R196-R200'><strong>Metadata None handling</strong></a><br>When merging template metadata into the dashboard config, the code only checks for the presence of the "metadata" key. If config["metadata"] exists but is None or not a dict, assigning config["metadata"]["template_info"] will raise a TypeError. Ensure config["metadata"] is a dict before assignment.<br> - [ ] <a href='https://github.com/apache/superset/pull/36677/files#diff-83f266461d3ba6c4fb9ecd019c9a219a10583e7d4525d44eaa41278fc5643427R94-R96'><strong>Filtering semantics</strong></a><br>The comprehension uses "and v" to filter values, which removes falsy but meaningful values such as False, 0, empty list, or empty string. This may drop legitimate template fields (e.g., explicit False for boolean fields). Consider filtering on None instead, or otherwise decide which falsy values should be preserved.<br> - [ ] <a href='https://github.com/apache/superset/pull/36677/files#diff-b6a186993d7414cb29c7015ca427727a7191ad89ade1714cf1c659a38eae81ebR239-R261'><strong>Import Normalization</strong></a><br>The pre-load normalization in ImportV1ColumnSchema only fixes `extra` (string -> dict). It doesn't normalize empty strings for `python_date_format` or `datetime_format` like other schemas do for fields such as `template_params`. If incoming exports use empty strings for these fields, they may be stored incorrectly instead of being set to None.<br> - [ ] <a href='https://github.com/apache/superset/pull/36677/files#diff-b6a186993d7414cb29c7015ca427727a7191ad89ade1714cf1c659a38eae81ebR260-R260'><strong>Missing Validation</strong></a><br>The newly added `datetime_format` field on ImportV1ColumnSchema has no validation applied (length or date-format validation). This is inconsistent with DatasetColumnsPutSchema which validates `datetime_format` using `Length` and `validate_python_date_format`. Unvalidated values may allow invalid date formats to be imported and later cause parsing/runtime errors.<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]
