Skip to content

[improve][CLI] Support filtering system topic when get list. #15410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Technoboy- merged 8 commits into from
May 8, 2022
Merged

[improve][CLI] Support filtering system topic when get list. #15410

Technoboy- merged 8 commits into from
May 8, 2022

Conversation

@Technoboy-
Copy link
Contributor

@Technoboy- Technoboy- commented May 2, 2022 •
edited
Loading

Motivation

When users use CLI or REST to get topic list, there is going to show all the topics including system topics. This may confuse users, because of not created by themselves.

In this patch, I decided to add an option --include-system-topic when getting the topic list. The default value if false, only show user-created topics.

Modification

  • Add --include-system-topic for list and list-partitioned-topics for CLI.
  • Use ListTopicsOptions instead of Map as the query condition.
  • Deprecate getList(String namespace, TopicDomain topicDomain, Map<QueryParam, Object> params)

Documentation

  • no-need-doc
    (Please explain why)

@Technoboy- Technoboy- self-assigned this May 2, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 2, 2022
@Technoboy- Technoboy- added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/cli area/admin labels May 2, 2022
@Technoboy- Technoboy- added this to the 2.11.0 milestone May 2, 2022
@Technoboy- Technoboy- changed the title Support filtering system topic when get list. [improve][CLI] Support filtering system topic when get list. May 2, 2022
Jason918
Jason918 reviewed May 2, 2022
View reviewed changes
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

This seems to be a breaking change. Do we have anyway to mark this in release note?

@Technoboy-
Copy link
Contributor Author

Technoboy- commented May 5, 2022

This seems to be a breaking change. Do we have anyway to mark this in release note?

I have added the release/note-required label.

mattisonchao
mattisonchao approved these changes May 6, 2022
View reviewed changes
Copy link
Member

@mattisonchao mattisonchao left a comment

Choose a reason for hiding this comment

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

LGTM +1

@codelipenghui
Copy link
Contributor

codelipenghui commented May 6, 2022

@Jason918 Please help review again

gaoran10
gaoran10 approved these changes May 6, 2022
View reviewed changes
Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

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

LGTM

@Technoboy- Technoboy- merged commit a0dcab6 into apache:master May 8, 2022
@Technoboy- Technoboy- mentioned this pull request Jun 22, 2022
Merged
1 task
codelipenghui pushed a commit that referenced this pull request Jun 26, 2022
@Technoboy-
[feature][CLI] Add query options to the get topics in Namespace. (#16167
6c715e0 
)

### Motivation

Since #2025 has added query param - `Mode`,  but the CLI is still not supported.

#15410 has introduced to filter system topic. 

So this patch is supported the above query params for Namespace#getTopics.
@Technoboy- Technoboy- deleted the list-topic-with-system-topic-option branch August 10, 2022 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gaoran10 gaoran10 gaoran10 approved these changes

@Jason918 Jason918 Jason918 approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@shibd shibd shibd approved these changes

@mattisonchao mattisonchao mattisonchao approved these changes

@congbobo184 congbobo184 Awaiting requested review from congbobo184

Assignees

@Technoboy- Technoboy-

Labels

area/admin area/cli doc-not-needed Your PR changes do not impact docs type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Milestone

2.11.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Technoboy- @codelipenghui @Jason918 @gaoran10 @shibd @mattisonchao