Skip to content

fix(trino): Bugfixes & QoL changes - #311

Open
labrenbe wants to merge 8 commits into
mainfrom
fix/various-fixes-qol-changes
Open

fix(trino): Bugfixes & QoL changes#311
labrenbe wants to merge 8 commits into
mainfrom
fix/various-fixes-qol-changes

Conversation

@labrenbe

@labrenbe labrenbe commented Sep 7, 2026

Copy link
Copy Markdown
Member

User-feedback fixes and QoL improvements for the Trino module.

Fixed

  • Catalog browser now scrolls long schema lists (was clipped after ~30 entries).
  • Queries stopped at the 10,000-row limit now actually cancel in Trino — via the client-protocol DELETE nextUri instead of the kill-query-gated DELETE /v1/query/{id}.
  • Cancellation and polling now send X-Trino-User when impersonation is enabled (previously ran as the service principal).
  • Saving a Trino connection now verifies it (SELECT 1, 10 s timeout) before storing, instead of always reporting success.
  • Transport-level query failures now show the reason instead of a bare "Failed" badge.
  • Fixed a silent unhandled rejection when the catalog default-schema dropdown failed to load.
  • Long catalog names now truncate instead of overflowing horizontally.

Added

  • trino.publicUrl: separate browser-facing URL for the "View in Trino" deep link (falls back to trino.url); wired through the Helm chart.
  • Resizable catalog browser with a persisted, keyboard-adjustable width.

Tests

  • Unit: cancelViaUri, impersonation headers, AbortSignal forwarding, resolveTrinoPublicUrl, row-limit cancel.
  • E2E: catalog scroll + resize, transport-error surfacing.

@labrenbe
labrenbe requested a review from dervoeti September 7, 2026 13:59
@labrenbe labrenbe self-assigned this Sep 7, 2026
Comment thread src/routes/(app)/trino/+page.server.ts Outdated
const reason =
name === 'TimeoutError' || name === 'AbortError'
? 'Connection test timed out'
: `Could not connect to Trino: ${detail}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should not return detail here. The user could try to connect to whatever cluster-internal service here and probably see the raw response from it this way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in db825ef.

options: { user: string; catalog?: string; schema?: string }
options: { user: string; catalog?: string; schema?: string; signal?: AbortSignal }
): Promise<TrinoQueryResult> {
const headers: Record<string, string> = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably also use headersForUser here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 7e8bcf4.

@dervoeti dervoeti moved this to Development: In Review in Stackable Engineering Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Development: In Review

Development

Successfully merging this pull request may close these issues.

2 participants