virajjasani commented on code in PR #5554:
URL: https://github.com/apache/hadoop/pull/5554#discussion_r1167254526
##########
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:
Something like:
AddMountAttributes a = new AddMountAttributes();
a.setXyz();
a.setAbc();
a.getMountTableEntryWithUpdatedAttributes();
Here, `getMountTableEntryWithUpdatedAttributes` gives everything to router
admin and all that admin has to do is just use the mount table object and call
`mountTable.addMountTableEntry` or `mountTable.addMountTableEntries` directly.
@goiri this is what you meant right? If so, I think this is good idea, it
reduces all this logic in router admin and might be a good refactor.
--
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]
