codeant-ai-for-open-source[bot] commented on code in PR #40935:
URL: https://github.com/apache/superset/pull/40935#discussion_r3433547174
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -408,74 +410,24 @@ def _parse_column_fields(cls, value: Any) -> Any:
return parsed
-class CreateDatasetRequest(BaseModel):
- """Request schema for create_dataset to register a physical table as a
dataset."""
-
- model_config = ConfigDict(populate_by_name=True)
-
- database_id: Annotated[
- int,
- Field(
- description="ID of the database connection to register the table
against"
- ),
- ]
- schema_: Annotated[
- str | None,
- Field(
- default=None,
- alias="schema",
- serialization_alias="schema",
- max_length=250,
- description="Schema (namespace) where the table lives, e.g.
'public'. "
- "Omit or pass None for databases without schema namespaces (e.g.
SQLite).",
- ),
- ]
- catalog: Annotated[
- str | None,
- Field(
- default=None,
- max_length=250,
- description="Catalog where the table lives. Omit for databases
without "
- "catalog support.",
- ),
- ]
- table_name: Annotated[
- str,
- Field(
- min_length=1,
- max_length=250,
- description="Name of the physical table to register as a dataset",
- ),
- ]
- owners: Annotated[
- List[int] | None,
- Field(
- default=None,
- description="Optional list of owner user IDs. "
- "Defaults to the calling user.",
- ),
- ]
-
- @field_validator("schema_", "catalog", mode="before")
- @classmethod
- def _normalize_optional_str(cls, v: object) -> object:
- """Strip whitespace and convert blank strings to None.
+class CreateDatasetMetric(BaseModel):
+ metric_name: str = Field(..., description="Name of the metric")
+ expression: str = Field(..., description="SQL expression for the metric")
+ verbose_name: str | None = None
+ description: str | None = None
+ metric_type: str | None = None
+ d3format: str | None = None
+ warning_text: str | None = None
- Non-string values pass through unchanged so Pydantic's type validation
- rejects them, rather than silently treating a malformed value (e.g. an
- int or dict) as an omitted namespace.
- """
- if isinstance(v, str):
- return v.strip() or None
- return v
- @field_validator("table_name", mode="before")
- @classmethod
- def _strip_table_name(cls, v: object) -> object:
- """Strip leading/trailing whitespace from table_name."""
- if isinstance(v, str):
- return v.strip()
- return v
+class CreateDatasetCalculatedColumn(BaseModel):
Review Comment:
**Suggestion:** Add an inline class docstring that explains the role of this
model and the semantics of the calculated column definition it captures.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The final file state shows this newly added class also has no docstring.
Since the rule says new Python classes should be documented inline, this is
a verified violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a13a5e33a98d458bb90372f762d48d27&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a13a5e33a98d458bb90372f762d48d27&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/schemas.py
**Line:** 423:423
**Comment:**
*Custom Rule: Add an inline class docstring that explains the role of
this model and the semantics of the calculated column definition it captures.
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%2F40935&comment_hash=36fe3fa68d99451f6b8690c0d249bd0c37b0205851176373814395ef47e8605b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=36fe3fa68d99451f6b8690c0d249bd0c37b0205851176373814395ef47e8605b&reaction=dislike'>👎</a>
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -408,74 +410,24 @@ def _parse_column_fields(cls, value: Any) -> Any:
return parsed
-class CreateDatasetRequest(BaseModel):
- """Request schema for create_dataset to register a physical table as a
dataset."""
-
- model_config = ConfigDict(populate_by_name=True)
-
- database_id: Annotated[
- int,
- Field(
- description="ID of the database connection to register the table
against"
- ),
- ]
- schema_: Annotated[
- str | None,
- Field(
- default=None,
- alias="schema",
- serialization_alias="schema",
- max_length=250,
- description="Schema (namespace) where the table lives, e.g.
'public'. "
- "Omit or pass None for databases without schema namespaces (e.g.
SQLite).",
- ),
- ]
- catalog: Annotated[
- str | None,
- Field(
- default=None,
- max_length=250,
- description="Catalog where the table lives. Omit for databases
without "
- "catalog support.",
- ),
- ]
- table_name: Annotated[
- str,
- Field(
- min_length=1,
- max_length=250,
- description="Name of the physical table to register as a dataset",
- ),
- ]
- owners: Annotated[
- List[int] | None,
- Field(
- default=None,
- description="Optional list of owner user IDs. "
- "Defaults to the calling user.",
- ),
- ]
-
- @field_validator("schema_", "catalog", mode="before")
- @classmethod
- def _normalize_optional_str(cls, v: object) -> object:
- """Strip whitespace and convert blank strings to None.
+class CreateDatasetMetric(BaseModel):
Review Comment:
**Suggestion:** Add an inline class docstring describing the purpose of this
request payload model and what kind of metric object it represents.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The final file state shows this newly added class has no docstring
immediately
under the class definition. The custom rule requires newly added Python
classes to include docstrings, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e2581af2f62543b9aabb7f3900ecc8ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e2581af2f62543b9aabb7f3900ecc8ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/dataset/schemas.py
**Line:** 413:413
**Comment:**
*Custom Rule: Add an inline class docstring describing the purpose of
this request payload model and what kind of metric object it represents.
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%2F40935&comment_hash=d20f389485bb7d40c5c15237b02771b73db043899f4055d348215dfd82ddae4b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40935&comment_hash=d20f389485bb7d40c5c15237b02771b73db043899f4055d348215dfd82ddae4b&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]
