[GitHub] [nifi] MikeThomsen commented on a diff in pull request #6294: NIFI-2827 - Add support for ZSTD compression format

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

MikeThomsen commented on code in PR #6294:
URL: https://github.com/apache/nifi/pull/6294#discussion_r962161081


##########
minifi/pom.xml:
##########
@@ -504,6 +504,11 @@ limitations under the License.
                 <artifactId>lzma-java</artifactId>
                 <version>1.3</version>
             </dependency>
+            <dependency>

Review Comment:
   Not specific to this line, but minifi and nifi-standard-bundle need their NOTICE files updated to include this dependency so Facebook gets the appropriate attribution since we are redistributing their binaries.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/CompressContent.java:
##########
@@ -328,6 +335,11 @@ public void process(final InputStream rawIn, final OutputStream rawOut) throws I
                                     mimeTypeRef.set("application/x-lz4-framed");
                                     compressionOut = new CompressorStreamFactory().createCompressorOutputStream(compressionFormat.toLowerCase(), bufferedOut);
                                     break;
+                                case COMPRESSION_FORMAT_ZSTD:
+                                    final int zstdcompressionLevel = context.getProperty(COMPRESSION_LEVEL).asInteger() * 2;

Review Comment:
   Non-standard variable name. Also, why is the value multiplied by 2?



##########
nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/JsonRecordSetWriter.java:
##########
@@ -72,6 +72,7 @@ public class JsonRecordSetWriter extends DateTimeTextRecordSetWriter implements
     public static final String COMPRESSION_FORMAT_SNAPPY = "snappy";
     public static final String COMPRESSION_FORMAT_SNAPPY_FRAMED = "snappy framed";
     public static final String COMPRESSION_FORMAT_NONE = "none";
+    public static final String COMPRESSION_FORMAT_ZSTD = "zstd";

Review Comment:
   Would like to see a unit test that validates this doesn't break anything.



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