Make PreBuiltAnalyzerProviderFactory plugable via AnalysisPlugin and#31095
Conversation
|
Pinging @elastic/es-search-aggs |
nik9000
left a comment
There was a problem hiding this comment.
I left a few minor things but I'm marking this "request changes" because I'm confused about how closing works with PreBuiltAnalyzers.
There was a problem hiding this comment.
I think the old indentation was better here.
There was a problem hiding this comment.
I think it'd be nice to remove this second ctor so we're explicit every time.
There was a problem hiding this comment.
I think this is worth some javadoc and comments to explain why we do it. It makes sense but things like an empty put method will confuse future me when I read this class in six months.
There was a problem hiding this comment.
It'd be super nice to explain that we only need this because of PreBuiltAnalyzers.
There was a problem hiding this comment.
It looks to me like this will hit a NullPointerException.
There was a problem hiding this comment.
When you use it with an instance of PreBuiltAnalyzers.
There was a problem hiding this comment.
good catch. I will fix that.
There was a problem hiding this comment.
setupPreBuiltAnalyzerProviderFactories?
|
Thanks @nik9000 for reviewing. I've updated this PR. |
move `finger_print`, `pattern` and `standard_html_strip` analyzers to analysis-common module. (both AnalysisProvider and PreBuiltAnalyzerProvider) Changed PreBuiltAnalyzerProviderFactory to extend from PreConfiguredAnalysisComponent and changed to make sure that predefined analyzers are always instantiated with the current ES version and if an instance is requested for a different version then delegate to PreBuiltCache. This is similar to the behaviour that exists today in AnalysisRegistry.PreBuiltAnalysis and PreBuiltAnalyzerProviderFactory. Relates to elastic#23658
506bdcc to
97f7539
Compare
move `finger_print`, `pattern` and `standard_html_strip` analyzers to analysis-common module. (both AnalysisProvider and PreBuiltAnalyzerProvider) Changed PreBuiltAnalyzerProviderFactory to extend from PreConfiguredAnalysisComponent and changed to make sure that predefined analyzers are always instantiated with the current ES version and if an instance is requested for a different version then delegate to PreBuiltCache. This is similar to the behaviour that exists today in AnalysisRegistry.PreBuiltAnalysis and PreBuiltAnalyzerProviderFactory. (#31095) Relates to #23658
move
finger_print,patternandstandard_html_stripanalyzersto analysis-common module. (both AnalysisProvider and PreBuiltAnalyzerProvider)
Changed PreBuiltAnalyzerProviderFactory to extend from PreConfiguredAnalysisComponent and
changed to make sure that predefined analyzers are always instantiated with the current
ES version and if an instance is requested for a different version then delegate to PreBuiltCache.
This is similar to the behaviour that exists today in AnalysisRegistry.PreBuiltAnalysis and
PreBuiltAnalyzerProviderFactory.
Relates to #23658