You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "poorbarcode (via GitHub)" <gi...@apache.org> on 2023/02/01 09:26:17 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #19383: [fix] [admin] set offload threshold should fail when ns policies is read-only
poorbarcode opened a new pull request, #19383:
URL: https://github.com/apache/pulsar/pull/19383
### Motivation
https://github.com/apache/pulsar/pull/19370#discussion_r1092918655 Thanks @lhotari for helping to check
Admin API set offload threshold should fail when namespace policies are read-only.
### Modifications
solve the wrong logic and add test.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository:
- https://github.com/poorbarcode/pulsar/pull/61
--
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] lhotari merged pull request #19383: [fix] [admin] set offload threshold should fail if ns policies is read-only
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #19383:
URL: https://github.com/apache/pulsar/pull/19383
--
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] nicoloboschi commented on a diff in pull request #19383: [fix] [admin] set offload threshold should fail if ns policies is read-only
Posted by "nicoloboschi (via GitHub)" <gi...@apache.org>.
nicoloboschi commented on code in PR #19383:
URL: https://github.com/apache/pulsar/pull/19383#discussion_r1093252244
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -290,6 +293,21 @@ public void testSetNamespaceOffloadPolicies() throws Exception {
assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), policies);
}
+ @Test
+ public void testSetNamespaceOffloadPoliciesFailByReadOnly() throws Exception {
+ pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(),
+ Optional.empty()).join();
+ try {
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ fail("set offload threshold should fail when ns policies is readonly");
+ } catch (Exception ex){
+ // ignore.
+ }
+ // cleanup.
+ pulsar.getConfigurationMetadataStore().delete(NamespaceResources.POLICIES_READONLY_FLAG_PATH,
Review Comment:
this must be done in a `finally` block otherwise, If the test fails, the current state is not cleaned
--
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] poorbarcode commented on pull request #19383: [fix] [admin] set offload threshold should fail if ns policies is read-only
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on PR #19383:
URL: https://github.com/apache/pulsar/pull/19383#issuecomment-1412553051
/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] poorbarcode commented on a diff in pull request #19383: [fix] [admin] set offload threshold should fail if ns policies is read-only
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #19383:
URL: https://github.com/apache/pulsar/pull/19383#discussion_r1093442014
##########
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java:
##########
@@ -290,6 +293,21 @@ public void testSetNamespaceOffloadPolicies() throws Exception {
assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), policies);
}
+ @Test
+ public void testSetNamespaceOffloadPoliciesFailByReadOnly() throws Exception {
+ pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(),
+ Optional.empty()).join();
+ try {
+ admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300);
+ fail("set offload threshold should fail when ns policies is readonly");
+ } catch (Exception ex){
+ // ignore.
+ }
+ // cleanup.
+ pulsar.getConfigurationMetadataStore().delete(NamespaceResources.POLICIES_READONLY_FLAG_PATH,
Review Comment:
Already fixed. Thanks
--
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] lhotari commented on pull request #19383: [fix] [admin] set offload threshold should fail if ns policies is read-only
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #19383:
URL: https://github.com/apache/pulsar/pull/19383#issuecomment-1412322282
/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