Repository navigation
Share the CLI output utilities between lakectl and lakefs - #10610
Open
shaktimaan00 wants to merge 1 commit into
Open
shaktimaan00 wants to merge 1 commit into
shaktimaan00 wants to merge 1 commit into
Conversation
lakectl renders output through a small template engine (WriteTo and the red/yellow/blue/table/paginate functions) but it lived in cmd/lakectl/cmd, so lakefs could not use it and kept its own printing helpers. Move the renderer and the Table, Pagination and ColoredText types to pkg/cmdutils, which both binaries already depend on. lakectl keeps thin aliases and wrappers so its call sites are unchanged, and lakefs renders its info, error and fatal messages through the same engine. Messages printed by lakefs are byte-for-byte identical to before.
This branch has not been deployed
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.
Closes #7904
Change Description
Background
lakectlrenders everything it prints through a small template engine:WriteTowith thered/yellow/blue/bold/table/paginatefunctions, plus theTable,PaginationandColoredTexttypes. That code lived incmd/lakectl/cmd, solakefscould not import it and kept its ownprintInfo/printNonFatalError/printMsgAndExithelpers that duplicated the same idea with directgo-prettycalls.New Feature
No new user-facing behaviour. The renderer now lives in
pkg/cmdutils/output.go, a package both binaries already use:pkg/cmdutils:WriteTo,Write,ColoredText/Colored,Table,Pagination, andDisableColors/Interactivefor the terminal-vs-pipe rendering decision. The code is moved as-is, solakectloutput is unchanged.cmd/lakectl/cmd: keeps itslakectl-specific templates andDie/Warning/PrintTablehelpers, with type aliases (Table,Pagination,ColoredText) and thinWrite/WriteTo/DisableColorswrappers so its ~60 call sites are untouched.cmd/lakefs/cmd:printInfo,printNonFatalErrorandprintMsgAndExitnow render throughcmdutils.WriteTowith the same templateslakectluses, instead of hand-rolledfmt.Fprintln/text.Sprintfcalls.Messages printed by
lakefsare byte-for-byte identical to before: I compared the old and new implementations for plain, multi-argument anderror-valued arguments.Testing Details
go build ./cmd/...passes.go test ./pkg/cmdutils/... ./cmd/lakectl/cmd/... ./cmd/lakefs/cmd/...passes.TestColorsmoved with the renderer topkg/cmdutils/output_test.go, and two tests were added for the non-interactive paths (tables rendered as tab-separated rows, pagination hint suppressed), which were previously untested.go vetis clean;gofmt -lis clean.go tool golangci-lintcould not run locally — the pinned linter ingo.modcannot read Go 1.27 export data — so CI will be the first lint run.lakefsoutput: oldfmt.Fprintln(os.Stderr, "Error:", ...)vs the new template, and oldtext.FgHiBlue.Sprintf/text.FgHiRed.Sprintfvs{{.Message|blue}}/{{.Message|red}}, produced identical bytes for allprintInfo/printNonFatalError/printMsgAndExitcall shapes.Breaking Change?
No. No API, CLI flag or output format changes. The only new exported surface is
pkg/cmdutils, which is internal to this repository.Additional info
The issue mentioned the basic helpers added in #7854 (
cmd/lakefs/cmd/common_helpers.go); those are now the three callers of the shared renderer and no longer importgo-prettydirectly.