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

[Branch-2.7][Broker] Fix using partitioned topic name to get topic policies #11897

Conversation

gaoran10
Copy link
Contributor

@gaoran10 gaoran10 commented Sep 2, 2021

Motivation

There is a bug that using the partitioned topic name to get topic policies. The PR #11294 fix this issue, but it's hard to cherry-pick the PR to branch-2.7, so create this PR to fix the issue in branch-2.7.

This PR contains PR-11294 and PR-11863.

Modifications

  1. Fix using the partitioned topic name to get topic policies.
  2. Change some warning logs to debug level for the getting topic policies operation.

Verifying this change

The test method TopicPoliciesTest#testBacklogQuotaWithPartitionedTopic is used to verify getting topic policies by the partitioned topic name.

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

If yes was chosen, please highlight the changes

  • 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)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@gaoran10 gaoran10 self-assigned this Sep 2, 2021
@gaoran10 gaoran10 added component/topic-policy type/bug The PR fixed a bug or issue reported a bug labels Sep 2, 2021
Copy link
Contributor

@congbobo184 congbobo184 left a comment

Choose a reason for hiding this comment

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

LGTM. left one comment.

@@ -593,8 +593,6 @@ public void testTlsEnabledWithoutNonTlsServicePorts() throws Exception {

} catch (Exception e) {
fail("should not fail");
} finally {
Copy link
Contributor

Choose a reason for hiding this comment

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

why delete this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If close the pulsarClient, it will cause other test methods can't use the pulsarClient and there is a clean operation after the test.

@codelipenghui codelipenghui merged commit 3876c56 into apache:branch-2.7 Sep 6, 2021
@gaoran10 gaoran10 deleted the gaoran/branch-2.7-fix-partition-topic-policy branch September 6, 2021 03:38
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Dec 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life doc-not-needed Your PR changes do not impact docs release/2.7.4 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants