Copilot commented on code in PR #40630:
URL: https://github.com/apache/superset/pull/40630#discussion_r3337831329
##########
superset/sqllab/api.py:
##########
@@ -493,8 +496,62 @@ def get_results(self, **kwargs: Any) -> FlaskResponse:
$ref: '#/components/responses/500'
"""
params = kwargs["rison"]
- key = params.get("key")
- rows = params.get("rows")
+ return self._get_results_response(
+ key=params.get("key"), rows=params.get("rows")
+ )
+
+ @expose("/results/", methods=("POST",))
+ @protect()
+ @permission_name("get_results")
+ @statsd_metrics
+ @requires_json
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs:
f"{self.__class__.__name__}.get_results",
+ log_to_statsd=False,
Review Comment:
`post_results` is logged as `...get_results` in the event logger action,
which makes audit/event logs ambiguous and misleading (GET vs POST are
indistinguishable). This should use the actual handler name (or a distinct
action) so consumers can tell which route/method was invoked.
##########
superset/sqllab/schemas.py:
##########
@@ -27,6 +27,18 @@
}
+class SqlLabResultsSchema(Schema):
+ key = fields.String(
+ required=True,
+ metadata={"description": "The results key of the cached query
results"},
+ )
+ rows = fields.Integer(
+ required=False,
+ allow_none=True,
+ metadata={"description": "The maximum number of rows to return"},
+ )
Review Comment:
`rows` is described as a maximum row limit, but the schema currently allows
negative (and 0) values. Negative values lead to surprising slicing semantics
in `apply_display_max_row_configuration_if_require` (e.g. `data[: -1]`) and 0
is silently ignored because `if self._rows:` is falsy. Add a validation
constraint so only positive integers are accepted.
--
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]
