New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch analysis from push to pull #19073

Merged
merged 1 commit into from Jun 26, 2016

Conversation

Projects
None yet
4 participants
@nik9000
Contributor

nik9000 commented Jun 24, 2016

Instead of plugins calling registerTokenizer and friends to add custom analysis components they now instead have to implement AnalysisPlugin and override getTokenizer and friends. This lines up extending plugins in with extending scripts and allows AnalysisModule to construct the AnalysisRegistry
immediately as part of its constructor which makes testing analysis much simpler.

This also moves the default analysis configuration into AnalysisModule which is how search is setup.

And it stops binding HunspellService in guice and reworks its test so that is no longer required.

Finally this removed extends AbstractModule from AnalysisService. AnalysisService is now only responsible for building the AnalysisRegistry and Node binds that with guice directly.

@nik9000

This comment has been minimized.

Show comment
Hide comment
@nik9000

nik9000 Jun 24, 2016

Contributor

@s1monw: this is another startup cleanup - it doesn't actually remove any @Inject annotations but it makes the AnalysisRegistry much simpler to access during startup which seems like the first step to removing it from guice.

Contributor

nik9000 commented Jun 24, 2016

@s1monw: this is another startup cleanup - it doesn't actually remove any @Inject annotations but it makes the AnalysisRegistry much simpler to access during startup which seems like the first step to removing it from guice.

@rjernst

View changes

Show outdated Hide outdated core/src/main/java/org/elasticsearch/plugins/AnalysisPlugin.java
@s1monw

View changes

Show outdated Hide outdated core/src/main/java/org/elasticsearch/plugins/AnalysisPlugin.java
@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Jun 25, 2016

Contributor

Great LGTM left some minors

Contributor

s1monw commented Jun 25, 2016

Great LGTM left some minors

Switch analysis from push to pull
Instead of plugins calling `registerTokenizer` to extend the analyzer
they now instead have to implement `AnalysisPlugin` and override
`getTokenizer`. This lines up extending plugins in with extending
scripts. This allows `AnalysisModule` to construct the `AnalysisRegistry`
immediately as part of its constructor which makes testing anslysis
much simpler.

This also moves the default analysis configuration into `AnalysisModule`
which is how search is setup.

Like `ScriptModule`, `AnalysisModule` no longer extends `AbstractModule`.
Instead it is only responsible for building `AnslysisRegistry`. We still
bind `AnalysisRegistry` but we only do so in `Node`. This is means it
is available at module construction time so we slowly remove the need to
bind it in guice.

@nik9000 nik9000 merged commit 71b95fb into elastic:master Jun 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment