[GitHub] [nifi] kent-nguyen commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor

2020-07-23 Thread GitBox 


kent-nguyen commented on a change in pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#discussion_r459806345



##
File path: 
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java
##
@@ -152,6 +153,14 @@
 .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
 .build();
 
+public static final PropertyDescriptor CACHE_CONTROL = new 
PropertyDescriptor.Builder()
+.name("Cache Control")
+.displayName("Cache Control")
+.description("Sets the Cache-Control HTTP header. Multiple 
directives are comma-separated.")

Review comment:
   Hi @turcsanyip Thank you for checking my PR. It's great to hear you ran 
the tests on it. I had carefully tested it on my local too.
   
   In contrast to the Content Type property, which can get the MIME string from 
the flow file itself, the flow file metadata wouldn't normally be relevant for 
the Cache Control property. I felt this was similar in nature to properties 
like Storage Class and Server Side Encryption, which are on set on the 
processor and do not support expression language.
   
   I have updated the description as your suggestion. Thank you again.





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

[GitHub] [nifi] kent-nguyen commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor

2020-07-23 Thread GitBox 


kent-nguyen commented on a change in pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#discussion_r459806345



##
File path: 
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java
##
@@ -152,6 +153,14 @@
 .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
 .build();
 
+public static final PropertyDescriptor CACHE_CONTROL = new 
PropertyDescriptor.Builder()
+.name("Cache Control")
+.displayName("Cache Control")
+.description("Sets the Cache-Control HTTP header. Multiple 
directives are comma-separated.")

Review comment:
   Hi @turcsanyip Thank you for checking my PR. It's great to hear you ran 
the tests on it. I had carefully tested it on my local too.
   
   In contrast to Content Type property, which can properly get MIME string 
from the flow file itself, the Cache Control is not relevant to flow file meta 
data, it is purely set to the processor itself.
   
   Other similar properties such as Storage Class, Server Side Encryption ... 
are purely set to the processor and does not support expression language either.
   
   I have updated the description as your suggestion. Thank you again.





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