Should ICompressionProvider be dropped and replaced by commons-compress abstraction?

[rmannibucau]

I have a dumb need to compress a file using zstd (I can detail the underlying need if it helps but skipping for now), issue is that with current abstraction in Apache Hop it requires a custom plugin.

My idea is to use commons-compress as primary API and support so we get all the supported formats almost for free which more or less cover all the possible public formats.

Any reason it is not already done?

Alternatively a generic commons-compress format can be a workaround - while it is built- in.

Side note: I'm ok adding jars/libs, I'm just trying to not have to code for that need of extensibility of format/better coverage.

edit: current workaround for future readers:

@CompressionPlugin(
  id = "Zstd",
  name = "Zstd",
  description = "Zstandard compression provider for benchmark"
)
public class ZstdCompressionProvider implements ICompressionProvider {
  @Override
  public CompressionOutputStream createOutputStream(final OutputStream out) throws IOException {
    return new CompressionOutputStream(new ZstdOutputStream(out));
  }

  @Override
  public CompressionInputStream createInputStream(final InputStream in) throws IOException {
    return new CompressionInputStream(new ZstdInputStream(in));
  }

  @Override
  public boolean supportsInput() {
    return true;
  }

  @Override
  public boolean supportsOutput() {
    return true;
  }

  @Override
  public String getDefaultExtension() {
    return "zst";
  }
}

[mattcasters]

Romain, it's just a plugin interface. You can add whichever library or class you like. The intent of the @CompressionPlugin / ICompressionProvider combo is to provide compression methods for other features in Hop, like Text File Input/Output etc.

So wrap whichever compression provider in that interface, paint the class with the annotation, and it becomes available where needed.

[rmannibucau]

@mattcasters no no, not blaming at all, just trying to validate 1. it is ok to impl it in "core", 2. that commons-compress is the abstraction we want and we drop overlapping hop one

then i can propose a PR

[mattcasters]

Sorry, I can't make up what you mean by "implement it in core" What do you mean by it and are you sure you want to tie yourself to a single library for all compression related things? Does it use the same interface as what the various transforms use? What is the point of doing this major refactoring? Do you have time to write all the integration tests to do this? Is the risk worth the efforts it takes? Those are the sorts of questions I would have.
Since this has come up before: create an issue that an be triaged with the description of the concrete problem you want to solve. It has to be more than "I found a library" ;-)

[rmannibucau]

core = in default hop distribution
are you sure you want to tie yourself to a single library for all compression related things = yes and no, there it overlaps quite much and covers way more, i'm not planning to break extensibility just reduce hop specific code
what is the point of doing this major refactoring = get 80% coverage for free with less difficulties than custom plugins for end users
do you have time to write all the integration tests to do this = hop already has a few and commons-compress more than hop so think the question is more "do you really want something better tested" from my window
Is the risk worth the efforts it takes = 200% yes, needing code in hop defeats the concept IMHO - and it is why Im asking how to solve it there

Since this has come up before: create an issue that an be triaged with the description of the concrete problem you want to solve. It has to be more than "I found a library" ;-)

hmm, last time i got asked to open a discussion 🤔 , when are discussion the way to go then?

[hansva]

I see no issue in adding it as long as the plugins keep working and they complement each other.
Someone could always need some obscure format that isn't supported by commons-compress

[mattcasters]

@hansva exactly.
@rmannibucau This wasn't a discussion.