exceptionfactory commented on code in PR #7139:
URL: https://github.com/apache/nifi/pull/7139#discussion_r1161852135
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/PutGCSObject.java:
##########
@@ -194,6 +194,16 @@ public class PutGCSObject extends AbstractGCSProcessor {
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ public static final PropertyDescriptor GZIPCONTENT = new PropertyDescriptor
+ .Builder().name("gzip.content")
+ .displayName("Allow GZIP Compression")
+ .description("Signals to the GCS Blob Writer whether GZIP
compression during transfer is desired. " +
+ "False means dont gzip and can boost performance in many
cases.")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .defaultValue("False")
+ .allowableValues("True", "False")
+ .defaultValue("True")
Review Comment:
The `defaultValue()` is being called twice, with the effective value being
set to `True`. This should be corrected to the preferred setting.
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/PutGCSObject.java:
##########
@@ -194,6 +194,16 @@ public class PutGCSObject extends AbstractGCSProcessor {
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ public static final PropertyDescriptor GZIPCONTENT = new PropertyDescriptor
+ .Builder().name("gzip.content")
Review Comment:
```suggestion
.Builder().name("gzip.compression.enabled")
```
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/test/java/org/apache/nifi/processors/gcp/storage/PutGCSObjectTest.java:
##########
@@ -180,7 +180,7 @@ public void testSuccessfulPutOperation() throws Exception {
runner.setProperty(PutGCSObject.ENCRYPTION_KEY, ENCRYPTION_KEY);
runner.setProperty(PutGCSObject.OVERWRITE, String.valueOf(OVERWRITE));
runner.setProperty(PutGCSObject.CONTENT_DISPOSITION_TYPE,
CONTENT_DISPOSITION_TYPE);
-
+ runner.setProperty(PutGCSObject.GZIPCONTENT, "False");
Review Comment:
```suggestion
runner.setProperty(PutGCSObject.GZIPCONTENT,
Boolean.FALSE.toString());
```
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/PutGCSObject.java:
##########
@@ -194,6 +194,16 @@ public class PutGCSObject extends AbstractGCSProcessor {
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ public static final PropertyDescriptor GZIPCONTENT = new PropertyDescriptor
+ .Builder().name("gzip.content")
+ .displayName("Allow GZIP Compression")
Review Comment:
Recommend adjusting the wording to use `Enabled` instead of `Allow` so the
behavior is clear.
```suggestion
.displayName("GZIP Compression Enabled")
```
##########
nifi-nar-bundles/nifi-gcp-bundle/nifi-gcp-processors/src/main/java/org/apache/nifi/processors/gcp/storage/PutGCSObject.java:
##########
@@ -194,6 +194,16 @@ public class PutGCSObject extends AbstractGCSProcessor {
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ public static final PropertyDescriptor GZIPCONTENT = new PropertyDescriptor
+ .Builder().name("gzip.content")
+ .displayName("Allow GZIP Compression")
+ .description("Signals to the GCS Blob Writer whether GZIP
compression during transfer is desired. " +
+ "False means dont gzip and can boost performance in many
cases.")
+ .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
+ .defaultValue("False")
+ .allowableValues("True", "False")
+ .defaultValue("True")
Review Comment:
Although various Processors are not consistent, other GCS Processors use the
lowercase values for Boolean settings. This also aligns with Java
Boolean.toString().
```suggestion
.allowableValues(Boolean.TRUE.toString(),
Boolean.FALSE.toString())
.defaultValue(Boolean.TRUE.toString())
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
