SQL: Handle uberjar scenario where the ES jdbc driver file is bundled in another jar #51856
Conversation
inside another jar file.
kept alive in Windows
|
Pinging @elastic/es-search (:Search/SQL) |
astefan
changed the title
| maj = vers[0]; | ||
| min = vers[1]; | ||
| rev = vers[2]; | ||
| } | ||
| } catch (Exception ex) { | ||
| throw new IllegalArgumentException("Detected Elasticsearch JDBC jar but cannot retrieve its version", ex); | ||
| } |
There was a problem hiding this comment.
Should we add a finally block to close the conn ?
(Maybe JarInputStream 's close() takes care of that, just wondering if we should be extra safe here)
There was a problem hiding this comment.
I don't think it's needed to close the URLConnection (actually, the class doesn't have a method to close/terminate it), only its stream: https://docs.oracle.com/javase/tutorial/networking/urls/readingWriting.html
| Manifest manifest = jar.getManifest(); | ||
| hash = manifest.getMainAttributes().getValue("Change"); | ||
| ver = manifest.getMainAttributes().getValue("X-Compile-Elasticsearch-Version"); | ||
| byte[] vers = from(ver); |
There was a problem hiding this comment.
I know this is unmodified code, however, it's noteworthy that server/src/main/java/org/elasticsearch/Version.java parses ints, not bytes. Not sure if that's because it later does some int maths with the components, or because it reserves the possibility to use some tagging values at some point (like 7.2.222 for a special release). Just mentioning it, should we ever want to align this.
| public void testVersionFromJarInJar() throws IOException { | ||
| Path dir = createTempDir(); | ||
| Path jarPath = dir.resolve("foo.jar"); // simulated uberjar containing the jdbc driver | ||
| Path innerJarPath = dir.resolve("es-sql-jdbc.jar"); // simulated ES JDBC driver file |
There was a problem hiding this comment.
Given it's a test, this is completely optional: extract the "es-sql-jdbc.jar" into a final var and reuse that below.
|
@elasticmachine update branch |
… in another jar (elastic#51856) (cherry picked from commit 6247b07)
Implementation for #50201