[GitHub] [nifi] exceptionfactory commented on a change in pull request #5475: NIFI-830 For GET requests, InvokeHTTP should set the filename of the 'Response' FlowFile based on the URL

[GitBox <gi...@apache.org>]

exceptionfactory commented on a change in pull request #5475:
URL: https://github.com/apache/nifi/pull/5475#discussion_r736643699



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java
##########
@@ -482,6 +490,15 @@
             .allowableValues("True", "False")
             .build();
 
+    public static final PropertyDescriptor UPDATE_FILENAME = new PropertyDescriptor.Builder()
+            .name("update-filename")
+            .description("If true and HTTP method is GET, the FlowFile's filename will be extracted from the remote URL.")
+            .displayName("Update Filename")

Review comment:
       The property name seems somewhat unclear, although the explanation is helpful.  For greater clarity, what do you think about naming this property something like `Set Filename from URL`?
   
   The linked Jira issue also mentions the possibility of considering the `Content-Disposition` header, when found on a response, which could include the filename.  That might make sense as a separate PR.  With that thought in mind, it might make more sense to use an enum value here instead of a Boolean.  For example: `Filename Strategy`, with values of `RANDOM` and `URL`, with the option for a future third option named `RESPONSE_HEADER`.




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org