Aggregations Refactor: Refactor Percentiles and Percentile Ranks Aggregation #14836
Conversation
| // NORELEASE remove this check when agg refactoring complete | ||
| if (factoryPrototype != null) { | ||
| namedWriteableRegistry.registerPrototype(AggregatorFactory.class, factoryPrototype); | ||
| if (factoryPrototypes != null) { |
There was a problem hiding this comment.
if we return an array, I'd rather like to enforce it to be non-null: if you don't want to register something you can provide an empty array?
There was a problem hiding this comment.
I absolutely agree. This null check is only here while there are Aggregations which are not refactored. Once the aggregation refactoring is complete this check will be removed and the method will need to return a non-null value (see NORELEASE comment above). I could go through and make all non refactored aggregation return an empty array for this method but I wanted to leave them returning null so that if this check is removed before all aggregations have been refactored we will be alerted to the factor we have not refactored one or more of the aggregations as the node will throw a NPE on start up.
There was a problem hiding this comment.
woops, the change made me miss the norelease comment above!
|
LGTM in general, I just left one comment. Could you also add javadocs to the factories as they will be exposed to users? |
colings86
force-pushed
the
feature/aggs-refactoring
branch
from
8f03a9d to
af625f0
Compare
clintongormley
added
:Search/Search
No description provided.