Add curated TUI table overrides for list commands#4731
Open
simonfaltum wants to merge 5 commits intosimonfaltum/list-tui-paginatedfrom
Open
Add curated TUI table overrides for list commands#4731simonfaltum wants to merge 5 commits intosimonfaltum/list-tui-paginatedfrom
simonfaltum wants to merge 5 commits intosimonfaltum/list-tui-paginatedfrom
Conversation
53b85d2 to
64f2b98
Compare
af5ef00 to
19c9b53
Compare
Collaborator
|
Commit: e76dee4
16 interesting tests: 7 SKIP, 6 RECOVERED, 3 flaky
Top 20 slowest tests (at least 2 minutes):
|
64f2b98 to
ef8c413
Compare
19c9b53 to
2495267
Compare
1bf0772 to
e672906
Compare
Co-authored-by: Isaac
2495267 to
78f3489
Compare
Contributor
shreyas-goenka
left a comment
There was a problem hiding this comment.
Note: This review was posted by Claude (AI assistant). Shreyas will do a separate, more thorough review pass.
Priority: HIGH — Potential data correctness bug in pipelines search
MAJOR: Pipelines search overwrites user's --filter flag
In cmd/workspace/pipelines/overrides.go, the override sets listReq.Filter = "name LIKE '%" + listReq.Filter + "%'" which overwrites whatever the user passed via --filter. If the user intended to pass a raw DLT filter expression (e.g., "state = 'RUNNING'"), it gets wrapped in a name LIKE expression, silently changing the query semantics. Consider preserving the original filter if it doesn't look like a plain name search, or using a separate flag for the name-search behavior.
Other Observations
- The pattern across the other 14 files is clean and consistent
- Good column selection choices for each resource type
RegisterConfigcalls correctly match the command paths
The pipelines search filter issue is the main thing to look at closely.
Treat pipeline search input literally when users type SQL LIKE wildcards, and add package-level override tests so nested SDK field access is exercised before runtime.
Previously, the TUI search callback overwrote req.Filter entirely with a name LIKE expression, discarding any filter the user passed via --filter. Now the name LIKE clause is combined with the existing filter using AND, so both constraints apply together.
Wrap the user-provided filter in parentheses before appending the AND clause. Without this, a filter like 'a OR b' combined with a name search would parse as 'a OR (b AND name LIKE ...)' instead of the intended '(a OR b) AND name LIKE ...'. Add a test case with an OR filter to verify correct parenthesization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR #4729 adds an interactive table for list commands, but its default behavior is to fall through to template rendering when no table configuration is registered. For the commands people use most, the table should start with the fields they actually care about, and it should enable search where the backend supports it.
These 15 commands were selected based on CLI usage as some of the most frequently used list commands by customer count.
Changes
Before: interactive list commands used template rendering with no curated table configuration.
Now: 15 common list commands register curated columns, and jobs list plus pipelines list-pipelines also enable / search in the TUI.
This PR builds on #4729. It does not change the table engine itself. It adds per-command overrides that:
Commands covered: jobs list, clusters list, pipelines list-pipelines, warehouses list, catalogs list, schemas list, tables list, apps list, repos list, instance-pools list, serving-endpoints list, volumes list, external-locations list, alerts list, workspace list
Test plan
go build ./...make checkspassesmake lintfullpasses (0 issues)