steveloughran commented on a change in pull request #2257:
URL: https://github.com/apache/hadoop/pull/2257#discussion_r484868210



##########
File path: 
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3Guard.java
##########
@@ -362,14 +340,16 @@ public static BulkOperationState initiateBulkWrite(
    * @return Final result of directory listing.
    * @throws IOException if metadata store update failed
    */
-  public static S3AFileStatus[] dirListingUnion(MetadataStore ms, Path path,
-      List<S3AFileStatus> backingStatuses, DirListingMetadata dirMeta,
-      boolean isAuthoritative, ITtlTimeProvider timeProvider)
-      throws IOException {
+  public static RemoteIterator<S3AFileStatus> dirListingUnion(
+          MetadataStore ms, Path path,
+          RemoteIterator<S3AFileStatus> backingStatuses,
+          DirListingMetadata dirMeta, boolean isAuthoritative,
+          ITtlTimeProvider timeProvider, Listing listing)

Review comment:
       we are into callback country here. Could you 
   
   1. create an interface (say in S3Guard) for 
createProvidedFileStatusIteratorForS3Guard  (it could contain all params for 
`createProvidedFileStatusIterator` oterh than the dir meta list, 2. Have 
Listing implement it
   3. Pass the interface in, not the reference to Listing. 
   
   I'm just trying to move away from having all the classes in here having 
intimate knowledge of every other class. An interface with a single method lets 
us control exactly what S3Guard does. 
   
   You could use a Lambda-expression if you want -for a single operation it 
would work, just take `Function<S3AFileStatus[], RemoteIterator<S3AFileStatus>` 
as a parameter and pass the operation in that way. But, given you use this 
operation in tests as well as production code, you should still implement this 
function as a single method in Listing




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to