EnxDev commented on code in PR #45283:
URL: https://github.com/apache/superset/pull/45283#discussion_r4281280490
##########
superset/queries/api.py:
##########
@@ -275,12 +276,21 @@ def stop_query(self) -> FlaskResponse:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
+ 422:
+ description: >
+ Query could not be stopped; includes query if already complete
500:
$ref: '#/components/responses/500'
"""
try:
body = self.stop_query_schema.load(request.json)
- QueryDAO.stop_query(body["client_id"])
+ query = QueryDAO.stop_query(body["client_id"])
+ if query.status != QueryStatus.STOPPED:
Review Comment:
This also covers the early return in `stop_query` for a query that was
already complete, which used to answer 200 `OK`. `test_stop_query` in
`tests/integration_tests/queries/api_tests.py:553` inserts its query through
`insert_query`, which defaults to `status="success"`, and asserts 200. It
should go red once the integration suites run here.
That test needs a RUNNING query, plus a new case for the 422 body. Since
`POST /api/v1/query/stop` is public API and scripts that stop a query that
might already be done will start seeing a 422, I'd also add a line to
`UPDATING.md`.
--
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]
