Migrate to github.com/olekukonko/tablewriter 1.0 #2590

jsoriano · 2025-05-13T17:00:43Z

Tablewriter introduced several breaking changes in its 1.0 version, both in its APIs and in its behaviour.

Adapt the code to the new major, configuring renderer to behave as similar as possible as in v0.

Take the opportunity to refactor the related code to reuse table settings where possible.

Tablewriter introduced several breaking changes in its 1.0 version, both in its APIs and in its behaviour. Adapt the code to the new major.

jsoriano · 2025-05-13T17:02:05Z

cmd/testdata/status-local-version-stage

+-------------+----------------------------+-------------------+-------+-----------------+
+| Production  | 1.1.0-beta1 (1.0.0, 1.0.1, | Beta              | Foo   | Foo integration |
+|             | 1.0.2)                     |                   |       |                 |
+-------------+----------------------------+-------------------+-------+-----------------+


Wrapping seems to change in 1.0, I haven't managed to mimic old behavior, but the new looks better.

jsoriano · 2025-05-13T17:03:09Z

cmd/status.go

-	table.SetRowLine(true)
-	table.AppendBulk(environmentTable)
-	table.Render()
+func formatHeaders(headers []string) []string {


This was done automatically in v0, I haven't found how to do this in v1.

jsoriano · 2025-05-13T17:07:45Z

cmd/tables.go

+			},
+		},
+		Settings:  defaultTableRendererSettings,
+		Symbols:   tw.NewSymbols(tw.StyleASCII),


We could remove this line to change to the default style in v1, that would render tables like this (in my shell all lines are continuous):

$ elastic-package status -C ./packages/apache --info kibana.version Package: apache Owner: elastic/obs-infraobs-integrations Package Versions: ┌─────────────┬─────────┬─────────┬────────────────────┬───────────────────────────────┬───────────────────┐ │ ENVIRONMENT │ VERSION │ RELEASE │ TITLE │ DESCRIPTION │ KIBANA VERSION │ ├─────────────┼─────────┼─────────┼────────────────────┼───────────────────────────────┼───────────────────┤ │ Local │ 2.0.0 │ GA │ Apache HTTP Server │ Collect logs and metrics from │ ^8.13.0 || ^9.0.0 │ │ │ │ │ │ Apache servers with Elastic │ │ │ │ │ │ │ Agent. │ │ ├─────────────┼─────────┼─────────┼────────────────────┼───────────────────────────────┼───────────────────┤ │ Production │ 2.0.0 │ GA │ Apache HTTP Server │ Collect logs and metrics from │ ^8.13.0 || ^9.0.0 │ │ │ │ │ │ Apache servers with Elastic │ │ │ │ │ │ │ Agent. │ │ └─────────────┴─────────┴─────────┴────────────────────┴───────────────────────────────┴───────────────────┘

FTR, this is the output with ASCII style:

$ go run github.com/elastic/elastic-package status -C ./packages/apache --info kibana.version Package: apache Owner: elastic/obs-infraobs-integrations Package Versions: +-------------+---------+---------+--------------------+--------------------------------+-------------------+ | ENVIRONMENT | VERSION | RELEASE | TITLE | DESCRIPTION | KIBANA VERSION | +-------------+---------+---------+--------------------+--------------------------------+-------------------+ | Local | 2.0.0 | GA | Apache HTTP Server | Collect logs and metrics from | ^8.13.0 || ^9.0.0 | | | | | | Apache servers with Elastic | | | | | | | Agent. | | +-------------+---------+---------+--------------------+--------------------------------+-------------------+ | Production | 2.0.0 | GA | Apache HTTP Server | Collect logs and metrics from | ^8.13.0 || ^9.0.0 | | | | | | Apache servers with Elastic | | | | | | | Agent. | | +-------------+---------+---------+--------------------+--------------------------------+-------------------+

Just take a look locally to the new default style in V1, and it looks better.
I guess that change would break the tests in CI, have you tested that? I hope it should be easy to update.

It is easy to change 🙂 We have a -generate flag. Changed and tests updated.

mrodm · 2025-05-19T16:10:33Z

cmd/tables.go

+			},
+		},
+		Settings:  defaultTableRendererSettings,
+		Symbols:   tw.NewSymbols(tw.StyleASCII),


Just take a look locally to the new default style in V1, and it looks better.
I guess that change would break the tests in CI, have you tested that? I hope it should be easy to update.

mrodm · 2025-05-19T16:12:18Z

cmd/tables.go

+// headerCellFilter mimics behaviour of tablewriter v0, where some symbols where replaced
+// with spaces.


mrodm

👍

mrodm · 2025-05-20T08:25:20Z

cmd/status.go

