codeant-ai-for-open-source[bot] commented on code in PR #43337:
URL: https://github.com/apache/superset/pull/43337#discussion_r3815308261
##########
superset/charts/schemas.py:
##########
@@ -972,21 +980,33 @@ class ChartDataGeodeticParseOptionsSchema(
class ChartDataPostProcessingOperationSchema(Schema):
+ _builtin_ops = [
+ name
+ for name, value in inspect.getmembers(pandas_postprocessing,
inspect.isfunction)
+ ]
+
operation = fields.String(
metadata={
"description": "Post processing operation type",
"example": "aggregate",
},
required=True,
- validate=validate.OneOf(
- choices=[
- name
- for name, value in inspect.getmembers(
- pandas_postprocessing, inspect.isfunction
- )
- ]
- ),
)
+
+ @validates("operation")
+ def validate_operation(self, value: str, **kwargs: object) -> None:
+ from flask import current_app
+
+ extra_op_names = [
+ fn.__name__
+ for fn in
current_app.config.get("EXTRA_PANDAS_POSTPROCESSING_OPS", [])
+ ]
Review Comment:
**Suggestion:** The configuration is typed as a list of arbitrary `Callable`
objects, but this assumes every callable has `__name__`. Valid callable
instances and `functools.partial` objects do not necessarily provide that
attribute, so configuring one causes request validation to raise
`AttributeError` instead of registering or rejecting the operation cleanly. Use
an explicit operation-name contract or support callable objects consistently in
both validation and dispatch. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Configured callable-instance operations break chart-data validation.
- ❌ Built-in post-processing requests fail with `AttributeError`.
- ⚠️ Custom operation dispatch repeats the same naming assumption.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/schemas.py
**Line:** 1000:1003
**Comment:**
*Type Error: The configuration is typed as a list of arbitrary
`Callable` objects, but this assumes every callable has `__name__`. Valid
callable instances and `functools.partial` objects do not necessarily provide
that attribute, so configuring one causes request validation to raise
`AttributeError` instead of registering or rejecting the operation cleanly. Use
an explicit operation-name contract or support callable objects consistently in
both validation and dispatch.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=ccb3fc2950ce2edb24ad4280c3f4d41bfe80f10d5f48b89db93c6688e3e23be0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=ccb3fc2950ce2edb24ad4280c3f4d41bfe80f10d5f48b89db93c6688e3e23be0&reaction=dislike'>👎</a>
##########
superset/charts/schemas.py:
##########
@@ -972,21 +980,33 @@ class ChartDataGeodeticParseOptionsSchema(
class ChartDataPostProcessingOperationSchema(Schema):
+ _builtin_ops = [
+ name
+ for name, value in inspect.getmembers(pandas_postprocessing,
inspect.isfunction)
+ ]
+
Review Comment:
**Suggestion:** `inspect.getmembers` collects every function exported by
`pandas_postprocessing`, including helper functions such as `escape_separator`
and `unescape_separator` that are imported into the package but are not
DataFrame post-processing operations. The schema therefore accepts these helper
names, and execution invokes them with a DataFrame and post-processing options,
producing incorrect transformations or runtime errors. Restrict the built-in
allowlist to actual post-processing operations. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Malformed post-processing requests can fail chart-data responses.
- ⚠️ `escape_separator` and `unescape_separator` are string helpers.
- ❌ DataFrame results may receive unintended helper transformations.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/schemas.py
**Line:** 983:987
**Comment:**
*Api Mismatch: `inspect.getmembers` collects every function exported by
`pandas_postprocessing`, including helper functions such as `escape_separator`
and `unescape_separator` that are imported into the package but are not
DataFrame post-processing operations. The schema therefore accepts these helper
names, and execution invokes them with a DataFrame and post-processing options,
producing incorrect transformations or runtime errors. Restrict the built-in
allowlist to actual post-processing operations.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=ffec24c2dec43517ac74bcf0fbbe86bb38426b0b1bc5ba2ed0378184657e089c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=ffec24c2dec43517ac74bcf0fbbe86bb38426b0b1bc5ba2ed0378184657e089c&reaction=dislike'>👎</a>
##########
superset/config.py:
##########
@@ -358,6 +358,15 @@ def _try_json_readsha(filepath: str, length: int) -> str |
None:
# Extends the default SQLGlot dialects with additional dialects
SQLGLOT_DIALECTS_EXTENSIONS: DialectExtensions | Callable[[],
DialectExtensions] = {}
+# Extra pandas post-processing operations to register alongside the built-in
ones.
+# Each entry is a callable with the signature:
+# def my_op(df: pandas.DataFrame, **options: Any) -> pandas.DataFrame
+# and will be available under its __name__ as an operation name.
+# Example:
+# from mypackage.ops import my_custom_op
+# EXTRA_PANDAS_POSTPROCESSING_OPS = [my_custom_op]
+EXTRA_PANDAS_POSTPROCESSING_OPS: list[Callable[..., Any]] = []
Review Comment:
**Suggestion:** The configuration type accepts any `Callable`, but the
registration and validation code later accesses each callable's `__name__`.
Callable instances and `functools.partial` objects are valid `Callable` values
without `__name__`, so configuring either will raise `AttributeError` while
processing a query instead of registering the operation. Restrict the
documented/configured contract to named functions or introduce an explicit
operation-name mapping. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Chart data validation fails for unnamed configured callables.
- ❌ Custom post-processing queries cannot return results.
- ⚠️ Any post-processing request may fail during allowed-name resolution.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 361:368
**Comment:**
*Api Mismatch: The configuration type accepts any `Callable`, but the
registration and validation code later accesses each callable's `__name__`.
Callable instances and `functools.partial` objects are valid `Callable` values
without `__name__`, so configuring either will raise `AttributeError` while
processing a query instead of registering the operation. Restrict the
documented/configured contract to named functions or introduce an explicit
operation-name mapping.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=4e7f142d5c74e3acbb8091b0c918c4c5a078d9a91d5075c1b3045c08499686a3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43337&comment_hash=4e7f142d5c74e3acbb8091b0c918c4c5a078d9a91d5075c1b3045c08499686a3&reaction=dislike'>👎</a>
--
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]
