Move pre-configured "keyword" tokenizer to the analysis-common module#24863
Move pre-configured "keyword" tokenizer to the analysis-common module#24863nik9000 merged 4 commits intoelastic:masterfrom
Conversation
|
@jpountz, can you have a look at this one? |
|
Does it mean we can't get normalization to work if analysis-common is not on the classpath? |
Yes. |
|
The change looks correct to me, but I'm a bit confused about how integration tests that rely on normalization work with this change? We don't put analysis-common on the classpath when running integ tests, do we? |
|
I suspect we don't actually test custom normalizers without analysis-common installed. We have unit tests but they don't need run this particular code. I could only find some documentation that builds a custom normalizer. I didn't see anything in the rest-api-spec. |
jpountz
left a comment
There was a problem hiding this comment.
OK I got it now, existing tests that make sure we apply normalization are done on predefined analyzers such as the standard analyzer, this is why it keeps working. Sorry for the confusion. LGTM.
Thanks for reviewing! I'll see about removing the conflicts later today. |
|
Thanks for reviewing @jpountz! |
Moves the
keywordtokenizer to the analysis-common module. Thekeywordtokenizer is special because it is used byCustomNormalizerProviderso I pulled it out into its own PR. To get the move to work I've reworked the lookup from static to one using theAnalysisRegistry. This seems safe enough.Part of #23658.