You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/04/27 06:43:33 UTC
[GitHub] [jackrabbit-oak] thomasmueller commented on a diff in pull request #552: OAK-9753: ES connection parameters should be easily accessible from ElasticConnectionRule
thomasmueller commented on code in PR #552:
URL: https://github.com/apache/jackrabbit-oak/pull/552#discussion_r859435185
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -130,6 +139,46 @@ private void downloadSimilaritySearchPluginIfNotExists(String localPluginPath, S
}
}
+
+
+ private ElasticConnectionConnectionModel initializeElasticConnectionModel (String elasticConnectionString) {
+ try {
+ URI uri = new URI(elasticConnectionString);
+ String host = uri.getHost();
+ String scheme = uri.getScheme();
+ int port = uri.getPort();
+ String query = uri.getQuery();
+
+ String api_key = null;
Review Comment:
apiKey
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -130,6 +139,46 @@ private void downloadSimilaritySearchPluginIfNotExists(String localPluginPath, S
}
}
+
+
+ private ElasticConnectionConnectionModel initializeElasticConnectionModel (String elasticConnectionString) {
Review Comment:
Unneeded space before (
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -75,18 +77,21 @@ public Statement apply(Statement base, Description description) {
final String pluginFileName = "elastiknn-" + pluginVersion + ".zip";
final String localPluginPath = "target/" + pluginFileName;
downloadSimilaritySearchPluginIfNotExists(localPluginPath, pluginVersion);
- if (elasticConnectionString == null || getElasticConnectionFromString() == null) {
+ if (!isValidUri(elasticConnectionString)) {
checkIfDockerClientAvailable();
Network network = Network.newNetwork();
elastic = new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:" + Version.CURRENT)
.withCopyFileToContainer(MountableFile.forHostPath(localPluginPath), "/tmp/plugins/" + pluginFileName)
.withCopyFileToContainer(MountableFile.forClasspathResource("elasticstartscript.sh"), "/tmp/elasticstartscript.sh")
.withCommand("bash /tmp/elasticstartscript.sh")
- .withNetwork(network);
+ .withNetwork(network).withStartupAttempts(3);
elastic.start();
-
setUseDocker(true);
+ initializeElasticConnectionModel(elastic);
+ }
+ else {
Review Comment:
I usually use "} else {" on the same line, but it's fine
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -130,6 +139,46 @@ private void downloadSimilaritySearchPluginIfNotExists(String localPluginPath, S
}
}
+
+
+ private ElasticConnectionConnectionModel initializeElasticConnectionModel (String elasticConnectionString) {
+ try {
+ URI uri = new URI(elasticConnectionString);
+ String host = uri.getHost();
+ String scheme = uri.getScheme();
+ int port = uri.getPort();
+ String query = uri.getQuery();
+
+ String api_key = null;
+ String api_secret = null;
+ if (query != null) {
+ api_key = query.split(",")[0].split("=")[1];
+ api_secret = query.split(",")[1].split("=")[1];
+ }
+ this.elasticConnectionConnectionModel = new ElasticConnectionConnectionModel();
+ elasticConnectionConnectionModel.scheme = scheme;
+ elasticConnectionConnectionModel.elasticHost = host;
+ elasticConnectionConnectionModel.elasticPort = port;
+ elasticConnectionConnectionModel.elasticApiKey = api_key;
+ elasticConnectionConnectionModel.elasticApiSecret = api_secret;
+ elasticConnectionConnectionModel.indexPrefix = INDEX_PREFIX + System.currentTimeMillis();
+ return elasticConnectionConnectionModel;
+ } catch (URISyntaxException e) {
+ return null;
Review Comment:
Log an exception?
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -154,17 +203,45 @@ public ElasticConnection getElasticConnectionFromString() {
}
}
+ private boolean isValidUri(String connectionString) {
+ if (connectionString == null) {
Review Comment:
I wonder if we should use "connectionString == null || connectionString.trim().isEmpty()"
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -154,17 +203,45 @@ public ElasticConnection getElasticConnectionFromString() {
}
}
+ private boolean isValidUri(String connectionString) {
+ if (connectionString == null) {
+ return false;
+ }
+ try {
+ new URI(connectionString);
+ return true;
+ } catch (URISyntaxException e) {
+ return false;
Review Comment:
What about log.debug("Not a vlid URI: {}", connectionString, e) ? So that one can debug without having to run it in debug mode. As somebody might have a typo in the URI.
##########
oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticConnectionRule.java:
##########
@@ -130,6 +139,46 @@ private void downloadSimilaritySearchPluginIfNotExists(String localPluginPath, S
}
}
+
+
+ private ElasticConnectionConnectionModel initializeElasticConnectionModel (String elasticConnectionString) {
+ try {
+ URI uri = new URI(elasticConnectionString);
+ String host = uri.getHost();
+ String scheme = uri.getScheme();
+ int port = uri.getPort();
+ String query = uri.getQuery();
+
+ String api_key = null;
+ String api_secret = null;
Review Comment:
apiSecret
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org