You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2023/05/21 10:35:53 UTC
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4737: HDDS-8654. Support dynamic changes ozone.readonly.administrators(OM)
adoroszlai commented on code in PR #4737:
URL: https://github.com/apache/ozone/pull/4737#discussion_r1199739503
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfigure.java:
##########
@@ -97,4 +98,23 @@ public void testOmAdminUsersReconfigure() throws Exception {
ozoneManager.getOmAdminUsernames().contains(userB));
}
+ /**
+ * Test reconfigure om "ozone.readonly.administrators"
+ */
+ @Test
+ public void testOmReadOnlyUSersReconfigure() throws Exception {
+ String userA = "mockUserA";
+ String userB = "mockUserB";
+ conf.set(OZONE_READONLY_ADMINISTRATORS, userA);
+ ozoneManager.reconfigurePropertyImpl(OZONE_READONLY_ADMINISTRATORS, userA);
+ assertTrue(userA + " should be an readOnly admin user",
Review Comment:
```suggestion
assertTrue(userA + " should be a readOnly admin user",
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -4067,6 +4081,10 @@ public Collection<String> getOmAdminUsernames() {
return omAdmins.getAdminUsernames();
}
+ public Collection<String> getOmReadOnlyUsernames() {
Review Comment:
For consistency:
```suggestion
public Collection<String> getOmReadOnlyAdminUsernames() {
```
(apply change to callers, too)
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfigure.java:
##########
@@ -97,4 +98,23 @@ public void testOmAdminUsersReconfigure() throws Exception {
ozoneManager.getOmAdminUsernames().contains(userB));
}
+ /**
+ * Test reconfigure om "ozone.readonly.administrators"
+ */
+ @Test
+ public void testOmReadOnlyUSersReconfigure() throws Exception {
Review Comment:
Typo:
```suggestion
public void testOmReadOnlyUsersReconfigure() throws Exception {
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfigure.java:
##########
@@ -97,4 +98,23 @@ public void testOmAdminUsersReconfigure() throws Exception {
ozoneManager.getOmAdminUsernames().contains(userB));
}
+ /**
+ * Test reconfigure om "ozone.readonly.administrators"
+ */
+ @Test
+ public void testOmReadOnlyUSersReconfigure() throws Exception {
+ String userA = "mockUserA";
+ String userB = "mockUserB";
+ conf.set(OZONE_READONLY_ADMINISTRATORS, userA);
+ ozoneManager.reconfigurePropertyImpl(OZONE_READONLY_ADMINISTRATORS, userA);
+ assertTrue(userA + " should be an readOnly admin user",
+ ozoneManager.getOmReadOnlyUsernames().contains(userA));
+
+ conf.set(OZONE_READONLY_ADMINISTRATORS, userB);
+ ozoneManager.reconfigurePropertyImpl(OZONE_READONLY_ADMINISTRATORS, userB);
+ assertFalse(userA + " should NOT be an admin user",
+ ozoneManager.getOmReadOnlyUsernames().contains(userA));
+ assertTrue(userB + " should be an admin user",
Review Comment:
```suggestion
assertFalse(userA + " should NOT be a readOnly admin user",
ozoneManager.getOmReadOnlyUsernames().contains(userA));
assertTrue(userB + " should be a readOnly admin user",
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfigure.java:
##########
@@ -97,4 +98,23 @@ public void testOmAdminUsersReconfigure() throws Exception {
ozoneManager.getOmAdminUsernames().contains(userB));
}
+ /**
+ * Test reconfigure om "ozone.readonly.administrators"
Review Comment:
Checkstyle complains about missing period:
```
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/reconfig/TestOmReconfigure.java
101: First sentence should end with a period.
```
```suggestion
* Test reconfigure om "ozone.readonly.administrators".
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org