You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/07/23 11:00:01 UTC
[GitHub] [nifi] kent-nguyen opened a new pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
kent-nguyen opened a new pull request #4422:
URL: https://github.com/apache/nifi/pull/4422
Add new property 'Cache Control' to allow user to
set the cache-control http header on the S3 object.
This property is not required, and has no default value.
The implementation is similar to the Content-Type property,
except that this property does not allow Expression Language.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi] turcsanyip commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
turcsanyip commented on a change in pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#discussion_r460393047
##########
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:
@kent-nguyen The generic approach is to add EL support with Variable Registry scope (`ExpressionLanguageScope.VARIABLE_REGISTRY`) at least and to support Flow File attributes ('ExpressionLanguageScope.FLOWFILE_ATTRIBUTES') when it is reasonable.
Storage Class property also has the option "Reference parameter..." (actually it is provided by the framework because it is a selectable property with a drop-down list, but in fact it is similar to setting Variable Registry scoped EL support from code).
As far as I see, you are not assigned the Contributor role yet. Could you please request for "Jira contributor access" on the mailing list dev@nifi.apache.org?
Then please assign the Jira ticket to yourself.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi] kent-nguyen commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [nifi] kent-nguyen commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [nifi] turcsanyip commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
turcsanyip commented on a change in pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#discussion_r459726996
##########
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:
Could you please add a similar explanation as in case of Content Type:
"Sets the Cache-Control HTTP header _indicating the caching directives of the associated object_." (or similar)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi] asfgit closed pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4422:
URL: https://github.com/apache/nifi/pull/4422
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi] turcsanyip commented on a change in pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
turcsanyip commented on a change in pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#discussion_r460393047
##########
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:
@kent-nguyen The generic approach is to add EL support with Variable Registry scope (`ExpressionLanguageScope.VARIABLE_REGISTRY`) at least and to support Flow File attributes (`ExpressionLanguageScope.FLOWFILE_ATTRIBUTES`) when it is reasonable.
Storage Class property also has the option "Reference parameter..." (actually it is provided by the framework because it is a selectable property with a drop-down list, but in fact it is similar to setting Variable Registry scoped EL support from code).
As far as I see, you are not assigned the Contributor role yet. Could you please request for "Jira contributor access" on the mailing list dev@nifi.apache.org?
Then please assign the Jira ticket to yourself.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [nifi] kent-nguyen commented on pull request #4422: NIFI-6332 - Add Cache Control property to PutS3Object processor
Posted by GitBox <gi...@apache.org>.
kent-nguyen commented on pull request #4422:
URL: https://github.com/apache/nifi/pull/4422#issuecomment-663342359
Apologies for the force push, realise now that wasn't a good idea. Was trying to trigger CI again since it looks to have encountered a transient error, but I'll ignore that in future.
----------------------------------------------------------------
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:
users@infra.apache.org