rpk Add Format Flags to topic's describe #23418

gene-redpanda · 2024-09-21T02:27:28Z

Adds --format to rpk topic describe.

Backports Required

Release Notes

Improvements

Adds --format support to rpk topic describe

src/go/rpk/pkg/cli/topic/list.go

src/go/rpk/pkg/cli/topic/describe.go

dotnwat

The first commit in this PR needs to have bazel run //:gazelle run and the changes committed.
Please wrap your commit messages at around 80 characters

List is working as intended. Everything is driven by the data and it flows through the various formats with formatting separate from the assembly of the data.

src/go/rpk/pkg/cli/topic/list.go

vbotbuildovich · 2024-10-29T21:45:14Z

Retry command for Build#57318

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/transactions/tx_atomic_produce_consume_test.py::TxAtomicProduceConsumeTest.test_basic_tx_consumer_transform_produce@{"with_failures":true}

vbotbuildovich · 2024-10-30T00:36:49Z

Retry command for Build#57327

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/transactions/tx_atomic_produce_consume_test.py::TxAtomicProduceConsumeTest.test_basic_tx_consumer_transform_produce@{"with_failures":true}

vbotbuildovich · 2024-10-30T05:36:09Z

Retry command for Build#57344

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/transactions/tx_atomic_produce_consume_test.py::TxAtomicProduceConsumeTest.test_basic_tx_consumer_transform_produce@{"with_failures":true}
tests/rptest/tests/idempotency_test.py::IdempotencyWriteCachingTest.test_idempotent_producers_write_caching

vbotbuildovich · 2024-11-14T21:59:38Z

Retry command for Build#58077

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/transactions/tx_atomic_produce_consume_test.py::TxAtomicProduceConsumeTest.test_basic_tx_consumer_transform_produce@{"with_failures":true}
tests/rptest/tests/raft_recovery_test.py::RaftRecoveryUpgradeTest.test_upgrade

Adds format flag to describe Supports JSON and YAML

vbotbuildovich · 2024-11-21T21:20:27Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/58482#01935040-d0f5-4a3b-ab3d-37f9e40d8ad5
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/58482#0193505c-56eb-4e3d-95ec-b6d05303e3f8

vbotbuildovich · 2024-11-21T21:20:49Z

Retry command for Build#58482

please wait until all jobs are finished before running the slash command

/ci-repeat 1
tests/rptest/transactions/tx_atomic_produce_consume_test.py::TxAtomicProduceConsumeTest.test_basic_tx_consumer_transform_produce@{"with_failures":true}
tests/rptest/tests/raft_recovery_test.py::RaftRecoveryUpgradeTest.test_upgrade
tests/rptest/tests/cluster_features_test.py::FeaturesNodeJoinTest.test_old_node_join

vbotbuildovich · 2024-11-21T21:50:31Z

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/58482#0193505c-56e9-453d-833a-7e13dce0f6e3:

"rptest.tests.raft_recovery_test.RaftRecoveryUpgradeTest.test_upgrade"

non flaky failures in https://buildkite.com/redpanda/redpanda/builds/58482#0193505c-56eb-4e3d-95ec-b6d05303e3f8:

"rptest.tests.cluster_features_test.FeaturesNodeJoinTest.test_old_node_join"

r-vasquez

Leaving this here for reference, this is being continued on #24387

r-vasquez · 2024-11-29T21:26:43Z

src/go/rpk/pkg/cli/topic/describe.go

-			// By default, if neither are specified, we opt in to
-			// the config section only.


Why remove this?

r-vasquez · 2024-11-29T21:27:06Z

src/go/rpk/pkg/cli/topic/describe.go

