[GitHub] storm pull request #2739: [STORM-3125/3126/3127]: Refactoring components for...

[revans2 <gi...@git.apache.org>]

Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2739#discussion_r198899945
  
    --- Diff: storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedBlob.java ---
    @@ -59,57 +59,47 @@ protected LocallyCachedBlob(String blobDescription, String blobKey) {
             this.blobKey = blobKey;
         }
     
    -    protected static long downloadToTempLocation(ClientBlobStore store, String key, long currentVersion, IAdvancedFSOps fsOps,
    -                                                 Function<Long, Path> getTempPath)
    -        throws KeyNotFoundException, AuthorizationException, IOException {
    +    /**
    +     * Helper function to download blob from blob store.
    +     * @param store Blob store to fetch blobs from
    +     * @param key Key to retrieve blobs
    +     * @param pathSupplier A function that supplies the download destination of a blob. It guarantees the validity
    +     *                     of path or throws {@link IOException}
    +     * @param outStreamSupplier A function that supplies the {@link OutputStream} object
    +     * @return The metadata of the download session, including blob's version and download destination
    +     * @throws KeyNotFoundException Thrown if key to retrieve blob is invalid
    +     * @throws AuthorizationException Thrown if the retrieval is not under security authorization
    +     * @throws IOException Thrown if any IO error occurs
    +     */
    +    protected DownloadMeta fetch(ClientBlobStore store, String key,
    +                                 IOFunction<Long, Path> pathSupplier,
    +                                 IOFunction<File, OutputStream> outStreamSupplier)
    +            throws KeyNotFoundException, AuthorizationException, IOException {
    +
             try (InputStreamWithMeta in = store.getBlob(key)) {
                 long newVersion = in.getVersion();
    -            if (newVersion == currentVersion) {
    -                LOG.warn("The version did not change, but going to download again {} {}", currentVersion, key);
    +            if (newVersion == getLocalVersion()) {
    --- End diff --
    
    The code change is fine, I just don't want to call the API twice if we don't have to.


---