sunithabeeram commented on a change in pull request #3834: Unify move method in PinotFS URL: https://github.com/apache/incubator-pinot/pull/3834#discussion_r260425184
########## File path: pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java ########## @@ -22,14 +22,19 @@ import java.io.File; import java.io.IOException; import java.net.URI; +import java.nio.file.Paths; import org.apache.commons.configuration.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The PinotFS is intended to be a thin wrapper on top of different filesystems. This interface is intended for internal * Pinot use only. This class will be implemented for each pluggable storage type. */ Review comment: Lets add notes on some of the core discussions we have around this so its clear to other why the interface is the way it is: "PinotFS is a restricted FS API that exposes functionality that is required for Pinot to use different FS implementations. The restrictions are in place due to 2 driving factors: - Prevent unexpected performance hit when a broader API is implemented - especially, we would like to reduce calls to remote filesystems that might be needed for a broader API, but not necessarily required by Pinot (see the documentation for move() method below). - Provide an interface that is simple to be implemented across different FS types. The contract that developers have to adhere to will be simpler. Please read the method level docs carefully to note the exceptions while using the APIs." ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
