This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new b89b6e021d HDDS-12058. Use CommandLine out/err in GenericCli
subclasses (#7673)
b89b6e021d is described below
commit b89b6e021d90e6e1b433aead64ca4cb3261de1f5
Author: Doroszlai, Attila <[email protected]>
AuthorDate: Sat Jan 11 20:36:30 2025 +0100
HDDS-12058. Use CommandLine out/err in GenericCli subclasses (#7673)
---
.../org/apache/hadoop/hdds/cli/GenericCli.java | 13 +++-
.../scm/server/StorageContainerManagerStarter.java | 4 +-
.../org/apache/hadoop/ozone/conf/OzoneGetConf.java | 26 ++------
.../GenerateOzoneRequiredConfigurations.java | 44 +++----------
.../java/org/apache/hadoop/ozone/shell/Shell.java | 2 +-
.../ozone/shell/checknative/CheckNative.java | 8 +--
.../hadoop/ozone/conf/TestGetConfOptions.java | 76 +++++++++-------------
7 files changed, 61 insertions(+), 112 deletions(-)
diff --git
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java
index c698a9f3d5..3ec9048dfc 100644
---
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java
+++
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java
@@ -17,6 +17,7 @@
package org.apache.hadoop.hdds.cli;
import java.io.IOException;
+import java.io.PrintWriter;
import java.util.Map;
import com.google.common.base.Strings;
@@ -87,9 +88,9 @@ public abstract class GenericCli implements
GenericParentCommand {
//message could be null in case of NPE. This is unexpected so we can
//print out the stack trace.
if (verbose || Strings.isNullOrEmpty(error.getMessage())) {
- error.printStackTrace(System.err);
+ error.printStackTrace(cmd.getErr());
} else {
- System.err.println(error.getMessage().split("\n")[0]);
+ cmd.getErr().println(error.getMessage().split("\n")[0]);
}
}
@@ -114,4 +115,12 @@ public abstract class GenericCli implements
GenericParentCommand {
public boolean isVerbose() {
return verbose;
}
+
+ protected PrintWriter out() {
+ return cmd.getOut();
+ }
+
+ protected PrintWriter err() {
+ return cmd.getErr();
+ }
}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java
index 1eef7bce14..353c6c5010 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java
@@ -92,8 +92,8 @@ public class StorageContainerManagerStarter extends
GenericCli implements Callab
versionProvider = HddsVersionProvider.class)
public void generateClusterId() {
commonInit();
- System.out.println("Generating new cluster id:");
- System.out.println(receiver.generateClusterId());
+ out().println("Generating new cluster id:");
+ out().println(receiver.generateClusterId());
}
/**
diff --git
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/conf/OzoneGetConf.java
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/conf/OzoneGetConf.java
index bdb749d521..1ae3aa51de 100644
---
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/conf/OzoneGetConf.java
+++
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/conf/OzoneGetConf.java
@@ -17,8 +17,6 @@
package org.apache.hadoop.ozone.conf;
-import java.io.PrintStream;
-
import org.apache.hadoop.hdds.cli.GenericCli;
import org.apache.hadoop.hdds.cli.HddsVersionProvider;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
@@ -45,31 +43,17 @@ import picocli.CommandLine;
OzoneManagersCommandHandler.class
})
public class OzoneGetConf extends GenericCli {
- private final PrintStream out; // Stream for printing command output
- private final PrintStream err; // Stream for printing error
- private OzoneConfiguration conf;
-
- protected OzoneGetConf(OzoneConfiguration conf) {
- this(conf, System.out, System.err);
- }
-
- protected OzoneGetConf(OzoneConfiguration conf, PrintStream out,
- PrintStream err) {
- this.conf = conf;
- this.out = out;
- this.err = err;
- }
void printError(String message) {
- err.println(message);
+ err().println(message);
}
void printOut(String message) {
- out.println(message);
+ out().println(message);
}
OzoneConfiguration getConf() {
- return this.conf;
+ return getOzoneConf();
}
public static void main(String[] argv) {
@@ -79,8 +63,6 @@ public class OzoneGetConf extends GenericCli {
.addAppender(new ConsoleAppender(new PatternLayout("%m%n")));
Logger.getLogger(NativeCodeLoader.class).setLevel(Level.ERROR);
- OzoneConfiguration conf = new OzoneConfiguration();
- conf.addResource(new OzoneConfiguration());
- new OzoneGetConf(conf).run(argv);
+ new OzoneGetConf().run(argv);
}
}
diff --git
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genconf/GenerateOzoneRequiredConfigurations.java
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genconf/GenerateOzoneRequiredConfigurations.java
index c88b6b2d69..c29a90ec0d 100644
---
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genconf/GenerateOzoneRequiredConfigurations.java
+++
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genconf/GenerateOzoneRequiredConfigurations.java
@@ -28,7 +28,6 @@ import org.apache.hadoop.ozone.om.OMConfigKeys;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
import picocli.CommandLine.Parameters;
-import picocli.CommandLine.PicocliException;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
@@ -68,50 +67,25 @@ public final class GenerateOzoneRequiredConfigurations
extends GenericCli implem
"template, update Kerberos principal and keytab file before use.")
private boolean genSecurityConf;
- /**
- * Entry point for using genconf tool.
- *
- * @param args
- *
- */
public static void main(String[] args) throws Exception {
new GenerateOzoneRequiredConfigurations().run(args);
}
@Override
public Void call() throws Exception {
- generateConfigurations(path, genSecurityConf);
+ generateConfigurations();
return null;
}
- /**
- * Generate ozone-site.xml at specified path.
- * @param path
- * @throws PicocliException
- * @throws JAXBException
- */
- public static void generateConfigurations(String path) throws
- PicocliException, JAXBException, IOException {
- generateConfigurations(path, false);
- }
-
- /**
- * Generate ozone-site.xml at specified path.
- * @param path
- * @param genSecurityConf
- * @throws PicocliException
- * @throws JAXBException
- */
- public static void generateConfigurations(String path,
- boolean genSecurityConf) throws
- PicocliException, JAXBException, IOException {
+ private void generateConfigurations() throws
+ JAXBException, IOException {
if (!isValidPath(path)) {
- throw new PicocliException("Invalid directory path.");
+ throw new IllegalArgumentException("Invalid directory path.");
}
if (!canWrite(path)) {
- throw new PicocliException("Insufficient permission.");
+ throw new IllegalArgumentException("Insufficient permission.");
}
OzoneConfiguration oc = new OzoneConfiguration();
@@ -171,9 +145,9 @@ public final class GenerateOzoneRequiredConfigurations
extends GenericCli implem
m.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE);
m.marshal(generatedConfig, output);
- System.out.println("ozone-site.xml has been generated at " + path);
+ out().println("ozone-site.xml has been generated at " + path);
} else {
- System.out.printf("ozone-site.xml already exists at %s and " +
+ out().printf("ozone-site.xml already exists at %s and " +
"will not be overwritten%n", path);
}
@@ -182,21 +156,19 @@ public final class GenerateOzoneRequiredConfigurations
extends GenericCli implem
/**
* Check if the path is valid directory.
*
- * @param path
* @return true, if path is valid directory, else return false
*/
public static boolean isValidPath(String path) {
try {
return Files.isDirectory(Paths.get(path));
} catch (InvalidPathException | NullPointerException ex) {
- return Boolean.FALSE;
+ return false;
}
}
/**
* Check if user has permission to write in the specified path.
*
- * @param path
* @return true, if the user has permission to write, else returns false
*/
public static boolean canWrite(String path) {
diff --git
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Shell.java
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Shell.java
index 8bca492f04..515dcec179 100644
--- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Shell.java
+++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Shell.java
@@ -109,7 +109,7 @@ public abstract class Shell extends GenericCli {
if (omException != null && !isVerbose()) {
// In non-verbose mode, reformat OMExceptions as error messages to the
// user.
- System.err.println(String.format("%s %s", omException.getResult().name(),
+ err().println(String.format("%s %s", omException.getResult().name(),
omException.getMessage()));
} else {
// Prints the stack trace when in verbose mode.
diff --git
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/checknative/CheckNative.java
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/checknative/CheckNative.java
index b6b5cc989b..1298811fa4 100644
---
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/checknative/CheckNative.java
+++
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/checknative/CheckNative.java
@@ -57,10 +57,10 @@ public class CheckNative extends GenericCli implements
Callable<Void> {
isalLoaded = true;
}
}
- System.out.println("Native library checking:");
- System.out.printf("hadoop: %b %s%n", nativeHadoopLoaded,
+ out().println("Native library checking:");
+ out().printf("hadoop: %b %s%n", nativeHadoopLoaded,
hadoopLibraryName);
- System.out.printf("ISA-L: %b %s%n", isalLoaded, isalDetail);
+ out().printf("ISA-L: %b %s%n", isalLoaded, isalDetail);
// Attempt to load the rocks-tools lib
boolean nativeRocksToolsLoaded =
NativeLibraryLoader.getInstance().loadLibrary(
@@ -70,7 +70,7 @@ public class CheckNative extends GenericCli implements
Callable<Void> {
if (nativeRocksToolsLoaded) {
rocksToolsDetail = NativeLibraryLoader.getJniLibraryFileName();
}
- System.out.printf("rocks-tools: %b %s%n", nativeRocksToolsLoaded,
rocksToolsDetail);
+ out().printf("rocks-tools: %b %s%n", nativeRocksToolsLoaded,
rocksToolsDetail);
return null;
}
}
diff --git
a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/conf/TestGetConfOptions.java
b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/conf/TestGetConfOptions.java
index 4f7233f148..ed16434f90 100644
---
a/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/conf/TestGetConfOptions.java
+++
b/hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/conf/TestGetConfOptions.java
@@ -17,82 +17,68 @@
*/
package org.apache.hadoop.ozone.conf;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.ozone.om.OMConfigKeys;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.apache.ozone.test.GenericTestUtils;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
-import java.io.UnsupportedEncodingException;
-
-import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* Tests the ozone getconf command.
*/
public class TestGetConfOptions {
- private static OzoneConfiguration conf;
- private static ByteArrayOutputStream bout;
- private static PrintStream psBackup;
- private static final String DEFAULT_ENCODING = UTF_8.name();
+ private static GenericTestUtils.PrintStreamCapturer out;
+ private static OzoneGetConf subject;
@BeforeAll
- public static void init() throws UnsupportedEncodingException {
- conf = new OzoneConfiguration();
- conf.set(OMConfigKeys.OZONE_OM_NODE_ID_KEY, "1");
- conf.set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "service1");
- conf.set(ScmConfigKeys.OZONE_SCM_NAMES, "localhost");
- psBackup = System.out;
- bout = new ByteArrayOutputStream();
- PrintStream psOut = new PrintStream(bout, false, DEFAULT_ENCODING);
- System.setOut(psOut);
+ public static void init() {
+ out = GenericTestUtils.captureOut();
+ subject = new OzoneGetConf();
+ subject.getConf().set(OMConfigKeys.OZONE_OM_NODE_ID_KEY, "1");
+ subject.getConf().set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "service1");
+ subject.getConf().set(ScmConfigKeys.OZONE_SCM_NAMES, "localhost");
}
@AfterEach
public void setUp() {
- bout.reset();
+ out.reset();
}
@AfterAll
public static void tearDown() {
- System.setOut(psBackup);
+ IOUtils.closeQuietly(out);
}
@Test
- public void testGetConfWithTheOptionConfKey()
- throws UnsupportedEncodingException {
- new OzoneGetConf(conf)
- .run(new String[] {"-confKey", ScmConfigKeys.OZONE_SCM_NAMES});
- assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
- bout.reset();
- new OzoneGetConf(conf)
- .run(new String[] {"confKey", OMConfigKeys.OZONE_OM_NODE_ID_KEY});
- assertEquals("1\n", bout.toString(DEFAULT_ENCODING));
+ public void testGetConfWithTheOptionConfKey() {
+ subject.run(new String[] {"-confKey", ScmConfigKeys.OZONE_SCM_NAMES});
+ assertEquals("localhost\n", out.get());
+ out.reset();
+ subject.run(new String[] {"confKey", OMConfigKeys.OZONE_OM_NODE_ID_KEY});
+ assertEquals("1\n", out.get());
}
@Test
- public void testGetConfWithTheOptionStorageContainerManagers()
- throws UnsupportedEncodingException {
- new OzoneGetConf(conf).run(new String[] {"-storagecontainermanagers"});
- assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
- bout.reset();
- new OzoneGetConf(conf).run(new String[] {"storagecontainermanagers"});
- assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
+ public void testGetConfWithTheOptionStorageContainerManagers() {
+ subject.execute(new String[] {"-storagecontainermanagers"});
+ assertEquals("localhost\n", out.get());
+ out.reset();
+ subject.execute(new String[] {"storagecontainermanagers"});
+ assertEquals("localhost\n", out.get());
}
@Test
- public void testGetConfWithTheOptionOzoneManagers()
- throws UnsupportedEncodingException {
- new OzoneGetConf(conf).run(new String[] {"-ozonemanagers"});
- assertEquals("", bout.toString(DEFAULT_ENCODING));
- bout.reset();
- new OzoneGetConf(conf).run(new String[] {"ozonemanagers"});
- assertEquals("", bout.toString(DEFAULT_ENCODING));
+ public void testGetConfWithTheOptionOzoneManagers() {
+ subject.execute(new String[] {"-ozonemanagers"});
+ assertEquals("", out.get());
+ out.reset();
+ subject.execute(new String[] {"ozonemanagers"});
+ assertEquals("", out.get());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]