Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/336#discussion_r96759997
--- Diff:
data/src/main/scala/org/apache/predictionio/data/storage/elasticsearch/ESAccessKeys.scala
---
@@ -15,44 +15,45 @@
* limitations under the License.
*/
-
package org.apache.predictionio.data.storage.elasticsearch
-import grizzled.slf4j.Logging
-import org.apache.predictionio.data.storage.StorageClientConfig
+import java.io.IOException
+
+import scala.collection.JavaConverters.mapAsJavaMapConverter
+
+import org.apache.http.entity.ContentType
+import org.apache.http.nio.entity.NStringEntity
+import org.apache.http.util.EntityUtils
import org.apache.predictionio.data.storage.AccessKey
import org.apache.predictionio.data.storage.AccessKeys
-import org.elasticsearch.ElasticsearchException
-import org.elasticsearch.client.Client
-import org.elasticsearch.index.query.FilterBuilders._
-import org.json4s.JsonDSL._
+import org.apache.predictionio.data.storage.StorageClientConfig
+import org.elasticsearch.client.RestClient
import org.json4s._
+import org.json4s.JsonDSL._
import org.json4s.native.JsonMethods._
-import org.json4s.native.Serialization.read
import org.json4s.native.Serialization.write
-import scala.util.Random
+import grizzled.slf4j.Logging
+import org.elasticsearch.client.ResponseException
/** Elasticsearch implementation of AccessKeys. */
-class ESAccessKeys(client: Client, config: StorageClientConfig, index:
String)
+class ESAccessKeys(client: ESClient, config: StorageClientConfig, index:
String)
extends AccessKeys with Logging {
implicit val formats = DefaultFormats.lossless
private val estype = "accesskeys"
- val indices = client.admin.indices
- val indexExistResponse = indices.prepareExists(index).get
- if (!indexExistResponse.isExists) {
- indices.prepareCreate(index).get
- }
- val typeExistResponse =
indices.prepareTypesExists(index).setTypes(estype).get
- if (!typeExistResponse.isExists) {
- val json =
+ val restClient = client.open()
--- End diff --
Should we reuse the same client after it's opened? According to
https://www.elastic.co/guide/en/elasticsearch/client/java-rest/current/_initialization.html
the official recommendation is to have the REST client have the same lifecycle
as the application. I realize this pattern is used extensively in this PR, so
please let us know what you think before you go ahead and change your code.
Right now PredictionIO does not provide a cleanup step in the storage layer
when the application shuts down. This is probably a good opportunity to add.
Would highly appreciate if you would like to take a stab at that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---