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

add test for auto-created partitioned system topic #11545

Conversation

wuzhanpeng
Copy link
Contributor

Motivations

When I tried to enable topic-level policy, I found that the tests for interacting with partitioned system topic (e.g. __change_events) was not very sufficient, so I added a test for using auto-created partitioned system topic.

Modifications

@wuzhanpeng
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@wuzhanpeng
Copy link
Contributor Author

/pulsarbot run-failure-checks

name = topicName.getLocalName();
}
return EVENTS_TOPIC_NAMES.contains(name);
return EVENTS_TOPIC_NAMES.contains(TopicName.get(topicName.getPartitionedTopicName()).getLocalName());
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a unit test to cover partitionedTopic and non-partitionedTopic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TopicName#getPartitionedTopicName will check whether topic is partitioned and give the corresponding solution to handle these two situations. That’s why I did this simplification.

@codelipenghui codelipenghui added this to the 2.9.0 milestone Aug 4, 2021
@Anonymitaet
Copy link
Member

Thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@wuzhanpeng
Copy link
Contributor Author

Thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

Sorry for missing some descriptions. This PR does not need to update docs.

Dependencies (does it add or upgrade a dependency): no
The public API: no
The schema: no
The default values of configurations: no
The wire protocol: no
The rest endpoints: no
The admin cli options: no
Anything that affects deployment: no

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Aug 5, 2021
@codelipenghui
Copy link
Contributor

@315157973 Could you please check the @wuzhanpeng's comment?

@codelipenghui codelipenghui merged commit 27dcb0a into apache:master Aug 5, 2021
codelipenghui pushed a commit that referenced this pull request Aug 6, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 6, 2021
LeBW pushed a commit to LeBW/pulsar that referenced this pull request Aug 9, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants