smengcl commented on code in PR #11736:
URL: https://github.com/apache/ozone/pull/11736#discussion_r4385151430


##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java:
##########
@@ -1754,6 +1754,15 @@ Map<String, String> getObjectTagging(String volumeName, 
String bucketName, Strin
   void putObjectTagging(String volumeName, String bucketName, String keyName,
                         Map<String, String> tags) throws IOException;
 
+  /**
+   * Sets the tags to an existing S3 key, resolving its volume on OM.
+   * @param bucketName Bucket name.
+   * @param keyName Key name.
+   * @param tags Tags to set on the key.
+   * @throws IOException
+   */
+  void putS3ObjectTagging(String bucketName, String keyName, Map<String, 
String> tags) throws IOException;

Review Comment:
   fyi HDDS-17452 (#11757) was merged to master after this branch was cut and 
added `S3ClientProtocol`, a final class that implements `ClientProtocol` by 
delegating every method. It does not implement this new abstract 
`putS3ObjectTagging`, so the merge result will not compile (`ozone-client` 
fails), even though the merge itself is textually clean. Could you merge master 
and add the delegation there, similar to `abortS3MultipartUpload`?
   
   ```java
     @Override
     public void putS3ObjectTagging(String bucketName, String keyName, 
Map<String, String> tags) throws IOException {
       delegate.putS3ObjectTagging(bucketName, keyName, tags);
     }
   ```
   
   `TestS3ClientProtocol` checks that every `ClientProtocol` method is 
delegated, so it should cover the new method once it is added.



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

Reply via email to