-
-
Notifications
You must be signed in to change notification settings - Fork 45
Upgrade to Elasticsearch 9.1.4 #174
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
base: develop
Are you sure you want to change the base?
Upgrade to Elasticsearch 9.1.4 #174
Conversation
- Upgrade Java to JVM 21 - Upgrade Gradle to 8.11 - Fix all deprecations - Fix integrations tests on Elasticsearch and re-enable them
f89efdb to
0994c0a
Compare
- Move sudachi lib requirement as a direct implementation dep (otherwise breaks entitlement policy) - Add entitlement policy test
0994c0a to
0ddb019
Compare
|
|
||
| java { | ||
| sourceCompatibility = JavaVersion.VERSION_21 | ||
| targetCompatibility = JavaVersion.VERSION_21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JVM 21 is required by ES9
| var esKind = sudachiEs.kind.get() | ||
| dependsOn embedVersion, packageJars, packageSpiJars | ||
| archiveBaseName.set("${esKind.engine.kind}-${esKind.version}-$archivesBaseName") | ||
| archiveBaseName.set("${esKind.engine.kind}-${esKind.version}-${base.archivesName.get()}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a deprecation fix in gradle
| class EsTestEnvExtension { | ||
| Path bundlePath = null | ||
| Path systemDic = null | ||
| Path configFile = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are now handled by addConfigFile (see below)
| dependencies { | ||
| spi(project(':spi')) | ||
| implementation(project(':spi')) | ||
| implementation('com.worksap.nlp:sudachi:0.7.4') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required to load the sudachi jar in ES9. We can't use the previous dynamic loading anymore due to entitlement-policy.yaml
| into "build/package/${version}/${esKind.engine.kind}-${esKind.version}/spi" | ||
| } else { | ||
| into "build/package/${version}/${esKind.engine.kind}-${esKind.version}" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above, we can't do dynamic loading due to entitlement-policy.yaml--at least I couldn't get it to work.
| private val basePath = resourcesPath(env, settings) | ||
| private val fullAnchor = PathAnchor.filesystem(basePath).andThen(anchor) | ||
| private val fullAnchor = | ||
| PathAnchor.filesystem(basePath).andThen(PathAnchor.classpath(ConfigAdapter::class.java)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this method ConfigAdapter(anchor: PathAnchor...), the anchor arg is no longer used in my code b/c we are now from forcing to load from either (1) config/sudachi dir or the (2) sudachi.jar itself. Things seem to work fine but perhaps I'm missing something. Perhaps we can remove the anchor arg entirely.
Note also the change to use ConfigAdapter::class.java. This is related to the fact that we are now loading the sudachi jar directly.
It would be a good idea to write more tests for this ConfigAdapter class so we can understand all usage scenarios.
Upgrade to Elasticsearch 9.1.4
This PR also does the following:
entitlement-policy.yamlframework.config/sudachidirectory. It is no longer allowed to load them from the root dir or other places..andThen(PathAnchor.classpath(ConfigAdapter::class.java)).Follow-up Ticket:
After these changes, Sudachi loads, but there still seem to be some entitlement policy failures in the log lines. These may be silent failures; it looks like ES still ultimately loads.
I believe these are coming from the
com.worksap.nlp.sudachi.PathAnchorclass (see below). It seems like it's trying to load from the user's home dir first (not allowed by entitlement-policy.yaml). Possible solutions:entitlement-policy.yamlto allow accessing the user's home dir --> strongly not preferred, would prefer to keep security locked down as per other ES plugin standards.