Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions availability/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,27 @@
NUMBER_OF_SHARDS = 2


class StorageException(Exception):
pass


def get_elasticsearch(check_availability=False):
"""Return Elasticsearch instance.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method must raise Exception. It's very important.

Code that is using this method should handle exception

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

:param check_availability: check if nodes are available
:returns: Elasticsearch or None on failure
:rtype: elasticsearch.Elasticsearch
:returns: elasticsearch.Elasticsearch
:raises: StorageException
"""
nodes = config.get_config()["backend"]["connection"]
try:
es = elasticsearch.Elasticsearch(nodes)
if check_availability:
es.info()
except Exception as e:
LOG.warning(
"Failed to query Elasticsearch nodes %s: %s"
% (nodes, str(e)))
raise
mesg = ("Failed to query Elasticsearch nodes %s: %s"
% (nodes, str(e)))
LOG.error(mesg)
raise StorageException(mesg)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like ExceptionSpoofing antipattern

return es


Expand Down
5 changes: 3 additions & 2 deletions availability/watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,9 @@ def main(period=None):
LOG.error("Unexpected backend: %(type)s" % backend)
return 1

if not storage.get_elasticsearch(check_availability=True):
LOG.error("Failed to set up Elasticsearch")
try:
storage.get_elasticsearch(check_availability=True)
except storage.StorageException:
return 1

LOG.info("Start watching with period %s seconds" % period)
Expand Down
9 changes: 4 additions & 5 deletions tests/unit/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,15 @@ def test_ensure_es_index_exists(
@mock.patch("availability.storage.LOG")
def test_get_elasticsearch(self, mock_log, mock_config, mock_elastic):
mock_es = mock.Mock()
mock_es.indices.exists.side_effect = ValueError
mock_elastic.return_value = mock_es
mock_config.get_config.return_value = (
{"backend": {"connection": "nodes"}})
self.assertFalse(mock_es.info.called)
self.assertEqual(mock_es, storage.get_elasticsearch())
self.assertFalse(mock_es.info.called)

mock_es.indices.exists.side_effect = None
mock_elastic.reset_mock()
self.assertEqual(mock_es, storage.get_elasticsearch(
check_availability=True))
mock_es.info.side_effect = ValueError
self.assertRaises(storage.StorageException,
storage.get_elasticsearch, check_availability=True)
mock_elastic.assert_called_once_with("nodes")
mock_es.info.assert_called_once_with()
16 changes: 8 additions & 8 deletions tests/unit/test_watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import mock
import requests

from availability import storage
from availability import watcher
from tests.unit import test

Expand Down Expand Up @@ -134,13 +135,13 @@ def test_watch_services(self, mock_log, mock_queue, mock_thread,
self.assertEqual(calls, mock_thread.mock_calls)

@mock.patch("availability.watcher.schedule")
@mock.patch("availability.watcher.storage")
@mock.patch("availability.watcher.storage.get_elasticsearch")
@mock.patch("availability.watcher.watch_services")
@mock.patch("availability.watcher.config.get_config")
@mock.patch("availability.watcher.time")
@mock.patch("availability.watcher.LOG")
def test_main(self, mock_log, mock_time, mock_get_config,
mock_watch_services, mock_storage, mock_schedule):
mock_watch_services, mock_get_elastic, mock_schedule):
class BreakInfinityCicle(Exception):
pass

Expand All @@ -166,19 +167,18 @@ class BreakInfinityCicle(Exception):
backend = {"type": "elastic", "connection": "foo_conn"}
mock_get_config.return_value = {"regions": ["foo_region"],
"backend": backend}
mock_storage.get_elasticsearch.return_value = None
mock_get_elastic.side_effect = storage.StorageException
self.assertEqual(1, watcher.main())
mock_storage.get_elasticsearch.assert_called_once_with(
mock_get_elastic.assert_called_once_with(
check_availability=True)

mock_storage.reset_mock()
mock_storage.get_elasticsearch.return_value = mock.Mock()
mock_get_elastic.reset_mock()
mock_get_elastic.side_effect = None

self.assertRaises(BreakInfinityCicle, watcher.main)
self.assertEqual([mock.call(1)] * 4, mock_time.sleep.mock_calls)
mock_schedule.every.assert_called_once_with(60)
mock_storage.get_elasticsearch.assert_called_once_with(
check_availability=True)
mock_get_elastic.assert_called_once_with(check_availability=True)

mock_get_config.return_value = {"regions": ["foo_region"],
"backend": backend, "period": 42}
Expand Down