bharatviswa504 commented on a change in pull request #1150:
URL: https://github.com/apache/hadoop-ozone/pull/1150#discussion_r462747061
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here.
message RenameKeysRequest {
optional RenameKeyArgs deleteKeys = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
I am not completely sure what is the benefit to be consistent with
RenameKeyRequest message structure.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here some thing like below.
message RenameKeysRequest {
optional RenameKeyArgs deleteKeys = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated RenameKey renameKeys = 3;
}
message RenameKey {
required string fromKeyName;
required string toKeyName;
}
I am not completely sure what is the benefit to be consistent with
RenameKeyRequest message structure.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
I agree with @adoroszlai here, we don't need KeyArgs.
HDDS-3930 created a new message DeleteKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated string keys = 3;
}
we can come up with a new proto structure here some thing like below.
message RenameKeysRequest {
optional RenameKeyArgs renameKeyArgs = 1;
}
message RenameKeyArgs {
required string volumeName = 1;
required string bucketName = 2;
repeated RenameKey renameKeys = 3;
}
message RenameKey {
required string fromKeyName;
required string toKeyName;
}
I am not completely sure what is the benefit to be consistent with
RenameKeyRequest message structure.
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -750,6 +751,25 @@ public void renameKey(String volumeName, String bucketName,
ozoneManagerClient.renameKey(keyArgs, toKeyName);
}
+ @Override
+ public void renameKeys(String volumeName, String bucketName,
+ Map<String, String> keyMap) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyMap);
+ Map <OmKeyArgs, String> keyArgsMap = new HashMap<>();
+ for (Map.Entry< String, String > entry : keyMap.entrySet()) {
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(volumeName)
+ .setBucketName(bucketName)
+ .setKeyName(entry.getKey())
+ .build();
+ keyArgsMap.put(keyArgs, entry.getValue());
+ }
Review comment:
sorry missed this, it has been already addressed. But I see we pass
volumeName/bucketName with each Renamekey, we can set them only once, as right
now rename is supported within a single volume/bucket
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
