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.

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

[Pulsar Admin CLI Tool] Use NoSplitter for subscription properties #16862

Merged
merged 1 commit into from Jul 29, 2022
Merged

[Pulsar Admin CLI Tool] Use NoSplitter for subscription properties #16862

merged 1 commit into from Jul 29, 2022

Conversation

michaeljmarshall
Copy link
Member

Motivation

Subscription properties are a new feature coming in 2.11. The admin api will allow for properties with commas, but the admin cli tool splits parameters with commas into multiple properties. This PR switches the splitter for the properties field and makes it possible to specify any valid property with the cli tool.

Note: there are other properties fields that the CLI tool allows you to set, but because those have already been released, I am not updating those here.

Modifications

  • Use the NoSplitter.class as the splitter for the subscription properties parameter.
  • Add test coverage for all changes.

Verifying this change

This PR includes new test coverage.

Does this pull request potentially affect one of the following parts:

This PR updates a CLI tool method that has not yet been released, so it does not break any existing functionality.

Documentation

  • doc-not-needed

@michaeljmarshall michaeljmarshall added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/cli area/admin doc-not-needed Your PR changes do not impact docs labels Jul 29, 2022
@michaeljmarshall michaeljmarshall added this to the 2.11.0 milestone Jul 29, 2022
@michaeljmarshall michaeljmarshall self-assigned this Jul 29, 2022
eolivelli
eolivelli approved these changes Jul 29, 2022
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@codelipenghui codelipenghui merged commit da5658c into apache:master Jul 29, 2022
@michaeljmarshall michaeljmarshall deleted the properties-with-commas branch July 29, 2022 14:35
Gleiphir2769 pushed a commit to Gleiphir2769/pulsar that referenced this pull request Aug 4, 2022
@michaeljmarshall @Gleiphir2769
[Pulsar Admin CLI Tool] Use NoSplitter for subscription properties (a…
0c7e1e4 
…pache#16862)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Technoboy- Technoboy- Technoboy- approved these changes

@eolivelli eolivelli eolivelli approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@congbobo184 congbobo184 Awaiting requested review from congbobo184

Assignees

@michaeljmarshall michaeljmarshall

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.

None yet
4 participants
@michaeljmarshall @Technoboy- @eolivelli @codelipenghui