sadanand48 commented on code in PR #8124:
URL: https://github.com/apache/ozone/pull/8124#discussion_r2048182609


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -1436,19 +1436,26 @@ private CancelSnapshotDiffResponse cancelSnapshotDiff(
   }
 
   private ListSnapshotDiffJobResponse listSnapshotDiffJobs(
-      ListSnapshotDiffJobRequest listSnapshotDiffJobRequest)
-      throws IOException {
-    List<SnapshotDiffJob> snapshotDiffJobs =
-        impl.listSnapshotDiffJobs(
-            listSnapshotDiffJobRequest.getVolumeName(),
-            listSnapshotDiffJobRequest.getBucketName(),
-            listSnapshotDiffJobRequest.getJobStatus(),
-            listSnapshotDiffJobRequest.getListAll());
-    ListSnapshotDiffJobResponse.Builder builder =
-        ListSnapshotDiffJobResponse.newBuilder();
-    for (SnapshotDiffJob diffJob : snapshotDiffJobs) {
+      ListSnapshotDiffJobRequest listSnapshotDiffJobRequest
+  ) throws IOException {
+    org.apache.hadoop.ozone.snapshot.ListSnapshotDiffJobResponse response = 
impl.listSnapshotDiffJobs(
+        listSnapshotDiffJobRequest.getVolumeName(),
+        listSnapshotDiffJobRequest.getBucketName(),
+        listSnapshotDiffJobRequest.getJobStatus(),
+        listSnapshotDiffJobRequest.getListAll(),
+        listSnapshotDiffJobRequest.getPrevSnapshotDiffJob(),
+        listSnapshotDiffJobRequest.getMaxListResult());

Review Comment:
   In the listKeys PR you shared the max will be based on default value so it 
will atleast list the default max (config) but here old client will list 0 
entries if the client is old .
   
   > but the client can avoid setting MaxListResult, which will result in 
unlimited listing
   
   This is how it is supposed to work right. If client does not pass the 
maxList arg it should print unlimited or if unlimited listing is a concern, at 
least limit it to the config (MAX_LIST_RESULT_DEFAULT)
   
   To state clearly , if maxList arg is not set by the client , it will list 0 
entries right now? Is this behaviour correct?



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