Skip to content
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.

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

feat(kafka topic): add search flag to list subcommand #709

Merged
merged 3 commits into from
Jun 11, 2021

Conversation

rkpattnaik780
Copy link
Contributor

Allow user to filter list of kafka topics by passing --search flag to rhoas kafka topic list

Closes #697

Screenshot from 2021-06-10 16-00-37

Verification Steps

  1. go run ./cmd/rhoas kafka topic list --search "pic(-1"

  2. It should throw the error message Error: illegal search value "pic(-1"; only letters (Aa-Zz), numbers, "_", "." and "-" are accepted
  3. Run go run ./cmd/rhoas kafka topic list --search <search input>

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change
  • Other (please specify)

Checklist

  • Documentation added for the feature
  • CI and all relevant tests are passing
  • Code Review completed
  • Verified independently by reviewer

@rkpattnaik780 rkpattnaik780 added the enhancement New feature or request label Jun 10, 2021
Copy link
Contributor

@craicoverflow craicoverflow left a comment

Choose a reason for hiding this comment

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

Couple of small changes needed.

pkg/kafka/topic/validators.go Show resolved Hide resolved
one = 'Kafka instance "{{.InstanceName}}" has no topics. Run "rhoas kafka topic create <topic-name>" to create one'
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably this message should not be used when the search flag is use - there could be topics but not showing up because of the filter value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed! --search for kafka instances returns No kafka instances were found for both no instance created and no instance found after searching. Should we modify the message in same manner for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

No Kafka instance were found is good because it does not call out that there are no topics - just that we did not find any. Sure, let's change the topic message to be consistent :)

@rkpattnaik780 rkpattnaik780 merged commit ac694d2 into main Jun 11, 2021
@rkpattnaik780 rkpattnaik780 deleted the topic_filter branch June 11, 2021 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add search flag for kafka topic list
2 participants