nodece commented on code in PR #16411:
URL: https://github.com/apache/pulsar/pull/16411#discussion_r920246448
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -802,15 +802,22 @@ public void removeAutoSubscriptionCreation(@Suspended
final AsyncResponse asyncR
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
@ApiResponse(code = 412, message = "Namespace is not setup to
split in bundles") })
- public BundlesData getBundlesData(@PathParam("property") String property,
@PathParam("cluster") String cluster,
- @PathParam("namespace") String namespace) {
- validatePoliciesReadOnlyAccess();
- validateNamespaceName(property, cluster, namespace);
- validateNamespaceOperation(NamespaceName.get(property, namespace),
NamespaceOperation.GET_BUNDLE);
-
- Policies policies = getNamespacePolicies(namespaceName);
-
- return policies.bundles;
+ public void getBundlesData(@Suspended final AsyncResponse asyncResponse,
+ @PathParam("property") String property,
+ @PathParam("cluster") String cluster,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(property, cluster, namespace);
+ validatePoliciesReadOnlyAccessAsync()
+ .thenCompose(__ ->
validateNamespaceOperationAsync(NamespaceName.get(property, namespace),
+ NamespaceOperation.GET_BUNDLE))
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenAccept(policies -> asyncResponse.resume(policies.bundles))
+ .exceptionally(ex -> {
+ log.error("[{}] Failed to get bundle data for namespace
{}: {} ", clientAppId(),
+ namespaceName, ex.getCause().getMessage(), ex);
Review Comment:
```suggestion
namespaceName, ex);
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -802,15 +802,22 @@ public void removeAutoSubscriptionCreation(@Suspended
final AsyncResponse asyncR
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Property or cluster or
namespace doesn't exist"),
@ApiResponse(code = 412, message = "Namespace is not setup to
split in bundles") })
- public BundlesData getBundlesData(@PathParam("property") String property,
@PathParam("cluster") String cluster,
- @PathParam("namespace") String namespace) {
- validatePoliciesReadOnlyAccess();
- validateNamespaceName(property, cluster, namespace);
- validateNamespaceOperation(NamespaceName.get(property, namespace),
NamespaceOperation.GET_BUNDLE);
-
- Policies policies = getNamespacePolicies(namespaceName);
-
- return policies.bundles;
+ public void getBundlesData(@Suspended final AsyncResponse asyncResponse,
+ @PathParam("property") String property,
+ @PathParam("cluster") String cluster,
+ @PathParam("namespace") String namespace) {
+ validateNamespaceName(property, cluster, namespace);
+ validatePoliciesReadOnlyAccessAsync()
+ .thenCompose(__ ->
validateNamespaceOperationAsync(NamespaceName.get(property, namespace),
+ NamespaceOperation.GET_BUNDLE))
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenAccept(policies -> asyncResponse.resume(policies.bundles))
+ .exceptionally(ex -> {
+ log.error("[{}] Failed to get bundle data for namespace
{}: {} ", clientAppId(),
Review Comment:
```suggestion
log.error("[{}] Failed to get bundle data for namespace
{} ", clientAppId(),
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -892,15 +899,29 @@ public void splitNamespaceBundle(
@QueryParam("authoritative") @DefaultValue("false") boolean
authoritative,
@QueryParam("unload") @DefaultValue("false") boolean unload,
@QueryParam("splitBoundaries") @DefaultValue("") List<Long>
splitBoundaries) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSplitNamespaceBundle(asyncResponse, bundleRange,
- authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries);
- } catch (WebApplicationException wae) {
- asyncResponse.resume(wae);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSplitNamespaceBundleAsync(bundleRange,
+ authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully split namespace bundle {}",
clientAppId(), bundleRange);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(ex -> {
+ if (!isRedirectException(ex)) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
Review Comment:
```suggestion
log.error("[{}] Failed to split namespace bundle
{}/{},
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -892,15 +899,29 @@ public void splitNamespaceBundle(
@QueryParam("authoritative") @DefaultValue("false") boolean
authoritative,
@QueryParam("unload") @DefaultValue("false") boolean unload,
@QueryParam("splitBoundaries") @DefaultValue("") List<Long>
splitBoundaries) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSplitNamespaceBundle(asyncResponse, bundleRange,
- authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries);
- } catch (WebApplicationException wae) {
- asyncResponse.resume(wae);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSplitNamespaceBundleAsync(bundleRange,
+ authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully split namespace bundle {}",
clientAppId(), bundleRange);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(ex -> {
+ if (!isRedirectException(ex)) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
+ clientAppId(), namespaceName, bundleRange,
ex.getMessage());
Review Comment:
```suggestion
clientAppId(), namespaceName, bundleRange,
ex);
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -726,15 +725,21 @@ public void removeAutoSubscriptionCreation(@Suspended
final AsyncResponse asyncR
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Tenant or cluster or namespace
doesn't exist"),
@ApiResponse(code = 412, message = "Namespace is not setup to
split in bundles") })
- public BundlesData getBundlesData(@PathParam("tenant") String tenant,
- @PathParam("namespace") String namespace) {
- validatePoliciesReadOnlyAccess();
+ public void getBundlesData(@Suspended final AsyncResponse asyncResponse,
+ @PathParam("tenant") String tenant,
+ @PathParam("namespace") String
namespace) {
validateNamespaceName(tenant, namespace);
- validateNamespaceOperation(NamespaceName.get(tenant, namespace),
NamespaceOperation.GET_BUNDLE);
-
- Policies policies = getNamespacePolicies(namespaceName);
-
- return policies.bundles;
+ validatePoliciesReadOnlyAccessAsync()
+ .thenCompose(__ ->
validateNamespaceOperationAsync(NamespaceName.get(tenant, namespace),
+ NamespaceOperation.GET_BUNDLE))
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenAccept(policies -> asyncResponse.resume(policies.bundles))
+ .exceptionally(ex -> {
+ log.error("[{}] Failed to get bundle data for namespace
{}: {} ", clientAppId(),
+ namespaceName, ex.getCause().getMessage(), ex);
Review Comment:
```suggestion
namespaceName, ex);
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -892,15 +899,29 @@ public void splitNamespaceBundle(
@QueryParam("authoritative") @DefaultValue("false") boolean
authoritative,
@QueryParam("unload") @DefaultValue("false") boolean unload,
@QueryParam("splitBoundaries") @DefaultValue("") List<Long>
splitBoundaries) {
- try {
- validateNamespaceName(property, cluster, namespace);
- internalSplitNamespaceBundle(asyncResponse, bundleRange,
- authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries);
- } catch (WebApplicationException wae) {
- asyncResponse.resume(wae);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(property, cluster, namespace);
+ internalSplitNamespaceBundleAsync(bundleRange,
+ authoritative, unload,
NamespaceBundleSplitAlgorithm.RANGE_EQUALLY_DIVIDE_NAME, splitBoundaries)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully split namespace bundle {}",
clientAppId(), bundleRange);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(ex -> {
+ if (!isRedirectException(ex)) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
+ clientAppId(), namespaceName, bundleRange,
ex.getMessage());
+ }
+ Throwable realCause =
FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof IllegalArgumentException) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
Review Comment:
Log repeatedly, you should remove this line.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -817,15 +822,28 @@ public void splitNamespaceBundle(
@QueryParam("unload") @DefaultValue("false") boolean unload,
@QueryParam("splitAlgorithmName") String splitAlgorithmName,
@ApiParam("splitBoundaries") List<Long> splitBoundaries) {
- try {
- validateNamespaceName(tenant, namespace);
- internalSplitNamespaceBundle(asyncResponse,
- bundleRange, authoritative, unload, splitAlgorithmName,
splitBoundaries);
- } catch (WebApplicationException wae) {
- asyncResponse.resume(wae);
- } catch (Exception e) {
- asyncResponse.resume(new RestException(e));
- }
+ validateNamespaceName(tenant, namespace);
+ internalSplitNamespaceBundleAsync(bundleRange, authoritative, unload,
splitAlgorithmName, splitBoundaries)
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully split namespace bundle {}",
clientAppId(), bundleRange);
+ asyncResponse.resume(Response.noContent().build());
+ })
+ .exceptionally(ex -> {
+ if (!isRedirectException(ex)) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
+ clientAppId(), namespaceName, bundleRange,
ex.getMessage());
+ }
+ Throwable realCause =
FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof IllegalArgumentException) {
+ log.error("[{}] Failed to split namespace bundle {}/{}
due to {}",
Review Comment:
Log repeatedly, you should remove this line.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/Namespaces.java:
##########
@@ -726,15 +725,21 @@ public void removeAutoSubscriptionCreation(@Suspended
final AsyncResponse asyncR
@ApiResponses(value = { @ApiResponse(code = 403, message = "Don't have
admin permission"),
@ApiResponse(code = 404, message = "Tenant or cluster or namespace
doesn't exist"),
@ApiResponse(code = 412, message = "Namespace is not setup to
split in bundles") })
- public BundlesData getBundlesData(@PathParam("tenant") String tenant,
- @PathParam("namespace") String namespace) {
- validatePoliciesReadOnlyAccess();
+ public void getBundlesData(@Suspended final AsyncResponse asyncResponse,
+ @PathParam("tenant") String tenant,
+ @PathParam("namespace") String
namespace) {
validateNamespaceName(tenant, namespace);
- validateNamespaceOperation(NamespaceName.get(tenant, namespace),
NamespaceOperation.GET_BUNDLE);
-
- Policies policies = getNamespacePolicies(namespaceName);
-
- return policies.bundles;
+ validatePoliciesReadOnlyAccessAsync()
+ .thenCompose(__ ->
validateNamespaceOperationAsync(NamespaceName.get(tenant, namespace),
+ NamespaceOperation.GET_BUNDLE))
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName))
+ .thenAccept(policies -> asyncResponse.resume(policies.bundles))
+ .exceptionally(ex -> {
+ log.error("[{}] Failed to get bundle data for namespace
{}: {} ", clientAppId(),
Review Comment:
```suggestion
log.error("[{}] Failed to get bundle data for namespace
{}", clientAppId(),
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
