[ 
https://issues.apache.org/jira/browse/FLINK-9599?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16519090#comment-16519090
 ] 

ASF GitHub Bot commented on FLINK-9599:
---------------------------------------

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

    https://github.com/apache/flink/pull/6178#discussion_r197055449
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/FileUploads.java
 ---
    @@ -0,0 +1,107 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.rest.handler;
    +
    +import org.apache.flink.util.FileUtils;
    +import org.apache.flink.util.Preconditions;
    +
    +import java.io.FileNotFoundException;
    +import java.io.IOException;
    +import java.nio.file.FileVisitResult;
    +import java.nio.file.Files;
    +import java.nio.file.Path;
    +import java.nio.file.SimpleFileVisitor;
    +import java.nio.file.attribute.BasicFileAttributes;
    +import java.util.ArrayList;
    +import java.util.Collection;
    +import java.util.Collections;
    +
    +/**
    + * A container for uploaded files.
    + *
    + * <p>Implementation note: The constructor also accepts directories to 
ensure that the upload directories are cleaned up.
    + * For convenience during testing it also accepts files directly.
    + */
    +public final class FileUploads implements AutoCloseable {
    +   private final Collection<Path> directoriesToClean;
    +   private final Collection<Path> uploadedFiles;
    +
    +   @SuppressWarnings("resource")
    +   public static final FileUploads EMPTY = new FileUploads();
    +
    +   public static FileUploads forDirectory(Path directory) throws 
IOException {
    +           final Collection<Path> files = new ArrayList<>(4);
    +           Preconditions.checkArgument(directory.isAbsolute(), "Path must 
be absolute.");
    +           Preconditions.checkArgument(Files.isDirectory(directory), "Path 
must be a directory.");
    +
    +           FileAdderVisitor visitor = new FileAdderVisitor();
    +           Files.walkFileTree(directory, visitor);
    +           files.addAll(visitor.getContainedFiles());
    +           
    +           return new FileUploads(Collections.singleton(directory), files);
    +   }
    +
    +   private FileUploads() {
    +           this.directoriesToClean = Collections.emptyList();
    +           this.uploadedFiles = Collections.emptyList();
    +   }
    +
    +   public FileUploads(Collection<Path> directoriesToClean, 
Collection<Path> uploadedFiles) {
    +           this.directoriesToClean = 
Preconditions.checkNotNull(directoriesToClean);
    +           this.uploadedFiles = Preconditions.checkNotNull(uploadedFiles);
    --- End diff --
    
    I know it's really nitpicking what I'm doing here, but I think it would be 
slightly better to let FileUploads only represent the upload directories and 
add a method FileUploads#getFiles which returns a Collection<File> which are 
all files being found in the upload directory. The difference is that we don't 
initialize FileUploads with it. That would effectively enforce that all files 
reside in the given upload directories. What we could do now is to initialize 
this class with directories /web/upload/a, /web/upload/b and files 
/web/different/path/file where the files are somewhere else located. Due to 
this, we not only need to delete the directories but also all files.


> Implement generic mechanism to receive files via rest
> -----------------------------------------------------
>
>                 Key: FLINK-9599
>                 URL: https://issues.apache.org/jira/browse/FLINK-9599
>             Project: Flink
>          Issue Type: New Feature
>          Components: REST
>            Reporter: Chesnay Schepler
>            Assignee: Chesnay Schepler
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.6.0
>
>
> As a prerequisite for a cleaner implementation of FLINK-9280 we should
>  * extend the RestClient to allow the upload of Files
>  * extend FileUploadHandler to accept mixed multi-part requests (json + files)
>  * generalize mechanism for accessing uploaded files in {{AbstractHandler}}
> Uploaded files can be forwarded to subsequent handlers as an attribute, 
> similar to the existing special case for the {{JarUploadHandler}}. The JSON 
> body can be forwarded by replacing the incoming http requests with a simple 
> {{DefaultFullHttpRequest}}.
> Uploaded files will be retrievable through the {{HandlerRequest}}.
> I'm not certain if/how we can document that a handler accepts files.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to