You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/05/16 14:20:56 UTC
[GitHub] [pulsar] shibd opened a new pull request, #15621: Make some operation auto topic creation in Namespaces async.
shibd opened a new pull request, #15621:
URL: https://github.com/apache/pulsar/pull/15621
### Motivation
See PIP https://github.com/apache/pulsar/issues/14365 and change tracker https://github.com/apache/pulsar/issues/15043.
Make NamespacesBase `getAutoTopicCreation / setAutoTopicCreation / removeAutoTopicCreation` methods to pure async.
### Documentation
- [x] `no-need-doc`
(Please explain why)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875401926
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -543,14 +550,25 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace,
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (e.getCause() instanceof NotFoundException) {
+ asyncResponse.resume(new RestException(Status.NOT_FOUND, "Namespace does not exist"));
+ }
Review Comment:
Need else here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1131253359
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
shibd commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1135296394
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875402080
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -561,14 +579,22 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace) {
- try {
validateNamespaceName(property, cluster, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId());
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to remove autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (e.getCause() instanceof NotFoundException) {
+ asyncResponse.resume(new RestException(Status.NOT_FOUND, "Namespace does not exist"));
+ }
Review Comment:
Need else hare
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875402347
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -543,14 +550,25 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace,
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (e.getCause() instanceof NotFoundException) {
Review Comment:
Use FutureUtils.unwrapCompletionException instead of e.getCause()
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -444,10 +444,17 @@ public void removeDeduplication(@PathParam("tenant") String tenant, @PathParam("
@ApiOperation(value = "Get autoTopicCreation info in a namespace")
@ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"),
@ApiResponse(code = 404, message = "Tenant or namespace doesn't exist")})
- public AutoTopicCreationOverride getAutoTopicCreation(@PathParam("tenant") String tenant,
+ public void getAutoTopicCreation(@Suspended AsyncResponse asyncResponse,
+ @PathParam("tenant") String tenant,
@PathParam("namespace") String namespace) {
validateNamespaceName(tenant, namespace);
- return internalGetAutoTopicCreation();
+ internalGetAutoTopicCreationAsync()
+ .thenAccept(autoTopicCreationOverride -> asyncResponse.resume(autoTopicCreationOverride))
+ .exceptionally(ex -> {
Review Comment:
asyncResponse::resume
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
shibd commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1135561640
@mattisonchao @nodece Thanks for your review. Sorry, I miss the change of #15653 when rebasing. All suggest fixed, Please help review again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875403036
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -480,14 +498,22 @@ public void setAutoTopicCreation(
@ApiResponse(code = 404, message = "Tenant or cluster or namespace doesn't exist") })
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace) {
- try {
- validateNamespaceName(tenant, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId());
+ asyncResponse.resume(Response.noContent().build());
Review Comment:
Seems forget to put `namespace`
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -480,14 +498,22 @@ public void setAutoTopicCreation(
@ApiResponse(code = 404, message = "Tenant or cluster or namespace doesn't exist") })
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace) {
- try {
- validateNamespaceName(tenant, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId());
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to remove autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (e.getCause() instanceof MetadataStoreException.NotFoundException) {
+ asyncResponse.resume(new RestException(Response.Status.NOT_FOUND, "Namespace does not exist"));
+ }
Review Comment:
Add else here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875402813
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -463,14 +470,25 @@ public void setAutoTopicCreation(
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace,
@ApiParam(value = "Settings for automatic topic creation", required = true)
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(tenant, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (e.getCause() instanceof MetadataStoreException.NotFoundException) {
+ asyncResponse.resume(new RestException(Response.Status.NOT_FOUND, "Namespace does not exist"));
+ }
Review Comment:
Add else here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875401145
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -524,11 +524,18 @@ public void modifyDeduplication(@PathParam("property") String property, @PathPar
@ApiOperation(value = "Get autoTopicCreation info in a namespace")
@ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have admin permission"),
@ApiResponse(code = 404, message = "Tenant or cluster or namespace doesn't exist")})
- public AutoTopicCreationOverride getAutoTopicCreation(@PathParam("property") String property,
- @PathParam("cluster") String cluster,
- @PathParam("namespace") String namespace) {
+ public void getAutoTopicCreation(@Suspended AsyncResponse asyncResponse,
+ @PathParam("property") String property,
+ @PathParam("cluster") String cluster,
+ @PathParam("namespace") String namespace) {
validateNamespaceName(property, cluster, namespace);
- return internalGetAutoTopicCreation();
+ internalGetAutoTopicCreationAsync()
+ .thenAccept(autoTopicCreationOverride -> asyncResponse.resume(autoTopicCreationOverride))
+ .exceptionally(ex -> {
Review Comment:
asyncResponse::resume
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r875403332
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -480,14 +498,22 @@ public void setAutoTopicCreation(
@ApiResponse(code = 404, message = "Tenant or cluster or namespace doesn't exist") })
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace) {
- try {
- validateNamespaceName(tenant, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId());
+ asyncResponse.resume(Response.noContent().build());
Review Comment:
log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId()) ->
log.info("[{}] Successfully remove autoTopicCreation on namespace {}", clientAppId(), namespaceName);
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
shibd commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1129529058
@Technoboy- Thanks for your review. Fixed, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
shibd commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1134507359
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece commented on a diff in pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#discussion_r880004483
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -572,14 +579,26 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace,
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
Review Comment:
Throwable throwable = FutureUtil.unwrapCompletionException(e);
```suggestion
log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(), namespaceName, throwable)
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -572,14 +579,26 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace,
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
+ namespaceName,
+ e.getCause());
+ if (FutureUtil.unwrapCompletionException(e) instanceof NotFoundException) {
Review Comment:
```suggestion
if (throwable instanceof NotFoundException) {
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -521,14 +540,24 @@ public void setAutoTopicCreation(
@ApiResponse(code = 404, message = "Tenant or cluster or namespace doesn't exist") })
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace) {
- try {
- validateNamespaceName(tenant, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}",
+ clientAppId(), namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to remove autoTopicCreation status on namespace {}", clientAppId(),
Review Comment:
Same above.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -590,14 +609,24 @@ public void setAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
public void removeAutoTopicCreation(@Suspended final AsyncResponse asyncResponse,
@PathParam("property") String property, @PathParam("cluster") String cluster,
@PathParam("namespace") String namespace) {
- try {
validateNamespaceName(property, cluster, namespace);
- internalRemoveAutoTopicCreation(asyncResponse);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ internalSetAutoTopicCreationAsync(null)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully remove autoTopicCreation on namespace {}",
+ clientAppId(), namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to remove autoTopicCreation status on namespace {}", clientAppId(),
Review Comment:
Same above.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -819,56 +818,38 @@ protected void internalSetSubscriptionExpirationTime(Integer expirationTime) {
});
}
- protected AutoTopicCreationOverride internalGetAutoTopicCreation() {
- validateNamespacePolicyOperation(namespaceName, PolicyName.AUTO_TOPIC_CREATION, PolicyOperation.READ);
- Policies policies = getNamespacePolicies(namespaceName);
- return policies.autoTopicCreationOverride;
- }
-
- protected void internalSetAutoTopicCreation(AsyncResponse asyncResponse,
- AutoTopicCreationOverride autoTopicCreationOverride) {
- final int maxPartitions = pulsar().getConfig().getMaxNumPartitionsPerPartitionedTopic();
- validateNamespacePolicyOperation(namespaceName, PolicyName.AUTO_TOPIC_CREATION, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
- if (autoTopicCreationOverride != null) {
- ValidateResult validateResult = AutoTopicCreationOverrideImpl.validateOverride(autoTopicCreationOverride);
- if (!validateResult.isSuccess()) {
- throw new RestException(Status.PRECONDITION_FAILED,
- "Invalid configuration for autoTopicCreationOverride. the detail is "
- + validateResult.getErrorInfo());
- }
- if (Objects.equals(autoTopicCreationOverride.getTopicType(), TopicType.PARTITIONED.toString())) {
- if (maxPartitions > 0 && autoTopicCreationOverride.getDefaultNumPartitions() > maxPartitions) {
- throw new RestException(Status.NOT_ACCEPTABLE,
- "Number of partitions should be less than or equal to " + maxPartitions);
- }
- }
- }
- // Force to read the data s.t. the watch to the cache content is setup.
- namespaceResources().setPoliciesAsync(namespaceName, policies -> {
- policies.autoTopicCreationOverride = autoTopicCreationOverride;
- return policies;
- }).thenApply(r -> {
- String autoOverride = (autoTopicCreationOverride != null
- && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
- log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
- autoOverride != null ? autoOverride : "removed", namespaceName);
- asyncResponse.resume(Response.noContent().build());
- return null;
- }).exceptionally(e -> {
- log.error("[{}] Failed to modify autoTopicCreation status on namespace {}", clientAppId(), namespaceName,
- e.getCause());
- if (e.getCause() instanceof NotFoundException) {
- asyncResponse.resume(new RestException(Status.NOT_FOUND, "Namespace does not exist"));
- return null;
- }
- asyncResponse.resume(new RestException(e.getCause()));
- return null;
- });
+ protected CompletableFuture<AutoTopicCreationOverride> internalGetAutoTopicCreationAsync() {
+ return validateNamespacePolicyOperationAsync(namespaceName, PolicyName.AUTO_TOPIC_CREATION,
+ PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenApply(policies -> policies.autoTopicCreationOverride);
}
- protected void internalRemoveAutoTopicCreation(AsyncResponse asyncResponse) {
- internalSetAutoTopicCreation(asyncResponse, null);
+ protected CompletableFuture<Void> internalSetAutoTopicCreationAsync(
+ AutoTopicCreationOverride autoTopicCreationOverride) {
+ return validateNamespacePolicyOperationAsync(namespaceName,
+ PolicyName.AUTO_TOPIC_CREATION, PolicyOperation.WRITE)
+ .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
+ .thenAccept(__ -> {
+ int maxPartitions = pulsar().getConfig().getMaxNumPartitionsPerPartitionedTopic();
+ if (autoTopicCreationOverride != null) {
+ ValidateResult validateResult =
+ AutoTopicCreationOverrideImpl.validateOverride(autoTopicCreationOverride);
+ if (!validateResult.isSuccess()) {
+ throw new RestException(Status.PRECONDITION_FAILED,
+ "Invalid configuration for autoTopicCreationOverride. the detail is "
+ + validateResult.getErrorInfo());
+ }
+ if (maxPartitions > 0 && autoTopicCreationOverride.getDefaultNumPartitions() > maxPartitions) {
Review Comment:
`if (Objects.equals(autoTopicCreationOverride.getTopicType(), TopicType.PARTITIONED.toString()))` is miss.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -504,14 +511,26 @@ public void setAutoTopicCreation(
@PathParam("tenant") String tenant, @PathParam("namespace") String namespace,
@ApiParam(value = "Settings for automatic topic creation", required = true)
AutoTopicCreationOverride autoTopicCreationOverride) {
- try {
- validateNamespaceName(tenant, namespace);
- internalSetAutoTopicCreation(asyncResponse, autoTopicCreationOverride);
- } catch (RestException e) {
- asyncResponse.resume(e);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSetAutoTopicCreationAsync(autoTopicCreationOverride)
+ .thenAccept(__ -> {
+ String autoOverride = (autoTopicCreationOverride != null
+ && autoTopicCreationOverride.isAllowAutoTopicCreation()) ? "enabled" : "disabled";
+ log.info("[{}] Successfully {} autoTopicCreation on namespace {}", clientAppId(),
+ autoOverride, namespaceName);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(e -> {
+ log.error("[{}] Failed to set autoTopicCreation status on namespace {}", clientAppId(),
Review Comment:
Same above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15621:
URL: https://github.com/apache/pulsar/pull/15621#issuecomment-1135296830
@shibd Looks like this fix is missing, is it rebase issue?
https://github.com/apache/pulsar/pull/15653
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #15621: [improve][broker] Make some operation auto topic creation in Namespaces async.
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #15621:
URL: https://github.com/apache/pulsar/pull/15621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org