aminghadersohi commented on PR #37214:
URL: https://github.com/apache/superset/pull/37214#issuecomment-3762160148
## Addressed Review Comments
Thank you for the review feedback! I've pushed fixes to address all the
issues:
### 1. Assert statement replaced with proper exception handling
**File:** `superset/mcp_service/chart/tool/generate_chart.py`
Changed from:
```python
assert validation_result.error is not None # Type narrowing for mypy
```
To:
```python
if validation_result.error is None:
raise RuntimeError("Validation failed but error object is missing")
```
### 2. Documentation inconsistency - Port reverted
**Files:** `superset/mcp_service/mcp_config.py`,
`superset/mcp_service/utils/url_utils.py`
Reverted the port from `9081` back to `9001` to maintain consistency with
the standard Superset default port. The MCP service port `5008` is correctly
separate for development access.
### 3. Test failures fixed
**File:**
`tests/unit_tests/mcp_service/chart/validation/test_runtime_validator.py`
Updated test assertions to match the new behavior where
`RuntimeValidator.validate_runtime_issues()` returns warnings metadata
`{"warnings": [...], "suggestions": [...]}` instead of `None` when semantic
warnings exist. This aligns with the PR's design that warnings are non-blocking.
All 7 tests now pass locally.
--
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]
