goiri commented on code in PR #5554:
URL: https://github.com/apache/hadoop/pull/5554#discussion_r1167306643
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java:
##########
@@ -511,90 +687,45 @@ private boolean refreshRouterCache() throws IOException {
* @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;
- }
-
- i++;
+ AddMountAttributes addMountAttributes = getAddMountAttributes(parameters,
i, false);
+ if (addMountAttributes == null) {
+ return false;
}
-
- return addMount(mount, nss, dest, readOnly, faultTolerant, order,
- new ACLEntity(owner, group, mode));
+ return addMount(addMountAttributes);
}
/**
* Add a mount table entry or update if it exists.
*
- * @param mount Mount point.
- * @param nss Namespaces where this is mounted to.
- * @param dest Destination path.
- * @param readonly If the mount point is read only.
- * @param order Order of the destination locations.
- * @param aclInfo the ACL info for mount point.
+ * @param addMountAttributes attributes associated with add mount point
request.
* @return If the mount point was added.
* @throws IOException Error adding the mount point.
*/
- public boolean addMount(String mount, String[] nss, String dest,
- boolean readonly, boolean faultTolerant, DestinationOrder order,
- ACLEntity aclInfo)
+ public boolean addMount(AddMountAttributes addMountAttributes)
throws IOException {
- mount = normalizeFileSystemPath(mount);
+ String mount = normalizeFileSystemPath(addMountAttributes.getMount());
// Get the existing entry
MountTableManager mountTable = client.getMountTableManager();
MountTable existingEntry = getMountEntry(mount, mountTable);
if (existingEntry == null) {
// Create and add the entry if it doesn't exist
Map<String, String> destMap = new LinkedHashMap<>();
- for (String ns : nss) {
- destMap.put(ns, dest);
+ for (String ns : addMountAttributes.getNss()) {
+ destMap.put(ns, addMountAttributes.getDest());
}
MountTable newEntry = MountTable.newInstance(mount, destMap);
- if (readonly) {
+ if (addMountAttributes.isReadonly()) {
newEntry.setReadOnly(true);
Review Comment:
Yes, something like that.
It would isolate the building of the data structure to its definition.
--
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]
