Fix #24, correct initialisation of elastic search (#27)

* Fix #24, correct initialisation of elastic search

* Update setup.py

search/search_service/config.py

@@ -1,17 +1,14 @@
 import os
 
-ELASTICSEARCH_ENDPOINT_KEY = 'ELASTICSEARCH_ENDPOINT'
 ELASTICSEARCH_INDEX_KEY = 'ELASTICSEARCH_INDEX'
-ELASTICSEARCH_AUTH_USER_KEY = 'ELASTICSEARCH_AUTH_USER'
-ELASTICSEARCH_AUTH_PW_KEY = 'ELASTICSEARCH_AUTH_PW'
-ELASTICSEARCH_CLIENT_KEY = 'ELASTICSEARCH_CLIENT'
 SEARCH_PAGE_SIZE_KEY = 'SEARCH_PAGE_SIZE'
 STATS_FEATURE_KEY = 'STATS'
 
 PROXY_ENDPOINT = 'PROXY_ENDPOINT'
 PROXY_USER = 'PROXY_USER'
 PROXY_PASSWORD = 'PROXY_PASSWORD'
 PROXY_CLIENT = 'PROXY_CLIENT'
+PROXY_CLIENT_KEY = 'PROXY_CLIENT_KEY'
 PROXY_CLIENTS = {
     'ELASTICSEARCH': 'search_service.proxy.elasticsearch.ElasticsearchProxy',
     'ATLAS': 'search_service.proxy.atlas.AtlasProxy'
@@ -51,5 +48,6 @@ class LocalConfig(Config):
                                         PORT=PROXY_PORT)
                                     )
     PROXY_CLIENT = PROXY_CLIENTS[os.environ.get('PROXY_CLIENT', 'ELASTICSEARCH')]
+    PROXY_CLIENT_KEY = os.environ.get('PROXY_CLIENT_KEY')
     PROXY_USER = os.environ.get('CREDENTIALS_PROXY_USER', 'elastic')
     PROXY_PASSWORD = os.environ.get('CREDENTIALS_PROXY_PASSWORD', 'elastic')

search/search_service/proxy/__init__.py

@@ -9,6 +9,8 @@
 _proxy_client = None
 _proxy_client_lock = Lock()
 
+DEFAULT_PAGE_SIZE = 10
+
 
 def get_proxy_client() -> BaseProxy:
     """
@@ -24,12 +26,17 @@ def get_proxy_client() -> BaseProxy:
         if _proxy_client:
             return _proxy_client
         else:
+            obj = current_app.config[config.PROXY_CLIENT_KEY]
+
             # Gather all the configuration to create a Proxy Client
             host = current_app.config[config.PROXY_ENDPOINT]
             user = current_app.config[config.PROXY_USER]
             password = current_app.config[config.PROXY_PASSWORD]
-
             client = import_string(current_app.config[config.PROXY_CLIENT])
-            _proxy_client = client(host=host, index=None, user=user, password=password)
+
+            # number of results per search page
+            page_size = current_app.config.get(config.SEARCH_PAGE_SIZE_KEY, DEFAULT_PAGE_SIZE)
+
+            _proxy_client = client(host=host, index=None, user=user, password=password, client=obj, page_size=page_size)
 
     return _proxy_client

search/search_service/proxy/atlas.py

@@ -12,7 +12,6 @@
 from search_service.proxy import BaseProxy
 from search_service.proxy.statsd_utilities import timer_with_counter
 
-DEFAULT_PAGE_SIZE = 10
 LOGGER = logging.getLogger(__name__)
 
 
@@ -33,7 +32,7 @@ def __init__(self, *,
                  index: str = None,
                  user: str = '',
                  password: str = '',
-                 page_size: int = DEFAULT_PAGE_SIZE) -> None:
+                 page_size: int = 10) -> None:
         self.atlas = Atlas(host, username=user, password=password)
         self.index = index
         self.page_size = page_size

search/search_service/proxy/elasticsearch.py

@@ -1,11 +1,8 @@
-from typing import List  # noqa: F401
-from threading import Lock
 import logging
 import re
 
 from elasticsearch import Elasticsearch
 from elasticsearch_dsl import Search, query
-
 from flask import current_app
 
 from search_service import config
@@ -17,9 +14,6 @@
 # Default Elasticsearch index to use, if none specified
 DEFAULT_ES_INDEX = 'table_search_index'
 
-# default search page size
-DEFAULT_PAGE_SIZE = 10
-
 LOGGING = logging.getLogger(__name__)
 
 
@@ -29,11 +23,12 @@ class ElasticsearchProxy(BaseProxy):
     """
     def __init__(self, *,
                  host: str = None,
+                 user: str = '',
                  index: str = None,
-                 auth_user: str = '',
-                 auth_pw: str = '',
-                 elasticsearch_client: Elasticsearch = None,
-                 page_size: int = DEFAULT_PAGE_SIZE) -> None:
+                 password: str = '',
+                 client: Elasticsearch = None,
+                 page_size: int = 10
+                 ) -> None:
         """
         Constructs Elasticsearch client for interactions with the cluster.
         Allows caller to pass a fully constructed Elasticsearch client, {elasticsearch_client}
@@ -46,29 +41,29 @@ def __init__(self, *,
         :param elasticsearch_client: Elasticsearch client to use, if provided
         :param  page_size: Number of search results to return per request
         """
