Skip to content

Conversation

@mattisonchao
Copy link
Member

Master Issue: #14013

Motivation

Make Rest API getPartitionedTopicList to async and to refactor code style to make it more clear.

Modifications

  • Make the getPartitionedTopicList method from sync to async.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

none.

Need to update docs?

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 30, 2022
@mattisonchao
Copy link
Member Author

@codelipenghui @eolivelli @Technoboy- @Jason918 @shoothzj @RobertIndie @nodece

PTAL, when you have time. :)

@mattisonchao mattisonchao changed the title [Issue 14013] RestAPI - Make getPartitionedTopicList to async. [Issue 14013] Make PersistentTopics#getPartitionedTopicList to pure async. Feb 9, 2022
}
return getPartitionedTopicList(TopicDomain.getEnum(domain()));
protected CompletableFuture<List<String>> internalGetPartitionedTopicList() {
return validateNamespaceOperationAsync(namespaceName, NamespaceOperation.GET_TOPICS)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about check namespaceExistsAsync before validateNamespaceOperationAsync

.get(config().getZooKeeperOperationTimeoutSeconds(), TimeUnit.SECONDS);
topics.addAll(getPartitionedTopicList(TopicDomain.persistent));
topics.addAll(getPartitionedTopicList(TopicDomain.non_persistent));
.get(timeoutSeconds, TimeUnit.SECONDS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not async?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not the focus of this refactoring, it just affects here. And will keep the original code logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants