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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to