errose28 commented on code in PR #6574:
URL: https://github.com/apache/ozone/pull/6574#discussion_r1586963084
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/OzoneRepair.java:
##########
@@ -61,4 +66,27 @@ public OzoneConfiguration getOzoneConf() {
public static void main(String[] argv) throws Exception {
new OzoneRepair().run(argv);
}
+
+ @Override
+ public int execute(String[] argv) {
+ String currentUser = getSystemUserName();
+ String s = getConsoleReadLineWithFormat(currentUser);
+ boolean shouldProceed = Boolean.parseBoolean(s) || "y".equalsIgnoreCase(s);
Review Comment:
```suggestion
boolean shouldProceed = "y".equalsIgnoreCase(s);
```
I think this is sufficient. I'm not sure we need to support people typing
"true" into the prompt.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/OzoneRepair.java:
##########
@@ -61,4 +66,27 @@ public OzoneConfiguration getOzoneConf() {
public static void main(String[] argv) throws Exception {
new OzoneRepair().run(argv);
}
+
+ @Override
+ public int execute(String[] argv) {
+ String currentUser = getSystemUserName();
+ String s = getConsoleReadLineWithFormat(currentUser);
+ boolean shouldProceed = Boolean.parseBoolean(s) || "y".equalsIgnoreCase(s);
+ if (!shouldProceed) {
+ System.out.println("Aborting command.");
+ return 1;
+ }
+ System.out.println("Run as user: " + currentUser);
+
+ return super.execute(argv);
+ }
+
+ public String getSystemUserName() {
+ return System.getProperty("user.name");
+ }
+
+ public String getConsoleReadLineWithFormat(String currentUser) {
+ return System.console().readLine(String.format(WARNING_SYS_USER_MESSAGE,
currentUser));
Review Comment:
In the test we can then make sure that this stderr message also contains the
user name. Currently we are only checking the "run as user" message.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/OzoneRepair.java:
##########
@@ -61,4 +66,27 @@ public OzoneConfiguration getOzoneConf() {
public static void main(String[] argv) throws Exception {
new OzoneRepair().run(argv);
}
+
+ @Override
+ public int execute(String[] argv) {
+ String currentUser = getSystemUserName();
+ String s = getConsoleReadLineWithFormat(currentUser);
+ boolean shouldProceed = Boolean.parseBoolean(s) || "y".equalsIgnoreCase(s);
+ if (!shouldProceed) {
+ System.out.println("Aborting command.");
+ return 1;
+ }
+ System.out.println("Run as user: " + currentUser);
+
+ return super.execute(argv);
+ }
+
+ public String getSystemUserName() {
+ return System.getProperty("user.name");
+ }
+
+ public String getConsoleReadLineWithFormat(String currentUser) {
+ return System.console().readLine(String.format(WARNING_SYS_USER_MESSAGE,
currentUser));
Review Comment:
Also make sure the prompt goes to stderr. There may be repair commands were
the output is redirected for reference later, and we don't want the prompt to
get lost there.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/OzoneRepair.java:
##########
@@ -61,4 +66,27 @@ public OzoneConfiguration getOzoneConf() {
public static void main(String[] argv) throws Exception {
new OzoneRepair().run(argv);
}
+
+ @Override
+ public int execute(String[] argv) {
+ String currentUser = getSystemUserName();
+ String s = getConsoleReadLineWithFormat(currentUser);
+ boolean shouldProceed = Boolean.parseBoolean(s) || "y".equalsIgnoreCase(s);
+ if (!shouldProceed) {
+ System.out.println("Aborting command.");
+ return 1;
+ }
+ System.out.println("Run as user: " + currentUser);
+
+ return super.execute(argv);
+ }
+
+ public String getSystemUserName() {
+ return System.getProperty("user.name");
+ }
+
+ public String getConsoleReadLineWithFormat(String currentUser) {
+ return System.console().readLine(String.format(WARNING_SYS_USER_MESSAGE,
currentUser));
Review Comment:
Looks like other commands like
[DecomissionSubCommand](https://github.com/apache/ozone/blob/92bc617801e31c3ad8a75602e1167300c6936b33/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionSubCommand.java#L63)
are using `Scanner`. We can use that here too for consistency. `Scanner` also
supports redirecting stdin like
[this](https://github.com/apache/ozone/blob/92bc617801e31c3ad8a75602e1167300c6936b33/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDecommissionSubCommand.java#L79)
so no need for `Spy` in the test when using this approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
