kl0u commented on a change in pull request #11307: [FLINK-16371] [BulkWriter] Fix Hadoop Compression BulkWriter URL: https://github.com/apache/flink/pull/11307#discussion_r389642473
########## File path: flink-formats/flink-compress/src/main/java/org/apache/flink/formats/compress/CompressWriterFactory.java ########## @@ -42,39 +47,57 @@ private Extractor<IN> extractor; private CompressionCodec hadoopCodec; + private String hadoopCodecName; + private Map<String, String> hadoopConfigurationMap; + private String codecExtension; public CompressWriterFactory(Extractor<IN> extractor) { - this.extractor = Preconditions.checkNotNull(extractor, "extractor cannot be null"); + this.extractor = checkNotNull(extractor, "Extractor cannot be null"); + this.hadoopConfigurationMap = new HashMap<>(); } public CompressWriterFactory<IN> withHadoopCompression(String hadoopCodecName) { return withHadoopCompression(hadoopCodecName, new Configuration()); } public CompressWriterFactory<IN> withHadoopCompression(String hadoopCodecName, Configuration hadoopConfiguration) { - return withHadoopCompression(new CompressionCodecFactory(hadoopConfiguration).getCodecByName(hadoopCodecName)); - } + CompressionCodec codec = new CompressionCodecFactory(hadoopConfiguration).getCodecByName(hadoopCodecName); + this.codecExtension = checkNotNull(codec, "Unable to load the provided Hadoop codec [" + hadoopCodecName + "]") + .getDefaultExtension(); + + this.hadoopCodecName = hadoopCodecName; + + for (Map.Entry<String, String> entry : hadoopConfiguration) { + hadoopConfigurationMap.put(entry.getKey(), entry.getValue()); + } - public CompressWriterFactory<IN> withHadoopCompression(CompressionCodec hadoopCodec) { - this.hadoopCodec = Preconditions.checkNotNull(hadoopCodec, "hadoopCodec cannot be null"); return this; } @Override public BulkWriter<IN> create(FSDataOutputStream out) throws IOException { - try { - return (hadoopCodec != null) - ? new HadoopCompressionBulkWriter<>(out, extractor, hadoopCodec) - : new NoCompressionBulkWriter<>(out, extractor); - } catch (Exception e) { - throw new IOException(e.getLocalizedMessage(), e); + if (hadoopCodecName == null || hadoopCodecName.length() == 0) { Review comment: The second check can become ` hadoopCodecName.trim().isEmpty()` if we want to be strict. ---------------------------------------------------------------- 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 With regards, Apache Git Services