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