goiri commented on code in PR #5195:
URL: https://github.com/apache/hadoop/pull/5195#discussion_r1045147361
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -467,310 +293,265 @@ public int run(String[] argv) throws Exception {
*
* @throws IOException if the operation was not successful.
*/
- private int refreshSuperUserGroupsConfiguration()
+ private int refreshSuperUserGroupsConfiguration(Configuration conf,
+ List<String> args)
throws IOException{
- RouterGenericManager proxy = client.getRouterGenericManager();
- String address = getConf().getTrimmed(
- RBFConfigKeys.DFS_ROUTER_ADMIN_ADDRESS_KEY,
- RBFConfigKeys.DFS_ROUTER_ADMIN_ADDRESS_DEFAULT);
- if(proxy.refreshSuperUserGroupsConfiguration()){
- System.out.println(
- "Successfully updated superuser proxy groups on router " + address);
- return 0;
+ StringUtils.ensureAllUsed(args);
+ try (RouterClient client = createClient(conf)) {
+ RouterGenericManager proxy = client.getRouterGenericManager();
+ String address = getAddress(conf);
+ if (proxy.refreshSuperUserGroupsConfiguration()) {
+ System.out.println(
+ "Successfully updated superuser proxy groups on router " +
address);
+ return 0;
+ }
}
return -1;
}
- private void refresh(String address) throws IOException {
- if (refreshRouterCache()) {
+ private int refresh(Configuration conf, List<String> args) throws
IOException {
+ StringUtils.ensureAllUsed(args);
+ if (refreshRouterCache(conf)) {
System.out.println(
- "Successfully updated mount table cache on router " + address);
+ "Successfully updated mount table cache on router " +
getAddress(conf));
+ return 0;
+ } else {
+ return -1;
}
}
/**
* Refresh mount table cache on connected router.
- *
+ * @param conf The configuration to use
* @return true if cache refreshed successfully
- * @throws IOException
*/
- private boolean refreshRouterCache() throws IOException {
- RefreshMountTableEntriesResponse response =
- client.getMountTableManager().refreshMountTableEntries(
- RefreshMountTableEntriesRequest.newInstance());
- return response.getResult();
+ private boolean refreshRouterCache(Configuration conf) throws IOException {
+ try (RouterClient client = createClient(conf)) {
+ RefreshMountTableEntriesResponse response =
+ client.getMountTableManager().refreshMountTableEntries(
+ RefreshMountTableEntriesRequest.newInstance());
+ return response.getResult();
+ }
}
-
/**
* Add a mount table entry or update if it exists.
- *
+ * @param conf Configuration
* @param parameters Parameters for the mount point.
- * @param i Index in the parameters.
- * @return If it was successful.
+ * @return 0 if it was successful.
* @throws IOException If it cannot add the mount point.
*/
- public boolean addMount(String[] parameters, int i) throws IOException {
- // Mandatory parameters
- String mount = parameters[i++];
- String[] nss = parameters[i++].split(",");
- String dest = parameters[i++];
-
- // Optional parameters
- boolean readOnly = false;
- boolean faultTolerant = false;
- String owner = null;
- String group = null;
- FsPermission mode = null;
- DestinationOrder order = DestinationOrder.HASH;
- while (i < parameters.length) {
- if (parameters[i].equals("-readonly")) {
- readOnly = true;
- } else if (parameters[i].equals("-faulttolerant")) {
- faultTolerant = true;
- } else if (parameters[i].equals("-order")) {
- i++;
- try {
- order = DestinationOrder.valueOf(parameters[i]);
- } catch(Exception e) {
- System.err.println("Cannot parse order: " + parameters[i]);
- }
- } else if (parameters[i].equals("-owner")) {
- i++;
- owner = parameters[i];
- } else if (parameters[i].equals("-group")) {
- i++;
- group = parameters[i];
- } else if (parameters[i].equals("-mode")) {
- i++;
- short modeValue = Short.parseShort(parameters[i], 8);
- mode = new FsPermission(modeValue);
- } else {
- printUsage("-add");
- return false;
+ public int addMount(Configuration conf,
+ List<String> parameters) throws IOException {
+ boolean readOnly = StringUtils.popOption("-readonly", parameters);
+ boolean faultTolerant = StringUtils.popOption("-faulttolerant",
parameters);
+ String owner = StringUtils.popOptionWithArgument("-owner", parameters);
+ String group = StringUtils.popOptionWithArgument("-group", parameters);
+ String modeStr = StringUtils.popOptionWithArgument("-mode", parameters);
+ DestinationOrder order = DestinationOrder.valueOf(
+ StringUtils.popOptionWithArgument("-order", parameters, "HASH"));
+ String mount = StringUtils.popFirstNonOption(parameters);
+ String nssString = StringUtils.popFirstNonOption(parameters);
+ String dest = StringUtils.popFirstNonOption(parameters);
+ if (mount == null || nssString == null || dest == null) {
+ throw new IllegalArgumentException("Required parameters not supplied.");
+ }
+ StringUtils.ensureAllUsed(parameters);
+ String[] nss = nssString.split(",");
+
+ FsPermission mode = modeStr == null ?
+ null : new FsPermission(Short.parseShort(modeStr, 8));
+
+ return addMount(conf, mount, nss, dest, readOnly, faultTolerant, order,
+ new ACLEntity(owner, group, mode), true);
+ }
+
+ /**
+ * Check for an optional boolean argument.
+ * @param name the name of the argument
+ * @param parameters the list of all of the parameters
+ * @return null if the argument wasn't present, true or false based on the
+ * string
+ */
+ static Boolean parseOptionalBoolean(String name, List<String> parameters) {
+ String val = StringUtils.popOptionWithArgument(name, parameters);
+ if (val == null) {
+ return null;
+ } else {
+ switch (val.toLowerCase()) {
Review Comment:
Isn't there a parseBool or something?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -467,310 +293,265 @@ public int run(String[] argv) throws Exception {
*
* @throws IOException if the operation was not successful.
*/
- private int refreshSuperUserGroupsConfiguration()
+ private int refreshSuperUserGroupsConfiguration(Configuration conf,
+ List<String> args)
throws IOException{
- RouterGenericManager proxy = client.getRouterGenericManager();
- String address = getConf().getTrimmed(
- RBFConfigKeys.DFS_ROUTER_ADMIN_ADDRESS_KEY,
- RBFConfigKeys.DFS_ROUTER_ADMIN_ADDRESS_DEFAULT);
- if(proxy.refreshSuperUserGroupsConfiguration()){
- System.out.println(
- "Successfully updated superuser proxy groups on router " + address);
- return 0;
+ StringUtils.ensureAllUsed(args);
+ try (RouterClient client = createClient(conf)) {
+ RouterGenericManager proxy = client.getRouterGenericManager();
+ String address = getAddress(conf);
+ if (proxy.refreshSuperUserGroupsConfiguration()) {
+ System.out.println(
+ "Successfully updated superuser proxy groups on router " +
address);
+ return 0;
+ }
}
return -1;
}
- private void refresh(String address) throws IOException {
- if (refreshRouterCache()) {
+ private int refresh(Configuration conf, List<String> args) throws
IOException {
+ StringUtils.ensureAllUsed(args);
+ if (refreshRouterCache(conf)) {
System.out.println(
- "Successfully updated mount table cache on router " + address);
+ "Successfully updated mount table cache on router " +
getAddress(conf));
+ return 0;
+ } else {
+ return -1;
}
}
/**
* Refresh mount table cache on connected router.
- *
+ * @param conf The configuration to use
* @return true if cache refreshed successfully
- * @throws IOException
*/
- private boolean refreshRouterCache() throws IOException {
- RefreshMountTableEntriesResponse response =
- client.getMountTableManager().refreshMountTableEntries(
- RefreshMountTableEntriesRequest.newInstance());
- return response.getResult();
+ private boolean refreshRouterCache(Configuration conf) throws IOException {
+ try (RouterClient client = createClient(conf)) {
+ RefreshMountTableEntriesResponse response =
+ client.getMountTableManager().refreshMountTableEntries(
+ RefreshMountTableEntriesRequest.newInstance());
+ return response.getResult();
+ }
}
-
/**
* Add a mount table entry or update if it exists.
- *
+ * @param conf Configuration
* @param parameters Parameters for the mount point.
- * @param i Index in the parameters.
- * @return If it was successful.
+ * @return 0 if it was successful.
* @throws IOException If it cannot add the mount point.
*/
- public boolean addMount(String[] parameters, int i) throws IOException {
- // Mandatory parameters
- String mount = parameters[i++];
- String[] nss = parameters[i++].split(",");
- String dest = parameters[i++];
-
- // Optional parameters
- boolean readOnly = false;
- boolean faultTolerant = false;
- String owner = null;
- String group = null;
- FsPermission mode = null;
- DestinationOrder order = DestinationOrder.HASH;
- while (i < parameters.length) {
- if (parameters[i].equals("-readonly")) {
- readOnly = true;
- } else if (parameters[i].equals("-faulttolerant")) {
- faultTolerant = true;
- } else if (parameters[i].equals("-order")) {
- i++;
- try {
- order = DestinationOrder.valueOf(parameters[i]);
- } catch(Exception e) {
- System.err.println("Cannot parse order: " + parameters[i]);
- }
- } else if (parameters[i].equals("-owner")) {
- i++;
- owner = parameters[i];
- } else if (parameters[i].equals("-group")) {
- i++;
- group = parameters[i];
- } else if (parameters[i].equals("-mode")) {
- i++;
- short modeValue = Short.parseShort(parameters[i], 8);
- mode = new FsPermission(modeValue);
- } else {
- printUsage("-add");
- return false;
+ public int addMount(Configuration conf,
+ List<String> parameters) throws IOException {
+ boolean readOnly = StringUtils.popOption("-readonly", parameters);
+ boolean faultTolerant = StringUtils.popOption("-faulttolerant",
parameters);
+ String owner = StringUtils.popOptionWithArgument("-owner", parameters);
+ String group = StringUtils.popOptionWithArgument("-group", parameters);
+ String modeStr = StringUtils.popOptionWithArgument("-mode", parameters);
+ DestinationOrder order = DestinationOrder.valueOf(
+ StringUtils.popOptionWithArgument("-order", parameters, "HASH"));
+ String mount = StringUtils.popFirstNonOption(parameters);
+ String nssString = StringUtils.popFirstNonOption(parameters);
+ String dest = StringUtils.popFirstNonOption(parameters);
+ if (mount == null || nssString == null || dest == null) {
+ throw new IllegalArgumentException("Required parameters not supplied.");
+ }
+ StringUtils.ensureAllUsed(parameters);
+ String[] nss = nssString.split(",");
+
+ FsPermission mode = modeStr == null ?
+ null : new FsPermission(Short.parseShort(modeStr, 8));
+
+ return addMount(conf, mount, nss, dest, readOnly, faultTolerant, order,
+ new ACLEntity(owner, group, mode), true);
+ }
+
+ /**
+ * Check for an optional boolean argument.
+ * @param name the name of the argument
+ * @param parameters the list of all of the parameters
+ * @return null if the argument wasn't present, true or false based on the
+ * string
+ */
+ static Boolean parseOptionalBoolean(String name, List<String> parameters) {
+ String val = StringUtils.popOptionWithArgument(name, parameters);
+ if (val == null) {
+ return null;
+ } else {
Review Comment:
As you return before, skip the else.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java:
##########
@@ -1182,6 +1200,19 @@ public static String popFirstNonOption(List<String>
args) {
}
return null;
}
+ /**
+ * From a list of command-line arguments, ensure that all of the arguments
+ * have been used except a possible "--".
+ *
+ * @param args List of arguments.
+ * @throws IllegalArgumentException if some arguments were not used
+ */
+ public static void ensureAllUsed(List<String> args) throws
IllegalArgumentException {
+ if (!args.isEmpty() && !(args.size() == 1 && "--".equals(args.get(0)))) {
Review Comment:
I don't think we need some of the parenthesis.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -439,24 +266,23 @@ public int run(String[] argv) throws Exception {
try {
String[] content;
content = e.getLocalizedMessage().split("\n");
- System.err.println(cmd.substring(1) + ": " + content[0]);
+ System.err.println(cmd+ ": " + content[0]);
Review Comment:
Space.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