+	colorCfg := defaultColorizedConfig()
+	table := tablewriter.NewTable(w,
+		tablewriter.WithRenderer(renderer.NewColorized(colorCfg)),
+		tablewriter.WithConfig(defaultTableConfig),


Just found that elastic-package stack status and other commands use another dependency to render data into tables: "github.com/jedib0t/go-pretty/table"

cmd/stack.go: "github.com/jedib0t/go-pretty/table" internal/benchrunner/runners/pipeline/format.go: "github.com/jedib0t/go-pretty/table" internal/benchrunner/runners/pipeline/format.go: "github.com/jedib0t/go-pretty/text" internal/benchrunner/runners/rally/report.go: "github.com/jedib0t/go-pretty/table" internal/benchrunner/runners/rally/report.go: "github.com/jedib0t/go-pretty/text" internal/benchrunner/runners/system/report.go: "github.com/jedib0t/go-pretty/table" internal/benchrunner/runners/system/report.go: "github.com/jedib0t/go-pretty/text" internal/testrunner/reporters/formats/human.go: "github.com/jedib0t/go-pretty/table"

Probably, it could be used eventually the same dependency everywhere (in follow-ups), WDYT ?

Good one, not sure which one is better.

By now I have done some changes for consistency in 80f5579:

stack status uses tablewriter now, so at least everything under cmd has consistent tables. This changes stack status to show colors, bold headers and lines between rows:

Default style modified to "Rounded", it looks nicer, and will be consistent with the other tables.

From this point, not sure what to do, do you have an opinion? go-pretty looks simpler, but also we are not using many formatting options, so at the end it may be similar in complexity.

Basically we have three options:

Migrate everything to go-pretty.

Migrate everything to tablewriter.

Leave things as they are now, using tablewriter for commands, and go-pretty for test reports.

If we go for using the same library, I can create an issue to delay the decision (maybe forever? 😅)

I think I prefer the previous, more compact format for stack status, I have modified it to:

I would still leave the lines as default because we have cases where it helps with readability.

From this point, not sure what to do, do you have an opinion? go-pretty looks simpler, but also we are not using many formatting options, so at the end it may be similar in complexity.

Basically we have three options:

Migrate everything to go-pretty.

Migrate everything to tablewriter.

Leave things as they are now, using tablewriter for commands, and go-pretty for test reports.

I guess, it would be good to just use one dependency for rendering tables. But I don't have an strong opinion in which one to choose.

For me, it could be kept as it is now in this PR. Trying to keep the usage of tablewriter in cmd and go-pretty for other usages.

If we go for using the same library, I can create an issue to delay the decision (maybe forever? 😅)

We could create an issue as a change that would be nice to have (using just one library).

I think I prefer the previous, more compact format for stack status, I have modified it to:

Comparing both versions, I also prefer the previous one. The one shown in the screenshot here: #2590 (comment)

Follow up issue created #2604.

jsoriano · 2025-05-21T16:06:33Z

cmd/stack.go

-
+	config := defaultColorizedConfig()
+	config.Settings.Separators.BetweenRows = tw.Off
+	table := tablewriter.NewTable(cmd.OutOrStderr(),


I am using cmd.OutOrStderr() here because it is what cmd.Println does. I found out by seeing that scripts/test-stack-command.sh uses 2> to redirect its output for comparison.

We should also work on consistency of outputs, I would say that all these tables should go to stdout.

Not to change in this PR in any case.

elasticmachine · 2025-05-21T16:18:50Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 36fc583

Failed CI Steps

:go: Integration test: system

History

💔 Build #5343 failed d61a1bc
💔 Build #5342 failed d942d65
💔 Build #5340 failed 80f5579
💔 Build #5336 failed 8e4a9ac

cc @jsoriano

jsoriano added 4 commits May 13, 2025 18:38

Migrate to github.com/olekukonko/tablewriter 1.0

8e559e8

Tablewriter introduced several breaking changes in its 1.0 version, both in its APIs and in its behaviour. Adapt the code to the new major.

Remove twcolor

34bf5d4

Use ASCII symbols

487d527

Set only required column colors

77f0bf1

jsoriano requested a review from a team May 13, 2025 17:00

jsoriano self-assigned this May 13, 2025

jsoriano commented May 13, 2025

View reviewed changes

This was referenced May 13, 2025

Chore(deps): bump github.com/olekukonko/tablewriter from 0.0.5 to 1.0.4 #2591

Closed

Chore(deps): bump github.com/olekukonko/tablewriter from 0.0.5 to 1.0.5 #2597

Closed

Update to 1.0.5

2fe0de2

jsoriano force-pushed the tablewriter-1.0 branch from 037b808 to 2fe0de2 Compare May 16, 2025 08:50

jsoriano added 2 commits May 16, 2025 10:58

Merge remote-tracking branch 'origin/main' into tablewriter-1.0

ed24ec3

Move header formatting to common code

5a03fbf

mrodm reviewed May 19, 2025

View reviewed changes

Use default symbols

6d5a4a8

jsoriano requested a review from mrodm May 19, 2025 17:51

tablewriter 1.0.6

8e4a9ac

jsoriano mentioned this pull request May 19, 2025

Chore(deps): bump github.com/olekukonko/tablewriter from 0.0.5 to 1.0.6 #2600

Closed

mrodm approved these changes May 20, 2025

View reviewed changes

jsoriano added 4 commits May 20, 2025 18:19

Use tablewriter in stack status too

80f5579

Go back to more compact tables in stack status

69d4e3f

Adjust expected status output

d942d65

Try again to fix test

d61a1bc

jsoriano mentioned this pull request May 21, 2025

Use single dependency to render tables #2604

Open

jsoriano added 2 commits May 21, 2025 17:33

Revert to write stack status to stderr

6758c64

Use same writer as cmd.Print

36fc583

jsoriano commented May 21, 2025

View reviewed changes

mrodm approved these changes May 22, 2025

View reviewed changes

jsoriano merged commit dd802f4 into elastic:main May 22, 2025
3 checks passed

jsoriano deleted the tablewriter-1.0 branch May 22, 2025 12:14

		// headerCellFilter mimics behaviour of tablewriter v0, where some symbols where replaced
		// with spaces.

Migrate to github.com/olekukonko/tablewriter 1.0 #2590

Migrate to github.com/olekukonko/tablewriter 1.0 #2590

Uh oh!

Conversation

jsoriano commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoriano May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrodm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

History

Uh oh!

Uh oh!

Uh oh!

jsoriano commented May 13, 2025 •

edited

Loading

jsoriano May 13, 2025 •

edited

Loading

jsoriano May 13, 2025 •

edited

Loading

elasticmachine commented May 21, 2025 •

edited

Loading