bito-code-review[bot] commented on code in PR #41103:
URL: https://github.com/apache/superset/pull/41103#discussion_r3418662898
##########
superset/commands/report/execute.py:
##########
@@ -506,9 +509,88 @@ def _get_pdf(self) -> bytes:
return pdf
+ def _get_chart_data_request_payload(
+ self,
+ result_format: ChartDataResultFormat,
+ ) -> dict[str, Any]:
+ """
+ Build the same POST payload shape used by frontend exports.
+ """
+ try:
+ query_context =
json.loads(self._report_schedule.chart.query_context)
+ except (TypeError, json.JSONDecodeError) as ex:
+ raise ReportScheduleExecuteUnexpectedError(
+ "Chart has no valid query context saved."
+ ) from ex
+
+ if not isinstance(query_context, dict):
+ raise ReportScheduleExecuteUnexpectedError(
+ "Chart has no valid query context saved."
+ )
+
+ result_type = ChartDataResultType.POST_PROCESSED.value
+ force = bool(self._report_schedule.force_screenshot)
+ query_context["result_format"] = result_format.value
+ query_context["result_type"] = result_type
+ query_context["force"] = force
+
+ form_data = query_context.get("form_data")
+ if isinstance(form_data, dict):
+ form_data["result_format"] = result_format.value
+ form_data["result_type"] = result_type
+ form_data["force"] = force
+
+ if form_data.get("server_pagination"):
+ row_limit = form_data.get("row_limit") or 0
+ queries = query_context.get("queries")
+ if isinstance(queries, list):
+ data_query_updated = False
+ download_queries = []
+ for query in queries:
+ if isinstance(query, dict) and
query.get("is_rowcount"):
+ continue
+ if isinstance(query, dict) and not data_query_updated:
+ query = {
+ **query,
+ "row_limit": row_limit,
+ "row_offset": 0,
+ }
+ data_query_updated = True
+ download_queries.append(query)
+ query_context["queries"] = download_queries
+
+ return query_context
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests for new method</b></div>
<div id="fix">
The `_get_chart_data_request_payload()` method has no dedicated unit tests.
Per BITO.md rule 11730, new tools should have comprehensive unit tests covering
success paths, error scenarios, and edge cases.
</div>
</div>
<small><i>Code Review Run #dc034c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/commands/report/execute.py:
##########
@@ -506,9 +509,88 @@ def _get_pdf(self) -> bytes:
return pdf
+ def _get_chart_data_request_payload(
+ self,
+ result_format: ChartDataResultFormat,
+ ) -> dict[str, Any]:
+ """
+ Build the same POST payload shape used by frontend exports.
+ """
+ try:
+ query_context =
json.loads(self._report_schedule.chart.query_context)
+ except (TypeError, json.JSONDecodeError) as ex:
+ raise ReportScheduleExecuteUnexpectedError(
+ "Chart has no valid query context saved."
+ ) from ex
+
+ if not isinstance(query_context, dict):
+ raise ReportScheduleExecuteUnexpectedError(
+ "Chart has no valid query context saved."
+ )
+
+ result_type = ChartDataResultType.POST_PROCESSED.value
+ force = bool(self._report_schedule.force_screenshot)
+ query_context["result_format"] = result_format.value
+ query_context["result_type"] = result_type
+ query_context["force"] = force
+
+ form_data = query_context.get("form_data")
+ if isinstance(form_data, dict):
+ form_data["result_format"] = result_format.value
+ form_data["result_type"] = result_type
+ form_data["force"] = force
+
+ if form_data.get("server_pagination"):
+ row_limit = form_data.get("row_limit") or 0
+ queries = query_context.get("queries")
+ if isinstance(queries, list):
+ data_query_updated = False
+ download_queries = []
+ for query in queries:
+ if isinstance(query, dict) and
query.get("is_rowcount"):
+ continue
+ if isinstance(query, dict) and not data_query_updated:
+ query = {
+ **query,
+ "row_limit": row_limit,
+ "row_offset": 0,
+ }
+ data_query_updated = True
+ download_queries.append(query)
+ query_context["queries"] = download_queries
+
+ return query_context
+
+ @staticmethod
+ def _post_chart_data(
+ chart_url: str,
+ auth_cookies: Optional[dict[str, str]],
+ request_payload: dict[str, Any],
+ ) -> Optional[bytes]:
+ if not auth_cookies:
+ return None
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Silent failure on missing auth</b></div>
<div id="fix">
When `auth_cookies` is falsy, `_post_chart_data()` returns `None`, which
causes `_get_csv_data()` to raise `ReportScheduleCsvFailedError()` with no
message. This silent failure path makes debugging difficult.
</div>
</div>
<small><i>Code Review Run #dc034c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
