Stable Plugin API module and analysis interfaces by pgomulka · Pull Request #88775 · elastic/elasticsearch

pgomulka · 2022-07-25T15:00:23Z

This commit adds stable analysis plugin API with analysis components interfaces and annotations.
It does not contain any usage of it yet. Separate changes to introduce example plugins or refactoring to existing ones will follow later.

It contains two gradle modules. One plugin-api with two annotations Nameable and NamedComponent, which can be reused for plugins other than analysis.
And second analysis-plugin-api which contains analysis components (TokenFilterFactory, CharFilterFactory etc)

NamedComponent - used by plugin developer - indicates that a Nameable component will be registered under a given name.
Nameable - for analysis plugins it is only used by the stable analysis api designers (ES) - indicates that component have a name and should be declared with NamedComponent

additional tasks that will follow: #88980

This commit adds stable plugin API module and introduces analysis components interfaces and annotations. Stable plugin API is influenced by org.elasticserach.index.analysis classes. It allows to remove the server dependency from plugins.

elasticsearchmachine · 2022-08-01T14:13:22Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

elasticsearchmachine · 2022-08-01T14:13:22Z

Hi @pgomulka, I've created a changelog YAML for you.

romseygeek

The API looks sensible to me. Is the plan to add this to a branch and then start migrating our internal components to use these factories?

romseygeek · 2022-08-02T09:41:37Z

libs/analysis-plugin-api/src/main/java/org/elasticsearch/analysis/plugin/api/Analyzer.java

+     * Returns a lucene org.apache.lucene.analysis.Analyzer instance
+     */
+    T create();
+


I think we'll need getAnalysisMode on here as well.

pgomulka · 2022-08-02T10:02:53Z

Is the plan to add this to a branch and then start migrating our internal components to use these factories?

@romseygeek we want merge this API and infrastructure to main branch. This will be followed by example plugins implementing this API and possibly some of analysis plugins we have in our repo (we did a PoC with nori)

… into stable_plugin_api

ChrisHegarty · 2022-08-08T20:14:55Z

libs/plugin-analysis-api/build.gradle

+dependencies {
+  api "org.apache.lucene:lucene-core:${versions.lucene}"
+  api "org.apache.lucene:lucene-analysis-common:${versions.lucene}"
+  api "org.apache.lucene:lucene-analysis-common:${versions.lucene}"


duplicate api dependency?

ChrisHegarty · 2022-08-08T20:17:30Z

libs/plugin-analysis-api/src/main/java/module-info.java

+
+module org.elasticsearch.analysis.plugin.api {
+    requires org.apache.lucene.core;
+    requires org.elasticsearch.plugin.api;


I noticed that there is no requires on lucene analysis common (which is in the Gradle dependencies)? Maybe it can be removed from the Gradle api dependency?

good point, I am not sure why I needed this in the PoC branch. We can always add it back when needed

libs/plugin-analysis-api/src/main/java/org/elasticsearch/plugin/analysis/api/AnalysisMode.java

ChrisHegarty · 2022-08-08T20:23:01Z

libs/plugin-analysis-api/src/main/java/org/elasticsearch/plugin/analysis/api/AnalysisMode.java

+        }
+    };
+
+    private String readableName;


this field can be final, right?

...ugin-analysis-api/src/main/java/org/elasticsearch/plugin/analysis/api/CharFilterFactory.java

ChrisHegarty · 2022-08-08T20:25:31Z

...ugin-analysis-api/src/main/java/org/elasticsearch/plugin/analysis/api/CharFilterFactory.java