-			} else if len(topicArg) == 0 {
-				out.Exit("did not match any topics, exiting.")


This is needed for regex, is there a reason to remove it?

r-vasquez · 2024-11-29T21:28:32Z

src/go/rpk/pkg/cli/topic/describe.go

+			if topic.Summary.Partitions > 0 {
+				tw.PrintColumn("REPLICAS", topic.Summary.Replicas)
+			}
+			if topic.Summary.isErr {


We can avoid the additional field isErr by just comparing topic.Sumary.Error != ""

r-vasquez · 2024-11-29T21:32:58Z

src/go/rpk/pkg/cli/topic/describe.go

+	reqResource.ResourceName = *topic.Topic
+	req.Resources = append(req.Resources, reqResource)
+
+	resp, err := req.RequestWith(context.Background(), cl)


Suggested change

resp, err := req.RequestWith(context.Background(), cl)

resp, err := req.RequestWith(cmd.Context(), cl)

r-vasquez · 2024-11-29T21:34:09Z

src/go/rpk/pkg/cli/topic/describe.go

+	out.MaybeDie(err, "unable to request configs: %v", err)
+	if len(resp.Resources) != 1 {
+		out.Die("config response returned %d resources when we asked for 1", len(resp.Resources))


For helper functions, it is better to return an error and avoid using out.MaybeDie and out.Die outside of the Run function.

r-vasquez · 2024-11-29T21:40:42Z

src/go/rpk/pkg/cli/topic/describe.go

-				if i < len(resp.Topics) {
-					fmt.Println()
+			var topicDescriptions []describedTopic
+			for _, topic := range resp.Topics {


We should modify this so:

If is text: only execute and print what's needed.

If it's JSON, execute everything.

Right now it executes everything no matter which format it is, and then later figure out what to print. For Config, for example, we could be doing extra request without needing them:

# For example, this will trigger a GetConfig request but won't print anything. $ rpk topic describe foo -p

r-vasquez · 2024-11-29T21:44:21Z

src/go/rpk/pkg/cli/topic/describe.go

+
+func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) (output []describeTopicConfig) {
+	output = make([]describeTopicConfig, 0, len(configs))
+	for _, cfg := range configs {


We are missing sorting the configs from the old code

r-vasquez · 2024-11-29T21:45:37Z

src/go/rpk/pkg/cli/topic/describe.go

+	return resp.Resources[0].Configs, err
+}
+
+func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) (output []describeTopicConfig) {


Suggested change

func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) (output []describeTopicConfig) {

func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) ([]describeTopicConfig) {

Since you still overwrite the zero value one line below.

r-vasquez · 2024-11-29T23:52:36Z

src/go/rpk/pkg/cli/topic/describe.go

+			if len(topicDescriptions) == 0 {
+				out.Exit("[]")
+			}


This shouldn't be an exit [], we should pass an empty slice and down the line print it as expected per the requested format. [] is not a good response for text format.

r-vasquez · 2024-11-29T23:53:46Z

src/go/rpk/pkg/cli/topic/describe.go

 			}
+			printDescribedTopics(summary, configs, partitions, topicDescriptions)
+			out.MaybeDie(err, "unable to request topic metadata: %v", err)


This line doesn't belong here.

r-vasquez · 2024-12-03T19:59:52Z

Continuing in #24387

github-actions bot added the area/rpk label Sep 21, 2024

gene-redpanda force-pushed the format-flags-topic-list-detail branch 3 times, most recently from 323d353 to 5dd05ef Compare October 7, 2024 22:48

gene-redpanda marked this pull request as ready for review October 7, 2024 22:56

gene-redpanda requested review from twmb, r-vasquez and Deflaimun as code owners October 7, 2024 22:56

gene-redpanda force-pushed the format-flags-topic-list-detail branch 2 times, most recently from 6e3bcd4 to d87103f Compare October 7, 2024 22:57

r-vasquez reviewed Oct 8, 2024

View reviewed changes

src/go/rpk/pkg/cli/topic/list.go Outdated Show resolved Hide resolved

src/go/rpk/pkg/cli/topic/list.go Outdated Show resolved Hide resolved

src/go/rpk/pkg/cli/topic/list.go Outdated Show resolved Hide resolved

src/go/rpk/pkg/cli/topic/describe.go Outdated Show resolved Hide resolved

gene-redpanda force-pushed the format-flags-topic-list-detail branch 4 times, most recently from 76c3ba2 to ac6b4f4 Compare October 11, 2024 14:51

r-vasquez self-requested a review October 11, 2024 14:58

dotnwat reviewed Oct 14, 2024

View reviewed changes

r-vasquez reviewed Oct 16, 2024

View reviewed changes

src/go/rpk/pkg/cli/topic/list.go Outdated Show resolved Hide resolved

src/go/rpk/pkg/cli/topic/list.go Outdated Show resolved Hide resolved

gene-redpanda force-pushed the format-flags-topic-list-detail branch from ac6b4f4 to 3c29b27 Compare October 17, 2024 19:17

github-actions bot added the area/build label Oct 17, 2024

gene-redpanda changed the title ~~Add Format Flags to topic's describe and list~~ Add Format Flags to topic's describe Oct 17, 2024

gene-redpanda force-pushed the format-flags-topic-list-detail branch 2 times, most recently from 4929174 to e60edbf Compare October 21, 2024 19:16

gene-redpanda changed the title ~~Add Format Flags to topic's describe~~ rpk Add Format Flags to topic's describe Oct 22, 2024

gene-redpanda force-pushed the format-flags-topic-list-detail branch 6 times, most recently from 9fd2385 to f5238f6 Compare October 28, 2024 19:30

gene-redpanda force-pushed the format-flags-topic-list-detail branch from c9beeb2 to dc9149c Compare October 29, 2024 22:06

gene-redpanda force-pushed the format-flags-topic-list-detail branch from dc9149c to 904125a Compare October 30, 2024 03:05

gene-redpanda force-pushed the format-flags-topic-list-detail branch from 904125a to 239b623 Compare November 14, 2024 19:13

feat: add --format to describe

60ca935

Adds format flag to describe Supports JSON and YAML

redpanda-data deleted a comment from vbotbuildovich Nov 19, 2024

r-vasquez force-pushed the format-flags-topic-list-detail branch from 239b623 to 60ca935 Compare November 21, 2024 18:30

r-vasquez reviewed Dec 3, 2024

View reviewed changes

r-vasquez closed this Dec 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rpk Add Format Flags to topic's describe #23418

rpk Add Format Flags to topic's describe #23418

gene-redpanda commented Sep 21, 2024 •

edited

Loading

dotnwat left a comment

vbotbuildovich commented Oct 29, 2024 •

edited

Loading

vbotbuildovich commented Oct 30, 2024 •

edited

Loading

vbotbuildovich commented Oct 30, 2024 •

edited

Loading

vbotbuildovich commented Nov 14, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading

r-vasquez left a comment

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez Nov 29, 2024

r-vasquez commented Dec 3, 2024

		// By default, if neither are specified, we opt in to
		// the config section only.

		} else if len(topicArg) == 0 {
		out.Exit("did not match any topics, exiting.")

	resp, err := req.RequestWith(context.Background(), cl)
	resp, err := req.RequestWith(cmd.Context(), cl)

	func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) (output []describeTopicConfig) {
	func buildDescribeTopicConfig(configs []kmsg.DescribeConfigsResponseResourceConfig) ([]describeTopicConfig) {

rpk Add Format Flags to topic's describe #23418

rpk Add Format Flags to topic's describe #23418

Conversation

gene-redpanda commented Sep 21, 2024 • edited Loading

Backports Required

Release Notes

Improvements

dotnwat left a comment

Choose a reason for hiding this comment

vbotbuildovich commented Oct 29, 2024 • edited Loading

Retry command for Build#57318

vbotbuildovich commented Oct 30, 2024 • edited Loading

Retry command for Build#57327

vbotbuildovich commented Oct 30, 2024 • edited Loading

Retry command for Build#57344

vbotbuildovich commented Nov 14, 2024 • edited Loading

Retry command for Build#58077

vbotbuildovich commented Nov 21, 2024 • edited Loading

vbotbuildovich commented Nov 21, 2024 • edited Loading

Retry command for Build#58482

vbotbuildovich commented Nov 21, 2024 • edited Loading

r-vasquez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-vasquez commented Dec 3, 2024

gene-redpanda commented Sep 21, 2024 •

edited

Loading

vbotbuildovich commented Oct 29, 2024 •

edited

Loading

vbotbuildovich commented Oct 30, 2024 •

edited

Loading

vbotbuildovich commented Oct 30, 2024 •

edited

Loading

vbotbuildovich commented Nov 14, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading

vbotbuildovich commented Nov 21, 2024 •

edited

Loading