yihua commented on code in PR #13836:
URL: https://github.com/apache/hudi/pull/13836#discussion_r2323726932


##########
hudi-aws/src/main/java/org/apache/hudi/aws/transaction/lock/S3StorageLockClient.java:
##########
@@ -267,6 +268,52 @@ private static S3Client createS3Client(Region region, long 
timeoutSecs, Properti
             .region(region).build();
   }
 
+  @Override
+  public Option<String> readObject(String filePath, boolean checkExistsFirst) {
+    try {
+      // Parse the file path to get bucket and key
+      URI uri = new URI(filePath);
+      String bucket = uri.getHost();
+      String key = uri.getPath().replaceFirst("/", "");

Review Comment:
   nit: extract the this logic to a method so that the constructor can also 
reuse it?
   ```
   S3StorageLockClient(String ownerId, String lockFileUri, Properties props, 
Functions.Function2<String, Properties, S3Client> s3ClientSupplier, Logger 
logger) {
       try {
         // This logic can likely be extended to other lock client 
implementations.
         // Consider creating base class with utilities, incl error handling.
         URI uri = new URI(lockFileUri);
         this.bucketName = uri.getHost();
         this.lockFilePath = uri.getPath().replaceFirst("/", "");
   ```



-- 
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]

Reply via email to