ppkarwasz commented on PR #4234: URL: https://github.com/apache/logging-log4j2/pull/4234#issuecomment-5757395430
Hi @katstack, Thank you for the PR. As @ramanathan1504 mentioned, we are considering removing `compressionLevel` in `3.x` in favor of a more general mechanism (#2950): every compression algorithm has its own set of parameters, and adding a `RolloverStrategy` attribute for each of them does not scale. Could you have a look at #2950 and tell us about your use case? Your point of view would help us shape that design. That said, the attribute exists today, so we might as well honor it wherever we can. ZStandard is not the only algorithm with a compression level: Deflate, GZip and BZip2 support one too. We prefer to change a whole class of implementations at once rather than a single one, so users get consistent behavior and the change can be properly described in the release notes. Would you be willing to extend the PR to the other three algorithms? Two remarks on the implementation: - Please move `ZstdCompressAction` to a new `actions.internal` package, so it is not exported via JPMS/OSGi. Everything we export becomes public API that we must keep compatible for years, so we only export components meant to be reused. These compression actions are Log4j-specific; reusable compression code belongs in Commons Compress. - `CommonsCompressAction`, `GzCompressAction` and `ZipCompressAction` already duplicate a lot of logic. Extracting a common `AbstractCompressAction` (also in `actions.internal`) would let the new action share it instead of adding a fourth copy. **TL;DR**: moving `ZstdCompressAction` to `actions.internal` is the one required change; covering the other `compressionLevel`-aware algorithms in this PR would be very welcome. -- 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]
