You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sarvekshayr (via GitHub)" <gi...@apache.org> on 2024/02/05 11:42:43 UTC
[PR] HDDS-10152. Field should be static [ozone]
sarvekshayr opened a new pull request, #6168:
URL: https://github.com/apache/ozone/pull/6168
## What changes were proposed in this pull request?
Changing the fields mentioned in the HDDS-10152.txt from final to static final. Renaming the variables and its subsequent appearance to ALL_CAPS format.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10152
## How was this patch tested?
Patch was tested by running the test class and the corresponding test class of server-side class to ensure that the changes made ran successfully without throwing an 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: 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
Re: [PR] HDDS-10152. Field should be static [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6168:
URL: https://github.com/apache/ozone/pull/6168#discussion_r1478071001
##########
hadoop-ozone/fault-injection-test/mini-chaos-tests/src/test/java/org/apache/hadoop/ozone/MiniOzoneChaosCluster.java:
##########
@@ -385,7 +385,7 @@ public Set<DatanodeDetails> dnToFail() {
}
@Override
- public void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode)
+ public void restartHddsDatanode(DatanodeDetails dn, boolean Datanode)
Review Comment:
This looks like unintentional change.
--
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
Re: [PR] HDDS-10152. Field should be static [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6168:
URL: https://github.com/apache/ozone/pull/6168#issuecomment-1930327386
Thanks @sarvekshayr for the patch, @tanvipenumudy for the review.
--
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
Re: [PR] HDDS-10152. Field should be static [ozone]
Posted by "sarvekshayr (via GitHub)" <gi...@apache.org>.
sarvekshayr commented on code in PR #6168:
URL: https://github.com/apache/ozone/pull/6168#discussion_r1479255573
##########
hadoop-ozone/fault-injection-test/mini-chaos-tests/src/test/java/org/apache/hadoop/ozone/MiniOzoneChaosCluster.java:
##########
@@ -385,7 +385,7 @@ public Set<DatanodeDetails> dnToFail() {
}
@Override
- public void restartHddsDatanode(DatanodeDetails dn, boolean waitForDatanode)
+ public void restartHddsDatanode(DatanodeDetails dn, boolean Datanode)
Review Comment:
Changes have been addressed and committed.
--
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
Re: [PR] HDDS-10152. Field should be static [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6168:
URL: https://github.com/apache/ozone/pull/6168#discussion_r1479541255
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -313,40 +313,40 @@ private static void doAs(UserGroupInformation ugi,
public void testListVolumeWithOtherUsersListAllAllowed() throws Exception {
setupEnvironment(true, true);
- // Login as user1, list other users' volumes
+ // Login as USER_1, list other users' volumes
Review Comment:
```suggestion
// Login as user1, list other users' volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -313,40 +313,40 @@ private static void doAs(UserGroupInformation ugi,
public void testListVolumeWithOtherUsersListAllAllowed() throws Exception {
setupEnvironment(true, true);
- // Login as user1, list other users' volumes
+ // Login as USER_1, list other users' volumes
doAs(userUGI1, () -> {
- checkUser(user2, Arrays.asList("volume2", "volume3", "volume4",
+ checkUser(USER_2, Arrays.asList("volume2", "volume3", "volume4",
"volume5"), true);
- checkUser(adminUser, Arrays
+ checkUser(ADMIN_USER, Arrays
.asList("volume1", "volume2", "volume3", "volume4", "volume5",
"volume6", "s3v"), true);
return true;
});
- // Login as user2, list other users' volumes
+ // Login as USER_2, list other users' volumes
Review Comment:
```suggestion
// Login as user2, list other users' volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -361,39 +361,39 @@ public void testListVolumeWithOtherUsersListAllAllowed() throws Exception {
public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception {
setupEnvironment(true, false);
- // Login as user1, list other users' volumes, expect failure
+ // Login as USER_1, list other users' volumes, expect failure
doAs(userUGI1, () -> {
- checkUser(user2, Arrays.asList("volume2", "volume3", "volume4",
+ checkUser(USER_2, Arrays.asList("volume2", "volume3", "volume4",
"volume5"), false);
- checkUser(adminUser, Arrays.asList("volume1", "volume2", "volume3",
+ checkUser(ADMIN_USER, Arrays.asList("volume1", "volume2", "volume3",
"volume4", "volume5", "volume6", "s3v"), false);
return true;
});
- // Login as user2, list other users' volumes, expect failure
+ // Login as USER_2, list other users' volumes, expect failure
Review Comment:
```suggestion
// Login as user2, list other users' volumes, expect failure
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -471,28 +471,28 @@ public void testAclEnabledListAllDisallowed() throws Exception {
public void testAclDisabledListAllAllowed() throws Exception {
setupEnvironment(false, true);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
doAs(userUGI1, () -> {
- checkUser(user1, Arrays.asList("volume1", "volume3", "volume5"),
+ checkUser(USER_1, Arrays.asList("volume1", "volume3", "volume5"),
true);
return true;
});
- // Login as user2, list their own volumes
+ // Login as USER_2, list their own volumes
Review Comment:
```suggestion
// Login as user2, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -142,18 +142,18 @@ private void createPrincipals() throws Exception {
String host = InetAddress.getLocalHost()
.getCanonicalHostName().toLowerCase();
String hostAndRealm = host + "@" + this.realm;
- this.adminPrincipal = adminUser + "/" + hostAndRealm;
- this.adminPrincipalInOtherHost = adminUser + "/otherhost@" + this.realm;
- this.adminKeytab = new File(workDir, adminUser + ".keytab");
- this.adminKeytabInOtherHost = new File(workDir, adminUser +
+ this.adminPrincipal = ADMIN_USER + "/" + hostAndRealm;
+ this.adminPrincipalInOtherHost = ADMIN_USER + "/otherhost@" + this.realm;
+ this.adminKeytab = new File(workDir, ADMIN_USER + ".keytab");
+ this.adminKeytabInOtherHost = new File(workDir, ADMIN_USER +
"InOtherHost.keytab");
createPrincipal(this.adminKeytab, adminPrincipal);
createPrincipal(this.adminKeytabInOtherHost, adminPrincipalInOtherHost);
- this.userPrincipal1 = this.user1 + "/" + hostAndRealm;
- this.userPrincipal2 = this.user2 + "/" + hostAndRealm;
- this.userKeytab1 = new File(workDir, this.user1 + ".keytab");
- this.userKeytab2 = new File(workDir, this.user2 + ".keytab");
+ this.userPrincipal1 = this.USER_1 + "/" + hostAndRealm;
+ this.userPrincipal2 = this.USER_2 + "/" + hostAndRealm;
+ this.userKeytab1 = new File(workDir, this.USER_1 + ".keytab");
+ this.userKeytab2 = new File(workDir, this.USER_2 + ".keytab");
Review Comment:
Better not access `static` via instance (`this`):
```suggestion
this.userPrincipal1 = USER_1 + "/" + hostAndRealm;
this.userPrincipal2 = USER_2 + "/" + hostAndRealm;
this.userKeytab1 = new File(workDir, USER_1 + ".keytab");
this.userKeytab2 = new File(workDir, USER_2 + ".keytab");
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -361,39 +361,39 @@ public void testListVolumeWithOtherUsersListAllAllowed() throws Exception {
public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception {
setupEnvironment(true, false);
- // Login as user1, list other users' volumes, expect failure
+ // Login as USER_1, list other users' volumes, expect failure
Review Comment:
```suggestion
// Login as user1, list other users' volumes, expect failure
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -502,28 +502,28 @@ public void testAclDisabledListAllAllowed() throws Exception {
public void testAclDisabledListAllDisallowed() throws Exception {
setupEnvironment(false, false);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
Review Comment:
```suggestion
// Login as user1, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -471,28 +471,28 @@ public void testAclEnabledListAllDisallowed() throws Exception {
public void testAclDisabledListAllAllowed() throws Exception {
setupEnvironment(false, true);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
Review Comment:
```suggestion
// Login as user1, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -502,28 +502,28 @@ public void testAclDisabledListAllAllowed() throws Exception {
public void testAclDisabledListAllDisallowed() throws Exception {
setupEnvironment(false, false);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
doAs(userUGI1, () -> {
- checkUser(user1, Arrays.asList("volume1", "volume3", "volume5"),
+ checkUser(USER_1, Arrays.asList("volume1", "volume3", "volume5"),
true);
return true;
});
- // Login as user2, list their own volumes
+ // Login as USER_2, list their own volumes
Review Comment:
```suggestion
// Login as user2, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -403,30 +403,30 @@ public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception {
public void testAclEnabledListAllAllowed() throws Exception {
setupEnvironment(true, true);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
doAs(userUGI1, () -> {
- checkUser(user1, Arrays.asList("volume1", "volume3", "volume4",
+ checkUser(USER_1, Arrays.asList("volume1", "volume3", "volume4",
"volume5"), true);
return true;
});
- // Login as user2, list their own volumes
+ // Login as USER_2, list their own volumes
Review Comment:
```suggestion
// Login as user2, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -403,30 +403,30 @@ public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception {
public void testAclEnabledListAllAllowed() throws Exception {
setupEnvironment(true, true);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
Review Comment:
```suggestion
// Login as user1, list their own volumes
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java:
##########
@@ -436,14 +436,14 @@ public void testAclEnabledListAllAllowed() throws Exception {
public void testAclEnabledListAllDisallowed() throws Exception {
setupEnvironment(true, false);
- // Login as user1, list their own volumes
+ // Login as USER_1, list their own volumes
Review Comment:
```suggestion
// Login as user1, list their own volumes
```
--
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
Re: [PR] HDDS-10152. Field should be static [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6168:
URL: https://github.com/apache/ozone/pull/6168
--
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