-
Notifications
You must be signed in to change notification settings - Fork 82
feat(webui): Add support for cancelling Presto SQL queries. #1175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (8)
WalkthroughAdds Presto query cancellation: server POST /cancel route, client API call, request handlers integrating search UI state, a Cancel button and composite PrestoSearchButton component, and related CSS and minor RunButton updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PrestoSearchButton
participant RunButton
participant CancelButton
participant SearchStore
participant ClientAPI
participant Server
User->>PrestoSearchButton: Click
PrestoSearchButton->>SearchStore: read searchUiState
alt state == QUERYING
PrestoSearchButton->>CancelButton: render
User->>CancelButton: Click Cancel
CancelButton->>SearchStore: read searchJobId
alt jobId present
CancelButton->>SearchStore: set searchUiState = DONE
CancelButton->>ClientAPI: cancelQuery(jobId)
ClientAPI->>Server: POST /api/presto-search/cancel
Server-->>ClientAPI: 204 No Content
ClientAPI-->>CancelButton: success
else jobId missing
CancelButton-->>User: log error
end
else
PrestoSearchButton->>RunButton: render
User->>RunButton: Click Run
RunButton->>SearchStore: read searchUiState
alt state in [DEFAULT, DONE]
RunButton->>ClientAPI: submitQuery(payload)
ClientAPI->>Server: POST /api/presto-search/submit
Server-->>ClientAPI: 200 (jobId)
ClientAPI-->>RunButton: jobId
RunButton->>SearchStore: set searchJobId, set searchUiState = QUERYING
else
RunButton-->>User: disabled / no-op
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Add the ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - have some style changes
components/webui/client/src/pages/SearchPage/SearchControls/Presto/RunButton/index.tsx
Outdated
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts
Outdated
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts
Outdated
Show resolved
Hide resolved
components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments
components/webui/client/src/pages/SearchPage/SearchControls/index.tsx
Outdated
Show resolved
Hide resolved
.../webui/client/src/pages/SearchPage/SearchControls/Presto/PrestoSearchButton/index.module.css
Show resolved
Hide resolved
title looks good |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring to @davemarco's review.
Description
This PR adds support for cancelling Presto SQL queries.
Checklist
breaking change.
Validation performed
SELECT * FROM tpch.sf100000.orders
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes