Skip to content

Add cancel button to preview UI#29315

Open
koszti wants to merge 1 commit into
trinodb:masterfrom
koszti:preview-ui-control-buttons
Open

Add cancel button to preview UI#29315
koszti wants to merge 1 commit into
trinodb:masterfrom
koszti:preview-ui-control-buttons

Conversation

@koszti
Copy link
Copy Markdown
Member

@koszti koszti commented May 4, 2026

Description

Add Preempt and Kill control buttons to the preview query overview header.

Screenshots:

image

..

image

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Add kill and preempt buttons to preview UII. ({issue}`29315`)

@cla-bot cla-bot Bot added the cla-signed label May 4, 2026
@github-actions github-actions Bot added the ui Web UI label May 4, 2026
@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 4, 2026

@sajjoseph can you also test this ?

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall pretty much good to go .. just some minor things to figure out.

Comment thread core/trino-web-ui/src/main/resources/webapp-preview/src/api/webapp/api.ts Outdated
Comment thread core/trino-web-ui/src/main/resources/webapp-preview/src/api/webapp/api.ts Outdated
@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 5, 2026

Also one more thought .. in case I understand this correctly..

Preempt only applies to queries that are not yet running
Kill only applies to queries that are already running

So could we not just have one button that says "Stop processing" or something .. and it does the right thing depending on the the query?

@RoeyoOgen
Copy link
Copy Markdown

Also one more thought .. in case I understand this correctly..

Preempt only applies to queries that are not yet running

Kill only applies to queries that are already running

So could we not just have one button that says "Stop processing" or something .. and it does the right thing depending on the the query?

Agree, makes sense

@koszti koszti force-pushed the preview-ui-control-buttons branch from ccbe9fb to ae8ce88 Compare May 5, 2026 08:51
@koszti koszti changed the title Add kill and preempt buttons to preview UI Add cancel button to preview UI May 5, 2026
@koszti koszti force-pushed the preview-ui-control-buttons branch from ae8ce88 to 83a38aa Compare May 5, 2026 09:00
@koszti
Copy link
Copy Markdown
Member Author

koszti commented May 5, 2026

@mosabua @RoeyoOgen , I looked through the backend java code and I think the only difference between kill and preempt is the message. Both paths call the same failQuery method, just with different exception messages: UiQueryResource.java#131

Given that I combined the two actions into a single Cancel query button, using the message Canceled via web UI.

let me know your thoughts.

image

...

image

@koszti koszti force-pushed the preview-ui-control-buttons branch 2 times, most recently from 85ed64c to c99461f Compare May 5, 2026 09:29
@sajjoseph
Copy link
Copy Markdown
Contributor

I like the cancel query approach to combine the actions.

@mosabua mosabua force-pushed the preview-ui-control-buttons branch from c99461f to 0bd9625 Compare May 9, 2026 19:51
@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 9, 2026

Just fyi @koszti .. I rebased and pushed to test and potentially merge

@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 10, 2026

@koszti I am getting build failures locally .. can you check?

[INFO] src/components/QueryOverview.tsx(519,37): error TS2769: No overload matches this call.
[INFO]   Overload 1 of 3, '(props: { href: string; } & ButtonOwnProps & Omit<ButtonBaseOwnProps, "classes"> & CommonProps & Omit<DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>, "children" | ... 27 more ... | "startIcon">): Element', gave the following error.
[INFO]     Type 'string | true | null' is not assignable to type 'boolean | undefined'.
[INFO]       Type 'null' is not assignable to type 'boolean | undefined'.
[INFO]   Overload 2 of 3, '(props: { component: ElementType<any, keyof IntrinsicElements>; } & ButtonOwnProps & Omit<ButtonBaseOwnProps, "classes"> & CommonProps & Omit<...>): Element | null', gave the following error.
[INFO]     Type 'string | true | null' is not assignable to type 'boolean | undefined'.
[INFO]       Type 'null' is not assignable to type 'boolean | undefined'.
[INFO]   Overload 3 of 3, '(props: DefaultComponentProps<ExtendButtonBaseTypeMap<ButtonTypeMap<{}, "button">>>): Element | null', gave the following error.
[INFO]     Type 'string | true | null' is not assignable to type 'boolean | undefined'.
[INFO]       Type 'null' is not assignable to type 'boolean | undefined'.
[INFO]

@koszti koszti force-pushed the preview-ui-control-buttons branch from 0bd9625 to 36cdaa5 Compare May 11, 2026 07:45
@koszti
Copy link
Copy Markdown
Member Author

koszti commented May 11, 2026

@mosabua I fixed the tests, everything is green now

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. Thank you @koszti

@mosabua mosabua added this pull request to the merge queue May 11, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 11, 2026
@koszti
Copy link
Copy Markdown
Member Author

koszti commented May 11, 2026

@mosabua do you know why this PR removed from the merge queue? It says build-success is failed (https://github.com/trinodb/trino/actions/runs/25678587154/job/75388409099), but all CI checks passed. Am I missing something?

@mosabua mosabua added this pull request to the merge queue May 11, 2026
@mosabua
Copy link
Copy Markdown
Member

mosabua commented May 11, 2026

The queue is weird .. if it fails even an unrelated test it gets bumped off it seems .. I put it back on the queue

@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 11, 2026
@mosabua mosabua added this pull request to the merge queue May 11, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 11, 2026
@mosabua mosabua added this pull request to the merge queue May 11, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 11, 2026
@mosabua mosabua added this pull request to the merge queue May 12, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 12, 2026
@mosabua mosabua added this pull request to the merge queue May 12, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 12, 2026
@koszti koszti force-pushed the preview-ui-control-buttons branch from 36cdaa5 to f6c8fc8 Compare May 12, 2026 20:20
@mosabua mosabua enabled auto-merge May 12, 2026 20:32
@mosabua mosabua disabled auto-merge May 12, 2026 23:48
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented May 13, 2026

## Section
* Add kill and preempt buttons to preview UII. ({issue}`29315`)

The section should be Web UI and there is a typo > UII.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

7 participants