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/01/26 14:20:41 UTC
[GitHub] [pulsar] mattisonchao opened a new pull request #13970: Make ``NamespaceResourceGroup`` get/set/delete to pure async.
mattisonchao opened a new pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970
### Motivation
Async rest api is more efficient than sync. I think we could refactor all sync api to async.
### Modification
- Change ``NamespaceResourceGroup`` get/set/delete to pure async.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
- [x] `no-need-doc`
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795055486
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1751,46 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ assertNotFound(__ -> admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName));
Review comment:
```suggestion
assertEquals(assertThrows(PulsarAdminException.class,
() -> admin.namespaces().setNamespaceResourceGroup(namespace,
resourceGroupName)).getStatusCode(), Response.Status.NOT_FOUND.getStatusCode());
```
You can use this code. By the way, add `PulsarAdminAssert` and `ThrowingConsumer` looks complex.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009264
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
Review comment:
You should move this to line 2760, and log `realCause`. Or miss the `realCause`?
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795030781
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
+ asyncResponse.resume(new RestException(realCause));
+ }
+ return null;
+ });
}
-
- private static final Logger log = LoggerFactory.getLogger(NamespacesBase.class);
+ protected void internalSetNamespaceResourceGroup(AsyncResponse asyncResponse, String rgName) {
+ validateNamespacePolicyOperationAsync(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE)
+ .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
+ .thenCompose(__ -> {
+ if (rgName != null) {
+ return resourceGroupResources().getResourceGroupAsync(rgName)
+ .thenAccept(resourceGroup -> {
+ // check resource group exists.
+ if (!resourceGroup.isPresent()) {
+ throw new RestException(Status.PRECONDITION_FAILED,
+ "ResourceGroup does not exist");
+ }
+ });
+ } else {
+ return CompletableFuture.completedFuture(null);
+ }
+ }).thenCompose(__ -> internalSetPoliciesAsync("resource_group_name", rgName))
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully to set namespace resource group {}", clientAppId(), rgName);
+ asyncResponse.resume(Response.noContent().build());
+ }).exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to set namespace resource group {} : {}", clientAppId(), rgName,
Review comment:
fixed
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
Review comment:
fixed
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
Review comment:
fixed
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
fixed
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
fixed
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
Review comment:
fixed
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795030959
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
Could you give me more information about why use it?
it's a sync method, I think we don't need to use ``Awaitility ``.
--
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 closed pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010622
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
Should we use the `Awaitility.await().untilAsserted()`?
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1023297025
@codelipenghui @liudezhi2098 @Technoboy- @Jason918 PTAL, when you have time : )
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795055486
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1751,46 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ assertNotFound(__ -> admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName));
Review comment:
```suggestion
assertEquals(assertThrows(PulsarAdminException.class,
() -> admin.namespaces().setNamespaceResourceGroup(namespace,
resourceGroupName)).getStatusCode(), Response.Status.NOT_FOUND.getStatusCode());
```
You can use this code, add `PulsarAdminAssert` and `ThrowingConsumer` looks complex.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795055486
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1751,46 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ assertNotFound(__ -> admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName));
Review comment:
```suggestion
assertEquals(assertThrows(PulsarAdminException.class,
() -> admin.namespaces().setNamespaceResourceGroup(namespace,
resourceGroupName)).getStatusCode(), Response.Status.NOT_FOUND.getStatusCode());
```
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794986882
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
Review comment:
Sure, I will do it later.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010048
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
Use `assertThrows` to check this.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794241218
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
##########
@@ -906,24 +907,42 @@ public void validateNamespaceOperation(NamespaceName namespaceName, NamespaceOpe
return CompletableFuture.completedFuture(null);
}
- public void validateNamespacePolicyOperation(NamespaceName namespaceName,
- PolicyName policy,
- PolicyOperation operation) {
+ public CompletableFuture<Void> validateNamespacePolicyOperationAsync(NamespaceName namespaceName,
+ PolicyName policy,
+ PolicyOperation operation) {
if (pulsar().getConfiguration().isAuthenticationEnabled()
- && pulsar().getBrokerService().isAuthorizationEnabled()) {
+ && pulsar().getBrokerService().isAuthorizationEnabled()) {
if (!isClientAuthenticated(clientAppId())) {
- throw new RestException(Status.FORBIDDEN, "Need to authenticate to perform the request");
+ return FutureUtil.failedFuture(
+ new RestException(Status.FORBIDDEN, "Need to authenticate to perform the request"));
}
+ return pulsar().getBrokerService().getAuthorizationService()
+ .allowNamespacePolicyOperationAsync(namespaceName, policy, operation,
+ originalPrincipal(), clientAppId(), clientAuthData())
+ .thenAccept(isAuthorized -> {
+ if (!isAuthorized) {
+ throw new RestException(Status.FORBIDDEN,
+ String.format("Unauthorized to validateNamespacePolicyOperation for"
+ + " operation [%s] on namespace [%s] on policy [%s]",
+ operation.toString(), namespaceName, policy.toString()));
+ }
+ });
+ }
+ return CompletableFuture.completedFuture(null);
+ }
- boolean isAuthorized = pulsar().getBrokerService().getAuthorizationService()
- .allowNamespacePolicyOperation(namespaceName, policy, operation,
- originalPrincipal(), clientAppId(), clientAuthData());
-
- if (!isAuthorized) {
- throw new RestException(Status.FORBIDDEN,
- String.format("Unauthorized to validateNamespacePolicyOperation for"
- + " operation [%s] on namespace [%s] on policy [%s]",
- operation.toString(), namespaceName, policy.toString()));
+ public void validateNamespacePolicyOperation(NamespaceName namespaceName,
+ PolicyName policy,
+ PolicyOperation operation) {
+ try {
+ validateNamespacePolicyOperationAsync(namespaceName, policy, operation)
+ .get(namespaceResources().getOperationTimeoutSec(), SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
```suggestion
throw realCause;
```
Looks enough.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
##########
@@ -972,15 +991,40 @@ public static ObjectMapper jsonMapper() {
return ObjectMapperFactory.getThreadLocal();
}
+ public CompletableFuture<Void> validatePoliciesReadOnlyAccessAsync() {
+ return pulsar().getPulsarResources()
+ .getNamespaceResources().getPoliciesReadOnlyAsync()
+ .thenAccept(arePoliciesReadOnly -> {
+ if (arePoliciesReadOnly) {
+ log.warn("Policies are read-only. Broker cannot do read-write operations");
+ throw new RestException(Status.FORBIDDEN, "Broker is forbidden to do read-write operations");
+ } else {
+ if (log.isDebugEnabled()) {
+ // Do nothing, just log the message.
+ log.debug("Broker is allowed to make read-write operations");
+ }
+ }
+ });
+ }
+
+ /**
+ * Checks whether the broker is allowed to do read-write operations based on the existence of a node in
+ * configuration metadata-store.
+ *
+ * @throws WebApplicationException
+ * if broker has a read only access if broker is not connected to the configuration metadata-store
+ */
public void validatePoliciesReadOnlyAccess() {
try {
- if (namespaceResources().getPoliciesReadOnly()) {
- log.debug("Policies are read-only. Broker cannot do read-write operations");
- throw new RestException(Status.FORBIDDEN, "Broker is forbidden to do read-write operations");
+ validatePoliciesReadOnlyAccessAsync()
+ .get(config().getZooKeeperOperationTimeoutSeconds(), TimeUnit.SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
```suggestion
throw realCause;
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2746,55 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> asyncResponse.resume(policies.resource_group_name)))
Review comment:
```suggestion
.whenComplete((policies,ex) -> {
if (ex == null) {
log.info("[{}] Successfully to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
asyncResponse.resume(policies.resource_group_name)
return;
}
Throwable realCause = FutureUtil.unwrapCompletionException(ex);
log.error("[{}] Fail to get namespace resource group {}/{}: {}", clientAppId(), tenant, namespace, realCause);
resumeAsyncResponseExceptionally(asyncResponse, realCause);
return;
))
```
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
Review comment:
Remove `try...catch`
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -1782,24 +1784,48 @@ protected void internalSetInactiveTopic(InactiveTopicPolicies inactiveTopicPolic
internalSetPolicies("inactive_topic_policies", inactiveTopicPolicies);
}
+ protected CompletableFuture<Void> internalSetPoliciesAsync(String fieldName, Object value) {
+ return namespaceResources().getPoliciesAsync(namespaceName)
+ .thenCompose(policiesOptional -> {
+ Policies policies = policiesOptional.orElseThrow(() -> new RestException(Status.NOT_FOUND,
+ "Namespace policies does not exist"));
+ try {
+ Field field = Policies.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ field.set(policies, value);
+ return namespaceResources()
+ .setPoliciesAsync(namespaceName, p -> policies)
+ .thenAccept(__ -> {
+ try {
+ log.info("[{}] Successfully updated {} configuration: namespace={}, value={}",
+ clientAppId(), fieldName,
+ namespaceName, jsonMapper().writeValueAsString(value));
+ } catch (JsonProcessingException ex) {
+ log.error("[{}] Failed to serialize value while update {} configuration"
+ + " for namespace {}", clientAppId(), fieldName , namespaceName, ex);
+ throw new RestException(ex);
Review comment:
I think we should ignore this exception.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -1782,24 +1784,48 @@ protected void internalSetInactiveTopic(InactiveTopicPolicies inactiveTopicPolic
internalSetPolicies("inactive_topic_policies", inactiveTopicPolicies);
}
+ protected CompletableFuture<Void> internalSetPoliciesAsync(String fieldName, Object value) {
+ return namespaceResources().getPoliciesAsync(namespaceName)
+ .thenCompose(policiesOptional -> {
+ Policies policies = policiesOptional.orElseThrow(() -> new RestException(Status.NOT_FOUND,
+ "Namespace policies does not exist"));
+ try {
+ Field field = Policies.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ field.set(policies, value);
+ return namespaceResources()
+ .setPoliciesAsync(namespaceName, p -> policies)
+ .thenAccept(__ -> {
+ try {
+ log.info("[{}] Successfully updated {} configuration: namespace={}, value={}",
+ clientAppId(), fieldName,
+ namespaceName, jsonMapper().writeValueAsString(value));
+ } catch (JsonProcessingException ex) {
+ log.error("[{}] Failed to serialize value while update {} configuration"
+ + " for namespace {}", clientAppId(), fieldName , namespaceName, ex);
+ throw new RestException(ex);
+ }
+ });
+ } catch (Exception ex) {
+ log.error("[{}] Failed to reflect invoke field set while update {} configuration for "
+ + "namespace {}", clientAppId(), fieldName
+ , namespaceName, ex);
+ throw new RestException(ex);
+ }
+ });
+ }
+
protected void internalSetPolicies(String fieldName, Object value) {
try {
- Policies policies = namespaceResources().getPolicies(namespaceName)
- .orElseThrow(() -> new RestException(Status.NOT_FOUND,
- "Namespace policies does not exist"));
- Field field = Policies.class.getDeclaredField(fieldName);
- field.setAccessible(true);
- field.set(policies, value);
- namespaceResources().setPolicies(namespaceName, p -> policies);
- log.info("[{}] Successfully updated {} configuration: namespace={}, value={}", clientAppId(), fieldName,
- namespaceName, jsonMapper().writeValueAsString(value));
-
- } catch (RestException pfe) {
- throw pfe;
- } catch (Exception e) {
- log.error("[{}] Failed to update {} configuration for namespace {}", clientAppId(), fieldName
- , namespaceName, e);
- throw new RestException(e);
+ internalSetPoliciesAsync(fieldName, value)
+ .get(config().getZooKeeperOperationTimeoutSeconds(), TimeUnit.SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
```suggestion
throw realCause;
```
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2746,55 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> asyncResponse.resume(policies.resource_group_name)))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
+ asyncResponse.resume(new RestException(realCause));
+ }
+ return null;
+ });
}
-
- private static final Logger log = LoggerFactory.getLogger(NamespacesBase.class);
+ protected void internalSetNamespaceResourceGroup(AsyncResponse asyncResponse, String rgName) {
+ validateNamespacePolicyOperationAsync(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE)
+ .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
+ .thenCompose(__ -> {
+ if (rgName != null) {
+ return resourceGroupResources().getResourceGroupAsync(rgName)
+ .thenAccept(resourceGroup -> {
+ // check resource group exists.
+ if (!resourceGroup.isPresent()) {
+ throw new RestException(Status.PRECONDITION_FAILED,
+ "ResourceGroup does not exist");
+ }
+ });
+ } else {
+ return CompletableFuture.completedFuture(null);
+ }
+ }).thenCompose(__ -> internalSetPoliciesAsync("resource_group_name", rgName))
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully to set namespace resource group {}", clientAppId(), rgName);
+ asyncResponse.resume(Response.noContent().build());
+ }).exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
Review comment:
```suggestion
log.error("[{}] Fail to set namespace resource group {}: {}", clientAppId(), rgName, realCause);
resumeAsyncResponseExceptionally(asyncResponse, realCause);
return null;
```
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
Review comment:
Could you test the get/set/delete non-exists resource group?
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795011528
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().createNamespace(namespace);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isEmpty(namespaceResourceGroup));
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isEmpty(namespaceResourceGroup));
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ assertEquals(admin.namespaces().getNamespaceResourceGroup(namespace), resourceGroupName);
Review comment:
We should call this in the `Awaitility.await().untilAsserted()`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().createNamespace(namespace);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isEmpty(namespaceResourceGroup));
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isEmpty(namespaceResourceGroup));
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ assertEquals(admin.namespaces().getNamespaceResourceGroup(namespace), resourceGroupName);
Review comment:
We should check this in the `Awaitility.await().untilAsserted()`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010764
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
Review comment:
We should check this in the `Awaitility.await().untilAsserted()`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
We should check this in the `Awaitility.await().untilAsserted()`.
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795030573
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
Review comment:
fixed
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795112284
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1751,46 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ assertNotFound(__ -> admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName));
Review comment:
``expectThrows`` is more suitable for us.
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1025045642
@nodece 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] nodece commented on a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010889
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().createNamespace(namespace);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
We should check this in the `Awaitility.await().untilAsserted()`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009264
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
Review comment:
You should move this to line 2760, and log `realCause`. or miss the `realCause`?
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010084
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
Use `assertThrows` to check this.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010048
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
Use `assertThrows` to check this, the HTTP status code should be `404`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
Use `assertThrows` to check this, the HTTP status code should be `404`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
Review comment:
Use `assertThrows` to check this, the HTTP status code should be `404`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010622
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
We should call this in the `Awaitility.await().untilAsserted()`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
We should call this in the `Awaitility.await().untilAsserted()`.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
Review comment:
We should call this in the `Awaitility.await().untilAsserted()`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010048
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
Use `assertThrows` to check this, HTTP status code should be `404`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009689
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
Review comment:
A easy way: `public void testNonExistNamespaceResourceGroup() throws Exception {`
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794986028
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2746,55 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> asyncResponse.resume(policies.resource_group_name)))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
+ asyncResponse.resume(new RestException(realCause));
+ }
+ return null;
+ });
}
-
- private static final Logger log = LoggerFactory.getLogger(NamespacesBase.class);
+ protected void internalSetNamespaceResourceGroup(AsyncResponse asyncResponse, String rgName) {
+ validateNamespacePolicyOperationAsync(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE)
+ .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
+ .thenCompose(__ -> {
+ if (rgName != null) {
+ return resourceGroupResources().getResourceGroupAsync(rgName)
+ .thenAccept(resourceGroup -> {
+ // check resource group exists.
+ if (!resourceGroup.isPresent()) {
+ throw new RestException(Status.PRECONDITION_FAILED,
+ "ResourceGroup does not exist");
+ }
+ });
+ } else {
+ return CompletableFuture.completedFuture(null);
+ }
+ }).thenCompose(__ -> internalSetPoliciesAsync("resource_group_name", rgName))
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully to set namespace resource group {}", clientAppId(), rgName);
+ asyncResponse.resume(Response.noContent().build());
+ }).exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
Review comment:
I think the exception message is enough.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009264
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
Review comment:
You should move this to line 2760, and log `realCause`.
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1024846006
/pulsarbot rerun-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] mattisonchao commented on pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1022785513
/pulsarbot rerun-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] mattisonchao commented on a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794985610
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
##########
@@ -906,24 +907,42 @@ public void validateNamespaceOperation(NamespaceName namespaceName, NamespaceOpe
return CompletableFuture.completedFuture(null);
}
- public void validateNamespacePolicyOperation(NamespaceName namespaceName,
- PolicyName policy,
- PolicyOperation operation) {
+ public CompletableFuture<Void> validateNamespacePolicyOperationAsync(NamespaceName namespaceName,
+ PolicyName policy,
+ PolicyOperation operation) {
if (pulsar().getConfiguration().isAuthenticationEnabled()
- && pulsar().getBrokerService().isAuthorizationEnabled()) {
+ && pulsar().getBrokerService().isAuthorizationEnabled()) {
if (!isClientAuthenticated(clientAppId())) {
- throw new RestException(Status.FORBIDDEN, "Need to authenticate to perform the request");
+ return FutureUtil.failedFuture(
+ new RestException(Status.FORBIDDEN, "Need to authenticate to perform the request"));
}
+ return pulsar().getBrokerService().getAuthorizationService()
+ .allowNamespacePolicyOperationAsync(namespaceName, policy, operation,
+ originalPrincipal(), clientAppId(), clientAuthData())
+ .thenAccept(isAuthorized -> {
+ if (!isAuthorized) {
+ throw new RestException(Status.FORBIDDEN,
+ String.format("Unauthorized to validateNamespacePolicyOperation for"
+ + " operation [%s] on namespace [%s] on policy [%s]",
+ operation.toString(), namespaceName, policy.toString()));
+ }
+ });
+ }
+ return CompletableFuture.completedFuture(null);
+ }
- boolean isAuthorized = pulsar().getBrokerService().getAuthorizationService()
- .allowNamespacePolicyOperation(namespaceName, policy, operation,
- originalPrincipal(), clientAppId(), clientAuthData());
-
- if (!isAuthorized) {
- throw new RestException(Status.FORBIDDEN,
- String.format("Unauthorized to validateNamespacePolicyOperation for"
- + " operation [%s] on namespace [%s] on policy [%s]",
- operation.toString(), namespaceName, policy.toString()));
+ public void validateNamespacePolicyOperation(NamespaceName namespaceName,
+ PolicyName policy,
+ PolicyOperation operation) {
+ try {
+ validateNamespacePolicyOperationAsync(namespaceName, policy, operation)
+ .get(namespaceResources().getOperationTimeoutSec(), SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
This change will cause a compile error.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009404
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2745,58 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> {
+ log.info("[{}] Successfully to get namespace resource group {}/{}",
+ clientAppId(), tenant, namespace);
+ asyncResponse.resume(policies.resource_group_name);
+ }))
+ .exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to get namespace resource group {}/{}", clientAppId(), tenant, namespace);
+ asyncResponse.resume(new RestException(realCause));
+ }
+ return null;
+ });
}
-
- private static final Logger log = LoggerFactory.getLogger(NamespacesBase.class);
+ protected void internalSetNamespaceResourceGroup(AsyncResponse asyncResponse, String rgName) {
+ validateNamespacePolicyOperationAsync(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE)
+ .thenCompose(__ -> validatePoliciesReadOnlyAccessAsync())
+ .thenCompose(__ -> {
+ if (rgName != null) {
+ return resourceGroupResources().getResourceGroupAsync(rgName)
+ .thenAccept(resourceGroup -> {
+ // check resource group exists.
+ if (!resourceGroup.isPresent()) {
+ throw new RestException(Status.PRECONDITION_FAILED,
+ "ResourceGroup does not exist");
+ }
+ });
+ } else {
+ return CompletableFuture.completedFuture(null);
+ }
+ }).thenCompose(__ -> internalSetPoliciesAsync("resource_group_name", rgName))
+ .thenAccept(__ -> {
+ log.info("[{}] Successfully to set namespace resource group {}", clientAppId(), rgName);
+ asyncResponse.resume(Response.noContent().build());
+ }).exceptionally(ex -> {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ asyncResponse.resume(realCause);
+ } else {
+ log.error("[{}] Fail to set namespace resource group {} : {}", clientAppId(), rgName,
Review comment:
Miss the `realCause`?
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009802
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
Review comment:
Remove `try...catch`, use `public void testNonExistNamespaceResourceGroup() throws Exception {` instead.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795009640
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
Review comment:
A easy way: `public void testExistNamespaceResourceGroup() throws Exception {`
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794986132
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -1782,24 +1784,48 @@ protected void internalSetInactiveTopic(InactiveTopicPolicies inactiveTopicPolic
internalSetPolicies("inactive_topic_policies", inactiveTopicPolicies);
}
+ protected CompletableFuture<Void> internalSetPoliciesAsync(String fieldName, Object value) {
+ return namespaceResources().getPoliciesAsync(namespaceName)
+ .thenCompose(policiesOptional -> {
+ Policies policies = policiesOptional.orElseThrow(() -> new RestException(Status.NOT_FOUND,
+ "Namespace policies does not exist"));
+ try {
+ Field field = Policies.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ field.set(policies, value);
+ return namespaceResources()
+ .setPoliciesAsync(namespaceName, p -> policies)
+ .thenAccept(__ -> {
+ try {
+ log.info("[{}] Successfully updated {} configuration: namespace={}, value={}",
+ clientAppId(), fieldName,
+ namespaceName, jsonMapper().writeValueAsString(value));
+ } catch (JsonProcessingException ex) {
+ log.error("[{}] Failed to serialize value while update {} configuration"
+ + " for namespace {}", clientAppId(), fieldName , namespaceName, ex);
+ throw new RestException(ex);
Review comment:
Good catch!
Agree with you +1
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794990654
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
Review comment:
done.
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1024896740
/pulsarbot rerun-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] mattisonchao commented on a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795030933
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
Could you give me more information about why use it?
it's a sync method, I think we don't need to use ``Awaitility ``.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
Review comment:
Could you give me more information about why use it?
it's a sync method, I think we don't need to use ``Awaitility ``.
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794985758
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java
##########
@@ -972,15 +991,40 @@ public static ObjectMapper jsonMapper() {
return ObjectMapperFactory.getThreadLocal();
}
+ public CompletableFuture<Void> validatePoliciesReadOnlyAccessAsync() {
+ return pulsar().getPulsarResources()
+ .getNamespaceResources().getPoliciesReadOnlyAsync()
+ .thenAccept(arePoliciesReadOnly -> {
+ if (arePoliciesReadOnly) {
+ log.warn("Policies are read-only. Broker cannot do read-write operations");
+ throw new RestException(Status.FORBIDDEN, "Broker is forbidden to do read-write operations");
+ } else {
+ if (log.isDebugEnabled()) {
+ // Do nothing, just log the message.
+ log.debug("Broker is allowed to make read-write operations");
+ }
+ }
+ });
+ }
+
+ /**
+ * Checks whether the broker is allowed to do read-write operations based on the existence of a node in
+ * configuration metadata-store.
+ *
+ * @throws WebApplicationException
+ * if broker has a read only access if broker is not connected to the configuration metadata-store
+ */
public void validatePoliciesReadOnlyAccess() {
try {
- if (namespaceResources().getPoliciesReadOnly()) {
- log.debug("Policies are read-only. Broker cannot do read-write operations");
- throw new RestException(Status.FORBIDDEN, "Broker is forbidden to do read-write operations");
+ validatePoliciesReadOnlyAccessAsync()
+ .get(config().getZooKeeperOperationTimeoutSeconds(), TimeUnit.SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
This change will cause a compile error.
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1738,26 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
Review comment:
This change will cause a compile error.
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794985622
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -1782,24 +1784,48 @@ protected void internalSetInactiveTopic(InactiveTopicPolicies inactiveTopicPolic
internalSetPolicies("inactive_topic_policies", inactiveTopicPolicies);
}
+ protected CompletableFuture<Void> internalSetPoliciesAsync(String fieldName, Object value) {
+ return namespaceResources().getPoliciesAsync(namespaceName)
+ .thenCompose(policiesOptional -> {
+ Policies policies = policiesOptional.orElseThrow(() -> new RestException(Status.NOT_FOUND,
+ "Namespace policies does not exist"));
+ try {
+ Field field = Policies.class.getDeclaredField(fieldName);
+ field.setAccessible(true);
+ field.set(policies, value);
+ return namespaceResources()
+ .setPoliciesAsync(namespaceName, p -> policies)
+ .thenAccept(__ -> {
+ try {
+ log.info("[{}] Successfully updated {} configuration: namespace={}, value={}",
+ clientAppId(), fieldName,
+ namespaceName, jsonMapper().writeValueAsString(value));
+ } catch (JsonProcessingException ex) {
+ log.error("[{}] Failed to serialize value while update {} configuration"
+ + " for namespace {}", clientAppId(), fieldName , namespaceName, ex);
+ throw new RestException(ex);
+ }
+ });
+ } catch (Exception ex) {
+ log.error("[{}] Failed to reflect invoke field set while update {} configuration for "
+ + "namespace {}", clientAppId(), fieldName
+ , namespaceName, ex);
+ throw new RestException(ex);
+ }
+ });
+ }
+
protected void internalSetPolicies(String fieldName, Object value) {
try {
- Policies policies = namespaceResources().getPolicies(namespaceName)
- .orElseThrow(() -> new RestException(Status.NOT_FOUND,
- "Namespace policies does not exist"));
- Field field = Policies.class.getDeclaredField(fieldName);
- field.setAccessible(true);
- field.set(policies, value);
- namespaceResources().setPolicies(namespaceName, p -> policies);
- log.info("[{}] Successfully updated {} configuration: namespace={}, value={}", clientAppId(), fieldName,
- namespaceName, jsonMapper().writeValueAsString(value));
-
- } catch (RestException pfe) {
- throw pfe;
- } catch (Exception e) {
- log.error("[{}] Failed to update {} configuration for namespace {}", clientAppId(), fieldName
- , namespaceName, e);
- throw new RestException(e);
+ internalSetPoliciesAsync(fieldName, value)
+ .get(config().getZooKeeperOperationTimeoutSeconds(), TimeUnit.SECONDS);
+ } catch (InterruptedException | ExecutionException | TimeoutException ex) {
+ Throwable realCause = FutureUtil.unwrapCompletionException(ex);
+ if (realCause instanceof WebApplicationException) {
+ throw (WebApplicationException) realCause;
Review comment:
This change will cause a compile error.
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1024883508
@nodece 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] nodece commented on a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795055486
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1751,46 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() throws PulsarAdminException {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ assertNotFound(__ -> admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName));
Review comment:
```suggestion
assertEquals(assertThrows(PulsarAdminException.class,
() -> admin.namespaces().setNamespaceResourceGroup(namespace,
resourceGroupName)).getStatusCode(), Response.Status.NOT_FOUND.getStatusCode());
```
You can use this code.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795055627
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/utils/PulsarAdminAssert.java
##########
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pulsar.utils;
+
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import javax.ws.rs.NotFoundException;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+public class PulsarAdminAssert {
+
+ public static void assertNotFound(ThrowingConsumer<Void> consumer) {
+ try {
+ consumer.accept(null);
Review comment:
If you don't use the code of the above comment, you need to add `fail()` 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] mattisonchao closed pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010142
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
Review comment:
Use `assertThrows` to check this.
--
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 a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r794991202
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -2720,25 +2746,55 @@ private void updatePolicies(NamespaceName ns, Function<Policies, Policies> updat
}
}
- protected void internalSetNamespaceResourceGroup(String rgName) {
- validateNamespacePolicyOperation(namespaceName, PolicyName.RESOURCEGROUP, PolicyOperation.WRITE);
- validatePoliciesReadOnlyAccess();
-
- if (rgName != null) {
- // check resourcegroup exists.
- try {
- if (!resourceGroupResources().resourceGroupExists(rgName)) {
- throw new RestException(Status.PRECONDITION_FAILED, "ResourceGroup does not exist");
- }
- } catch (Exception e) {
- log.error("[{}] Invalid ResourceGroup {}: {}", clientAppId(), rgName, e);
- throw new RestException(e);
- }
- }
- internalSetPolicies("resource_group_name", rgName);
+ protected void internalGetNamespaceResourceGroup(AsyncResponse asyncResponse, String tenant, String namespace) {
+ validateNamespacePolicyOperationAsync(NamespaceName.get(tenant, namespace),
+ PolicyName.RESOURCEGROUP, PolicyOperation.READ)
+ .thenCompose(__ -> getNamespacePoliciesAsync(namespaceName)
+ .thenAccept(policies -> asyncResponse.resume(policies.resource_group_name)))
Review comment:
I think we don't need to add ``whenComplete `` to increase code complexity. So I add successful log info in ``thenAccept``.
--
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 #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#issuecomment-1024821011
@nodece
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] nodece commented on a change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010889
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
+ assertEquals(namespaceResourceGroup, resourceGroupName);
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ assertTrue(StringUtils.isBlank(admin.namespaces().getNamespaceResourceGroup(namespace)));
+ } catch (PulsarAdminException e) {
+ fail();
+ }
+ }
+
+ @Test
+ public void testNonExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ } catch (PulsarAdminException ex) {
+ fail();
+ }
+ try {
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().getNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().removeNamespaceResourceGroup(namespace);
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getCause() instanceof NotFoundException);
+ }
+ try {
+ admin.namespaces().createNamespace(namespace);
+ String namespaceResourceGroup = admin.namespaces().getNamespaceResourceGroup(namespace);
Review comment:
We should call this in the `Awaitility.await().untilAsserted()`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795010622
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
We should check this in the `Awaitility.await().untilAsserted()`.
--
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 change in pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970#discussion_r795053628
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/NamespacesTest.java
##########
@@ -1748,4 +1750,68 @@ private void assertInvalidRetentionPolicyAsPartOfAllPolicies(Policies policies,
assertTrue(e.getMessage().startsWith("Invalid retention policy"));
}
}
+
+ @Test
+ public void testExistNamespaceResourceGroup() {
+ String namespace = BrokerTestUtil.newUniqueName(this.testTenant + "/namespace");
+ ResourceGroup testResourceGroup = new ResourceGroup();
+ testResourceGroup.setDispatchRateInBytes(10000L);
+ testResourceGroup.setDispatchRateInMsgs(100);
+ testResourceGroup.setPublishRateInMsgs(100);
+ testResourceGroup.setPublishRateInBytes(10000L);
+ final String resourceGroupName = "test-resource-group";
+ try {
+ admin.resourcegroups().createResourceGroup(resourceGroupName, testResourceGroup);
+ admin.namespaces().createNamespace(namespace);
+ admin.namespaces().setNamespaceResourceGroup(namespace, resourceGroupName);
Review comment:
You are right, we don't need to use `Awaitility`here, the topic policies need to use this, because the topic policies are updated asynchronously.
--
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 closed pull request #13970: [Rest API] Make ``NamespaceResourceGroup`` get/set/delete to pure async.
Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #13970:
URL: https://github.com/apache/pulsar/pull/13970
--
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