Split mutate processor into one processor per function#14938
Split mutate processor into one processor per function#14938javanna merged 1 commit intoelastic:feature/ingestfrom
Conversation
9ceec5a to
93a30e2
Compare
There was a problem hiding this comment.
should we maybe make ConfigurationUtils more flexible to enable its use here? I understand that the error message may want to be customized in this case.
There was a problem hiding this comment.
not sure, I think it is ok to use ConfigurationUtils for the top level element and then have specific code in each factory, otherwise we need to add too much generic code to ConfigurationUtils to handle all the cases. We can always reconsider in the future though.
|
just left a few questions, tests look great! |
There was a problem hiding this comment.
Just a note when back porting this piece of code needs to be changed. I think we should use java8 where possible and in this case back porting is trivial and the isolated. Only in cases where the back port change wouldn't isolated and difficult we should hold back on java8 usage.
|
This is great! Left some minor comments. I do wonder if we group some processors in a single package? Right now there are a lot of packages with a single class, which kind of defeats the purpose of packages imo. Regardless LGTM. |
93a30e2 to
6e87ceb
Compare
|
I rebased and addressed comments, also added missing javadocs to each of the new processor. I agree regarding packages, I followed the "one processor per package" convention that we seem to have adopted, but we do end up with lots of packages, I guess we can discuss this as a followup. |
|
LGTM |
6e87ceb to
246c314
Compare
246c314 to
8f1f5d4
Compare
Split mutate processor into one processor per function