codeant-ai-for-open-source[bot] commented on code in PR #41159:
URL: https://github.com/apache/superset/pull/41159#discussion_r3429782570
##########
superset/views/core.py:
##########
@@ -237,7 +237,7 @@ def _generate_xlsx(viz_obj: BaseViz) -> FlaskResponse:
@permission_name("explore_json")
@expose("/explore_json/data/<cache_key>", methods=("GET",))
@check_resource_permissions(check_explore_cache_perms)
- @deprecated(eol_version="5.0.0")
+ @deprecated(eol_version="5.0.0", new_target="/api/v1/chart/data")
Review Comment:
**Suggestion:** The deprecation target for the cached-result endpoint points
to `/api/v1/chart/data`, but this endpoint is actually replaced by the
cache-key route `/api/v1/chart/data/<cache_key>`. As written, operators
following the warning for `/explore_json/data/<cache_key>` will hit the wrong
API shape and lose the cache-key mapping needed to fetch async results. Update
the deprecation target to the cache-key variant (using a placeholder) so
migration guidance is correct. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Logs misdirect async cache users to wrong REST endpoint.
- ⚠️ Migrating clients may drop cache_key-based async retrieval.
- ⚠️ Async chart flows harder to port to v1 APIs.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The cached async explore endpoint is defined at
`superset/views/core.py:18-22` (in
snippet offset) as `@expose("/explore_json/data/<cache_key>",
methods=("GET",))` followed
by `def explore_json_data(self, cache_key: str)`, and is decorated with
`@deprecated(eol_version="5.0.0", new_target="/api/v1/chart/data")` at line
240 in the PR
hunk.
2. The `deprecated` decorator implementation in
`superset/views/base.py:5-28` builds a
warning message and, when `new_target` is provided, appends `". Use the
following API
endpoint instead: %s"` and logs the supplied `new_target` value; thus any
call to
`explore_json_data` logs `/api/v1/chart/data` as the recommended replacement
URL.
3. The new v1 chart data APIs are implemented in
`superset/charts/data/api.py:26-37` and
`360-107`: `data()` is exposed as `@expose("/data", methods=("POST",))`
(POST with a JSON
query context body), while `data_from_cache()` is exposed as
`@expose("/data/<cache_key>",
methods=("GET",))` and documented to "take a query context cache key and
return payload
data response for the given query".
4. `ChartDataRestApi` inherits from `ChartRestApi`
(`superset/charts/api.py:110-114`,
`resource_name = "chart"`) and is registered via
`appbuilder.add_api(ChartDataRestApi)` in
`superset/initialization/__init__.py:95-105`, which maps these routes under
`/api/v1/chart`. Therefore, the functional replacement for
`/superset/explore_json/data/<cache_key>` is `GET
/api/v1/chart/data/<cache_key>`
(ChartDataRestApi.data_from_cache), not `POST /api/v1/chart/data`
(ChartDataRestApi.data).
The current deprecation warning on `explore_json_data` incorrectly points
operators to
`/api/v1/chart/data`, which has a different HTTP method and request shape
and does not use
the async cache key, so migration guidance for async cached results is wrong.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2e6f00c69f1744bebffe55fac2d5cbbb&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=2e6f00c69f1744bebffe55fac2d5cbbb&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/views/core.py
**Line:** 240:240
**Comment:**
*Api Mismatch: The deprecation target for the cached-result endpoint
points to `/api/v1/chart/data`, but this endpoint is actually replaced by the
cache-key route `/api/v1/chart/data/<cache_key>`. As written, operators
following the warning for `/explore_json/data/<cache_key>` will hit the wrong
API shape and lose the cache-key mapping needed to fetch async results. Update
the deprecation target to the cache-key variant (using a placeholder) so
migration guidance is correct.
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%2F41159&comment_hash=39ce4ad3e1265f90097ff060db5008f058d75f01418004d4c87f753a914bd04c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41159&comment_hash=39ce4ad3e1265f90097ff060db5008f058d75f01418004d4c87f753a914bd04c&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]
