[pulsar-broker] Support of TopicLifecyclePolicies REST handler.#13195
[pulsar-broker] Support of TopicLifecyclePolicies REST handler.#13195ciaocloud wants to merge 3 commits intoapache:masterfrom
Conversation
|
@ciaocloud:Thanks for your contribution. For this PR, do we need to update docs? |
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
0235bad to
fa9e21c
Compare
ciaocloud
changed the title
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
fa9e21c to
db047f1
Compare
|
Hi @ciaocloud is this a code cleanup work and do not need to update docs? when submitting a PR, can you help provide a doc label (tick the box) in the PR template which contains info about doc? This helps others know more about the changes. Thanks |
Jason918
left a comment
Jason918
left a comment
There was a problem hiding this comment.
I am confused about the relation between InactiveTopicPolicies and TopicLifecycle. Can you bring more details?
| private boolean autoCreateTopics; | ||
|
|
||
| /** | ||
| * Whether topics should be automatically deleted when inactive, or rather or rather explicitly deleted through |
There was a problem hiding this comment.
Typo "or rather or rather"
There was a problem hiding this comment.
I am confused about the relation between
InactiveTopicPoliciesandTopicLifecycle. Can you bring more details?
Both InactiveTopicPolicies and TopicLifecycle are fields declared in Policies.
The InactiveTopicPolicies has a boolean value deleteWhileInactive which determines whether topics should be automatically deleted when inactive.
We ask it to be same as that defined in TopicLifecycle, while TopicLifecycle also defines whether topics should be automatically created.
This endpoint is used during tenant creation time.
(Also would update the above in the PR description)
There was a problem hiding this comment.
We ask it to be same as that defined in
TopicLifecycle
How can you guarantee this? Current code clearly missed some part.
And I don't think it's a good design that we have two field defines the same things ( InactiveTopicPolicies#deleteWhileInactive and TopicLifecyclePolicies#autoDeleteTopics )
| //If the topic-level policy already exists, the namespace-level policy cannot override the topic-level policy. | ||
| Optional<TopicPolicies> topicPolicies = getTopicPolicies(); | ||
|
|
||
| if (data.topicLifecycle != null) { |
There was a problem hiding this comment.
We should update all topicPolicies values in updateTopicPolicyByNamespacePolicy
There was a problem hiding this comment.
We can definitely refactor it into the updateTopicPolicyByNamespacePolicy method in AbstractTopic, but as that method would be used by both PersistentTopic and NonPersistentTopic, not sure it is the right thing to do for the non-persistent one to update inactiveTopicPolices with topicLifecycle info. will need to double-check.
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
Outdated
Show resolved
Hide resolved
|
@ciaocloud:Thanks for providing doc info! |
github-actions
bot
added
doc-not-needed
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
db047f1 to
3e195fd
Compare
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
3e195fd to
b110960
Compare
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
b110960 to
d1434c5
Compare
…hod in AbstractTopic.
ciaocloud
force-pushed
the
topiclifecyclepolicies
branch
from
074e7ea to
d62aba1
Compare
|
The pr had no activity for 30 days, mark with Stale label. |
|
The pr had no activity for 30 days, mark with Stale label. |
|
Closed as stale and conflict. Please rebase and resubmit the patch if it's still relevant. |
5 participants
Originally authored by @merlimat, fixed for compatibility with 2.10 branch. (Note that with the first commit only, it would adapt to 2.9 branch)
Motivation
Support of TopicLifecyclePolicies REST handler.
Modifications
The added endpoint
TopicLifecyclecan be used during tenant creation time.Both
InactiveTopicPoliciesandTopicLifecycleare fields declared inPolicies. TheInactiveTopicPolicieshas a boolean valuedeleteWhileInactivewhich determines whether topics should be automatically deleted when inactive. We ask it to be same as that defined inTopicLifecycle, whileTopicLifecycleadditionally defines whether topics should be automatically created when there exists producer/consumer connection (which is oppose to being created explicitly with API).Specifically, our changes include:
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
Documentation
no-need-doc