+    Reader create(Reader reader);
+
+    /**
+     * Normalize a tokenStream for use in multi-term queries


can we please end all sentences with a period/stop.

libs/plugin-api/src/main/java/org/elasticsearch/plugin/api/Nameable.java

ChrisHegarty · 2022-08-09T15:29:12Z

libs/plugin-api/src/main/java/org/elasticsearch/plugin/api/NamedComponent.java

+@Target({ ElementType.TYPE })
+public @interface NamedComponent {
+    /**
+     * A name used for registration and lookup


Trivially, can we change this to the definitive article, "A name" -> "The name"

ChrisHegarty · 2022-08-09T15:30:22Z

libs/plugin-api/src/main/java/org/elasticsearch/plugin/api/package-info.java

+ * <ul>
+ *      <li> The root package is org.elasticsearch.plugin</li>
+ *      <li> Specialised API jars have their name following the root package.
+ *           i.e. org.elasticsearch.analysis


This should be org.elasticsearch.plugin.analysis, right?

ChrisHegarty · 2022-08-09T15:30:38Z

libs/plugin-api/src/main/java/org/elasticsearch/plugin/api/package-info.java

+ *           i.e. org.elasticsearch.analysis
+ *      </li>
+ *      <li> Interfaces and annotations used by plugin developers are in `api` package
+ *           i.e org.elasticsearch.analysis.api or org.elasticsearch.api


o.e.plugin.analysis.api

somehow I missed those comments.. I will fix this in #89772

romseygeek · 2022-08-15T12:26:49Z

...plugin-analysis-api/src/main/java/org/elasticsearch/plugin/analysis/api/AnalyzerFactory.java

+/**
+ * An analysis component used to create Analyzers.
+ */
+public interface AnalyzerFactory<T extends org.apache.lucene.analysis.Analyzer> extends Nameable {


Do we need the generic parameter here? The other factories are just returning a base type, which I think should be enough?

agree, I don't think we need a generic here

romseygeek

LGTM

Stable Plugin API module and analysis interfaces

f0b0ef8

This commit adds stable plugin API module and introduces analysis components interfaces and annotations. Stable plugin API is influenced by org.elasticserach.index.analysis classes. It allows to remove the server dependency from plugins.

pgomulka added the :Core/Infra/Plugins Plugin API and infrastructure label Jul 25, 2022

elasticsearchmachine added the v8.4.0 label Jul 25, 2022

fix lucene notice

3d67300

mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022

pgomulka added 3 commits July 29, 2022 16:52

normalize and other

6bb8d35

analysis mode

6c60786

split into two modules

1d54832

pgomulka mentioned this pull request Aug 1, 2022

Stable Analysis Plugin API #88980

Open

25 tasks

pgomulka requested review from romseygeek and removed request for romseygeek August 1, 2022 11:47

pgomulka added 4 commits August 1, 2022 15:01

javadocs

041fd86

Merge remote-tracking branch 'upstream/main' into stable_plugin_api

249df5c

small javadoc changes

d4801f8

udpate shas

e3bdcc9

pgomulka marked this pull request as ready for review August 1, 2022 14:12

pgomulka added the >feature label Aug 1, 2022

pgomulka requested a review from romseygeek August 1, 2022 14:13

elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Aug 1, 2022

Update docs/changelog/88775.yaml

5051f62

romseygeek reviewed Aug 2, 2022

View reviewed changes

pgomulka added 4 commits August 3, 2022 13:20

rename analyzer

5fcc512

javadoc

a5d596f

Merge branch 'stable_plugin_api' of github.com:pgomulka/elasticsearch…

821b6d9

… into stable_plugin_api

package rename

1dd3e00

pgomulka requested a review from romseygeek August 8, 2022 10:02

pgomulka requested a review from ChrisHegarty August 8, 2022 10:02

ChrisHegarty reviewed Aug 8, 2022

View reviewed changes

pgomulka added 3 commits August 9, 2022 09:58

code review follow up. javadoc and dependency cleanup

9a5e4ea

Merge remote-tracking branch 'upstream/main' into stable_plugin_api

b00463e

javadoc and dependency

bf1c940

ChrisHegarty approved these changes Aug 9, 2022

View reviewed changes

romseygeek reviewed Aug 15, 2022

View reviewed changes

pgomulka added 2 commits August 29, 2022 11:25

remove generic parameter from AnalyzerFactory

504d8fe

Merge remote-tracking branch 'upstream/main' into stable_plugin_api

94f6d24

romseygeek approved these changes Aug 30, 2022

View reviewed changes

pgomulka merged commit bc4c678 into elastic:main Aug 30, 2022

Conversation

pgomulka commented Jul 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Aug 1, 2022

Uh oh!

elasticsearchmachine commented Aug 1, 2022

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka commented Aug 2, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pgomulka commented Jul 25, 2022 •

edited

Loading