quickwit icon indicating copy to clipboard operation
quickwit copied to clipboard

Refactor Source building

Open fulmicoton opened this issue 4 years ago • 4 comments

In #1058, we did one step from (dynamic factory + json) to a static enum world in order to have source type specific source params validation.

We are now half-way between two worlds.

Solution 1

Just go full enum. The factory is a function of the enum. Pros: Very simple Cons: Does not separate the concern super well.

Note: If we go for that route, source_type() should return an enum too.

Solution 2

Use a SourceParams trait that can build a Source type (does the job of the factory) and do dark magic, like what @fmassot did for the IndexConfig deserialization.

fulmicoton avatar Jan 11 '22 01:01 fulmicoton

@fmassot @guilload what do you think?

fulmicoton avatar Jan 11 '22 01:01 fulmicoton

yes totally! I must say that @guilload stopped himself in the middle of the river to have something ready for 0.2 :)

fmassot avatar Jan 11 '22 01:01 fmassot

Solution 1 or solution 2?

fulmicoton avatar Jan 11 '22 04:01 fulmicoton

I was working around that code yesterday, and I'm leaning towards solution 1. source_type() does now return an enum. We can dispatch directly and statically in load_source. We can optionally keep the SourceFactory trait around to preserve the decoupling.

guilload avatar Apr 26 '24 13:04 guilload