Skip to content

MINOR: Do not print an empty line when no topics exist#13762

Closed
KarboniteKream wants to merge 1 commit intoapache:trunkfrom
KarboniteKream:fix/list-no-topics
Closed

MINOR: Do not print an empty line when no topics exist#13762
KarboniteKream wants to merge 1 commit intoapache:trunkfrom
KarboniteKream:fix/list-no-topics

Conversation

@KarboniteKream
Copy link
Copy Markdown

@KarboniteKream KarboniteKream commented May 25, 2023

When running kafka-topics.sh --list, the command will print an empty line if no topics exist. This can cause some confusion when running with wc -l, so it's better to make sure the number of lines is zero in such a case.

This was tested against a cluster with 0 and 1+ topics to make sure the output is correct.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Copy Markdown
Member

@divijvaidya divijvaidya left a comment

Choose a reason for hiding this comment

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

Thank you for your first contribution to the project!

While the change looks good, I am afraid that changing the output format may break scripts that may be relying on the "bug" / presence of a new line.

I would be ok with this breaking change since we don't promise output compatibility of shell commands.

Let's hear what the others in the community have to say about this.

@vamossagar12
Copy link
Copy Markdown
Contributor

Thanks for the PR @KarboniteKream . I agree with Divij's statement above about existing client might break. Also, there is an ongoing effort to modularize the core monolith module which also covers some of the tools related migration. As part of that, TopicCommand is also covered and there's an open PR for the same #13201

Since your PR is against master, you might want to align your PR with that PR? WDYT?

@KarboniteKream
Copy link
Copy Markdown
Author

Sure, I understand the concerns.

you might want to align your PR with that PR?

Do you mean waiting for that PR to get merged, then rebase my changes? Or add my change to that PR?

@vamossagar12
Copy link
Copy Markdown
Contributor

Sure, I understand the concerns.

you might want to align your PR with that PR?

Do you mean waiting for that PR to get merged, then rebase my changes? Or add my change to that PR?

You could wait for the PR to get merged if that's ok to you. It's your call TBH :)

@KarboniteKream
Copy link
Copy Markdown
Author

Sure, it's no problem for me to wait. I just wanted to confirm what you had in mind.

@github-actions
Copy link
Copy Markdown

This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch)

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

@github-actions github-actions Bot added the stale Stale PRs label Sep 13, 2023
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This was logging as Created topic Optional[foobar].

@KarboniteKream
Copy link
Copy Markdown
Author

@divijvaidya @vamossagar12 I have rebased my PR now that #13201 has been merged.

@github-actions github-actions Bot removed the stale Stale PRs label Oct 18, 2023
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 2, 2025

This PR is being marked as stale since it has not had any activity in 90 days. If you
would like to keep this PR alive, please leave a comment asking for a review. If the PR has
merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

@github-actions github-actions Bot added the stale Stale PRs label Jan 2, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 2, 2025

This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.

@github-actions github-actions Bot added the closed-stale PRs that were closed due to inactivity label Feb 2, 2025
@github-actions github-actions Bot closed this Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closed-stale PRs that were closed due to inactivity stale Stale PRs tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants