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

[ISSUE 7757] Support Persistence Policies on topic level #7817

Merged
merged 4 commits into from
Aug 17, 2020
Merged

[ISSUE 7757] Support Persistence Policies on topic level #7817

merged 4 commits into from
Aug 17, 2020

Conversation

zhanghaou
Copy link
Contributor

Link #7757 and master issue #2688

Motivation

Support set/get/remove persistence policies on topic level.

Verifying this change

new unit test added.

zhanghaou and others added 3 commits August 14, 2020 11:16
@zhanghaou
[ISSUE 7757] Support Persistence Policy on topic level.
7dd9e0e
@zhanghaou
Merge pull request #1 from apache/master
c191054 
merge
@zhanghaou
Merge remote-tracking branch 'origin/master' into persistence-policie…
f43c748 
…s-on-topic-level

# Conflicts:
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
#	pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/Topics.java
#	pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java
@zhanghaou
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

@jianyun8023 Please help review this PR.

@codelipenghui codelipenghui added this to the 2.7.0 milestone Aug 14, 2020
jianyun8023
jianyun8023 reviewed Aug 14, 2020
View reviewed changes
Copy link
Contributor

@jianyun8023 jianyun8023 left a comment

Choose a reason for hiding this comment

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

left some minor comments.

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated
Comment on lines 2298 to 2311
protected void internalGetPersistence(AsyncResponse asyncResponse){
validateAdminAccessForTenant(namespaceName.getTenant());
validatePoliciesReadOnlyAccess();
if (topicName.isGlobal()) {
validateGlobalNamespaceOwnership(namespaceName);
}
Optional<PersistencePolicies> retention = getTopicPolicies(topicName)
.map(TopicPolicies::getPersistence);
if (!retention.isPresent()) {
asyncResponse.resume(Response.noContent().build());
}else {
asyncResponse.resume(retention.get());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: there is no need to use asyncResponse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is same to internalGetRetention(), maybe we can modify it all at once.

codelipenghui
codelipenghui requested changes Aug 14, 2020
View reviewed changes
Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

The change looks do not handle the topic creation according to the topic policy if the topic level persistent policy exists. You can refer to createPersistentTopic method in the BrokerService.java

@zhanghaou
Copy link
Contributor Author

The change looks do not handle the topic creation according to the topic policy if the topic level persistent policy exists. You can refer to createPersistentTopic method in the BrokerService.java

Done.

@zhanghaou
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit ff1780c into apache:master Aug 17, 2020
@codelipenghui codelipenghui mentioned this pull request Aug 17, 2020
Closed
@codelipenghui codelipenghui added the doc-required Your PR changes impact docs and you will update later. label Aug 17, 2020
@codelipenghui codelipenghui mentioned this pull request Aug 17, 2020
Closed
14 tasks
@Huanli-Meng Huanli-Meng mentioned this pull request Aug 19, 2020
Merged
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
@zhanghaou @huangdx0726
[ISSUE 7757] Support Persistence Policies on topic level (apache#7817)
8efa83a 
Link [https://github.com/apache/pulsar/issues/7757](https://github.com/apache/pulsar/issues/7757) and master issue [https://github.com/apache/pulsar/issues/2688](https://github.com/apache/pulsar/issues/2688)

### Motivation

Support set/get/remove persistence  policies on topic level.

### Verifying this change
new unit test added.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@zhanghaou
[ISSUE 7757] Support Persistence Policies on topic level (apache#7817)
59ae37b 
Link [https://github.com/apache/pulsar/issues/7757](https://github.com/apache/pulsar/issues/7757) and master issue [https://github.com/apache/pulsar/issues/2688](https://github.com/apache/pulsar/issues/2688)

### Motivation

Support set/get/remove persistence  policies on topic level.

### Verifying this change
new unit test added.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@zhanghaou
[ISSUE 7757] Support Persistence Policies on topic level (apache#7817)
fe51b88 
Link [https://github.com/apache/pulsar/issues/7757](https://github.com/apache/pulsar/issues/7757) and master issue [https://github.com/apache/pulsar/issues/2688](https://github.com/apache/pulsar/issues/2688)

### Motivation

Support set/get/remove persistence  policies on topic level.

### Verifying this change
new unit test added.
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Sep 5, 2020
@zhanghaou
[ISSUE 7757] Support Persistence Policies on topic level (apache#7817)
43b85b7 
Link [https://github.com/apache/pulsar/issues/7757](https://github.com/apache/pulsar/issues/7757) and master issue [https://github.com/apache/pulsar/issues/2688](https://github.com/apache/pulsar/issues/2688)

### Motivation

Support set/get/remove persistence  policies on topic level.

### Verifying this change
new unit test added.
jiazhai pushed a commit that referenced this pull request Sep 15, 2020
@Huanli-Meng
[Doc]--Update Pulsar Admin CLI doc to support to set/get/remove persi…
fd2319f 
…stence polices, message TTL, and backlog quota on topic level (#7852)

Motivation
In PRs, #7738, #7646, #7817, persistence polices, message TTL, and backlog quota policies are supported on topic level. The PR for code modification is updated and merged. but the doc is not updated accordingly.

This PR is used to support to set/get/remove persistence polices, message TTL, and backlog quota on topic level.

Modifications
Update the Pulsar Admin CLI doc to support to set/get/remove persistence polices, message TTL, and backlog quota on topic level.

* Update backlog-quota policies on topic level

* update contents

* update contents
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Oct 3, 2020
@Huanli-Meng
[Doc]--Update Pulsar Admin CLI doc to support to set/get/remove persi…
db965b5 
…stence polices, message TTL, and backlog quota on topic level (apache#7852)

Motivation
In PRs, apache#7738, apache#7646, apache#7817, persistence polices, message TTL, and backlog quota policies are supported on topic level. The PR for code modification is updated and merged. but the doc is not updated accordingly.

This PR is used to support to set/get/remove persistence polices, message TTL, and backlog quota on topic level.

Modifications
Update the Pulsar Admin CLI doc to support to set/get/remove persistence polices, message TTL, and backlog quota on topic level.

* Update backlog-quota policies on topic level

* update contents

* update contents
@Anonymitaet Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-required Your PR changes impact docs and you will update later. labels Feb 21, 2022
@mattisonchao mattisonchao mentioned this pull request Sep 21, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jianyun8023 jianyun8023 jianyun8023 left review comments

@codelipenghui codelipenghui codelipenghui approved these changes

@jiazhai jiazhai jiazhai approved these changes

Assignees

@zhanghaou zhanghaou

Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.
5 participants
@zhanghaou @codelipenghui @jiazhai @jianyun8023 @Anonymitaet