aljoscha commented on issue #9434: [FLINK-13634][Formats]: added module to compress data using StreamingFileSink URL: https://github.com/apache/flink/pull/9434#issuecomment-559187210 Thanks for the quick response! I'm afraid I didn't express myself too clearly. We can still have the builder pattern, i.e. `CompressionWriterFactory` can have the builder methods for setting extractor and codec. The naming of the `BulkWriter.Factory` is unfortunately not very good and it could have been called `BulkWriter.Builder` instead. I thought that an `Extractor` was always needed so the static method on `HadoopCompressionWriters` could have been `forExtractor(Extractor)` and that returns the factory where you have the methods for setting the compression codec. If that's not the case you can also have a static method without parameters and do everything via the "builder". Also, we shouldn't have both `hadoopcompress` and `hadoop`. We can either have only `hadoopcompress` or `compress` and then inside that `compress.hadoop`. Are you actually planning to add more compression codecs besides Hadoop? I think planning for those future additions would mean we have to potentially make the interfaces more independent of Hadoop. Some Javadoc comments are outdated now, for example `CompressionWriterFactory` has ``` * A {@link BulkWriter} implementation that compress file. ``` Sorry for the inconvenience for you!
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