-        if elasticsearch_client:
-            self.elasticsearch = elasticsearch_client
+        if client:
+            self.elasticsearch = client
         else:
             self.elasticsearch = self._create_client_from_credentials(host=host,
-                                                                      auth_user=auth_user,
-                                                                      auth_pw=auth_pw)
+                                                                      user=user,
+                                                                      password=password)
 
-        self.index = index
+        self.index = index or current_app.config.get(config.ELASTICSEARCH_INDEX_KEY, DEFAULT_ES_INDEX)
         self.page_size = page_size
 
     @staticmethod
     def _create_client_from_credentials(*,
                                         host: str = None,
-                                        auth_user: str = '',
-                                        auth_pw: str = '') -> Elasticsearch:
+                                        user: str = '',
+                                        password: str = '') -> Elasticsearch:
         """
         Construct Elasticsearch client that connects to cluster at {host}
         and authenticates using {auth_user} and {auth_pw}
         Uses default {ConnectionPool} and {Transport} class in constructing
         the client
         :return: Elasticsearch client object
         """
-        return Elasticsearch(host, http_auth=(auth_user, auth_pw))
+        return Elasticsearch(host, http_auth=(user, password))
 
     def _get_search_result(self, page_index: int,
                            client: Search) -> SearchResult:
@@ -243,48 +238,3 @@ def fetch_search_results(self, *,
         return self._search_helper(query_term=query_term,
                                    page_index=page_index,
                                    client=s)
-
-
-_elasticsearch_proxy = None
-_elasticsearch_lock = Lock()
-
-
-def get_elasticsearch_proxy() -> ElasticsearchProxy:
-    """
-    Fetch ElasticSearch proxy instance. Use a lock to create an instance
-    if one doesn't exist
-    :return: ElasticSearchProxy instance
-    """
-    global _elasticsearch_proxy
-
-    # elasticsearch cluster host to connect to
-    host = current_app.config.get(config.ELASTICSEARCH_ENDPOINT_KEY, None)
-
-    # elasticsearch index
-    index = current_app.config.get(config.ELASTICSEARCH_INDEX_KEY, DEFAULT_ES_INDEX)
-
-    # user name and password to connect to elasticsearch cluster
-    auth_user = current_app.config.get(config.ELASTICSEARCH_AUTH_USER_KEY, '')
-    auth_pw = current_app.config.get(config.ELASTICSEARCH_AUTH_PW_KEY, '')
-
-    # fully constructed client object to use, if provided
-    elasticsearch_client = current_app.config.get(config.ELASTICSEARCH_CLIENT_KEY, None)
-
-    # number of results per search page
-    page_size = current_app.config.get(config.SEARCH_PAGE_SIZE_KEY, DEFAULT_PAGE_SIZE)
-
-    if _elasticsearch_proxy:
-        return _elasticsearch_proxy
-
-    with _elasticsearch_lock:
-        if _elasticsearch_proxy:
-            return _elasticsearch_proxy
-        else:
-            _elasticsearch_proxy = ElasticsearchProxy(host=host,
-                                                      index=index,
-                                                      auth_user=auth_user,
-                                                      auth_pw=auth_pw,
-                                                      elasticsearch_client=elasticsearch_client,
-                                                      page_size=page_size)
-
-    return _elasticsearch_proxy

search/setup.py

@@ -1,6 +1,6 @@
 from setuptools import setup, find_packages
 
-__version__ = '1.0.1'
+__version__ = '1.0.2'
 
 
 setup(

search/tests/unit/proxy/test_elasticsearch.py

@@ -1,8 +1,10 @@
 import unittest
 from unittest.mock import patch, MagicMock
+
 from typing import Iterable
 
 from search_service import create_app
+from search_service.proxy import get_proxy_client
 from search_service.proxy.elasticsearch import ElasticsearchProxy
 from search_service.models.search_result import SearchResult
 from search_service.models.table import Table
@@ -38,7 +40,7 @@ def setUp(self) -> None:
         self.app_context.push()
 
         mock_elasticsearch_client = MagicMock()
-        self.es_proxy = ElasticsearchProxy(elasticsearch_client=mock_elasticsearch_client)
+        self.es_proxy = ElasticsearchProxy(client=mock_elasticsearch_client)
 
         self.mock_result1 = MockSearchResult(table_name='test_table',
                                              table_key='test_key',
@@ -70,6 +72,27 @@ def setUp(self) -> None:
                                   tags=['match'],
                                   last_updated_epoch=1527283287)
 
+    def test_setup_client(self) -> None:
+        self.es_proxy = ElasticsearchProxy(
+            host="http://0.0.0.0:9200",
+            index="random123",
+            user="elastic",
+            password="elastic"
+        )
+        a = self.es_proxy.elasticsearch
+        for client in [a, a.cat, a.cluster, a.indices, a.ingest, a.nodes, a.snapshot, a.tasks]:
+            self.assertEqual(client.transport.hosts[0]['host'], "0.0.0.0")
+            self.assertEqual(client.transport.hosts[0]['port'], 9200)
+        self.assertEqual(self.es_proxy.index, "random123")
+
+    def test_setup_config(self) -> None:
+        es: ElasticsearchProxy = get_proxy_client()
+        a = es.elasticsearch
+        for client in [a, a.cat, a.cluster, a.indices, a.ingest, a.nodes, a.snapshot, a.tasks]:
+            self.assertEqual(client.transport.hosts[0]['host'], "0.0.0.0")
+            self.assertEqual(client.transport.hosts[0]['port'], 9200)
+        self.assertEqual(self.es_proxy.index, "table_search_index")
+
     @patch('elasticsearch_dsl.Search.execute')
     def test_search_with_empty_query_string(self, mock_search: MagicMock) -> None:
 

search/tests/unit/proxy/test_statsd_utilities.py

@@ -60,7 +60,7 @@ def test_with_elasticsearch_proxy(self,
                                       mock_search: MagicMock) -> None:
 
         mock_elasticsearch_client = MagicMock()
-        es_proxy = ElasticsearchProxy(elasticsearch_client=mock_elasticsearch_client)
+        es_proxy = ElasticsearchProxy(client=mock_elasticsearch_client)
 
         with patch.object(statsd_utilities, '_get_statsd_client') as mock_statsd_client:
             mock_success_incr = MagicMock()