diff --git a/search/search_service/config.py b/search/search_service/config.py index 30d7e9da42..014eec8a5e 100644 --- a/search/search_service/config.py +++ b/search/search_service/config.py @@ -1,10 +1,6 @@ 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' @@ -12,6 +8,7 @@ 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') diff --git a/search/search_service/proxy/__init__.py b/search/search_service/proxy/__init__.py index 933b6ea2a7..6ba0245b85 100644 --- a/search/search_service/proxy/__init__.py +++ b/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 diff --git a/search/search_service/proxy/atlas.py b/search/search_service/proxy/atlas.py index 0f99616232..c5f376fef4 100644 --- a/search/search_service/proxy/atlas.py +++ b/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 diff --git a/search/search_service/proxy/elasticsearch.py b/search/search_service/proxy/elasticsearch.py index 9ce2e248fe..ed2861c71b 100644 --- a/search/search_service/proxy/elasticsearch.py +++ b/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,21 +41,21 @@ 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} @@ -68,7 +63,7 @@ def _create_client_from_credentials(*, 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 diff --git a/search/setup.py b/search/setup.py index 0c233b62e8..0b40e83e86 100644 --- a/search/setup.py +++ b/search/setup.py @@ -1,6 +1,6 @@ from setuptools import setup, find_packages -__version__ = '1.0.1' +__version__ = '1.0.2' setup( diff --git a/search/tests/unit/proxy/test_elasticsearch.py b/search/tests/unit/proxy/test_elasticsearch.py index 385b4cbeca..b7f2ac9ba1 100644 --- a/search/tests/unit/proxy/test_elasticsearch.py +++ b/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: diff --git a/search/tests/unit/proxy/test_statsd_utilities.py b/search/tests/unit/proxy/test_statsd_utilities.py index dce818fa28..17bb7c4288 100644 --- a/search/tests/unit/proxy/test_statsd_utilities.py +++ b/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()