From ca124354baa190593f9201dfaedfabe76ba8b43b Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Mon, 14 Sep 2015 14:34:06 -0400 Subject: [PATCH] Add error handling for elastic search. --- lms/djangoapps/teams/errors.py | 9 ++++- lms/djangoapps/teams/search_indexes.py | 19 +++++++-- lms/djangoapps/teams/tests/test_views.py | 49 ++++++++++++++++++++++++ lms/djangoapps/teams/views.py | 14 ++++++- 4 files changed, 84 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/teams/errors.py b/lms/djangoapps/teams/errors.py index 0f718d93a6..9f3b023d24 100644 --- a/lms/djangoapps/teams/errors.py +++ b/lms/djangoapps/teams/errors.py @@ -16,6 +16,11 @@ class AlreadyOnTeamInCourse(TeamAPIRequestError): pass -class ImmutableMembershipFieldException(Exception): - """An attempt was made to change an immutable field on a CourseTeamMembership model""" +class ElasticSearchConnectionError(TeamAPIRequestError): + """The system was unable to connect to the configured elasticsearch instance.""" + pass + + +class ImmutableMembershipFieldException(Exception): + """An attempt was made to change an immutable field on a CourseTeamMembership model.""" pass diff --git a/lms/djangoapps/teams/search_indexes.py b/lms/djangoapps/teams/search_indexes.py index c1acc2e3bd..588a2c7892 100644 --- a/lms/djangoapps/teams/search_indexes.py +++ b/lms/djangoapps/teams/search_indexes.py @@ -1,5 +1,8 @@ """ Search index used to load data into elasticsearch""" +import logging +from elasticsearch.exceptions import ConnectionError + from django.conf import settings from django.db.models.signals import post_delete, post_save from django.dispatch import receiver @@ -8,6 +11,7 @@ from functools import wraps from search.search_engine_base import SearchEngine +from .errors import ElasticSearchConnectionError from .serializers import CourseTeamSerializer, CourseTeam @@ -103,8 +107,11 @@ class CourseTeamIndexer(object): """ Return course team search engine (if feature is enabled). """ - if cls.search_is_enabled(): + try: return SearchEngine.get_search_engine(index=cls.INDEX_NAME) + except ConnectionError as err: + logging.error('Error connecting to elasticsearch: %s', err) + raise ElasticSearchConnectionError @classmethod def search_is_enabled(cls): @@ -119,7 +126,10 @@ def course_team_post_save_callback(**kwargs): """ Reindex object after save. """ - CourseTeamIndexer.index(kwargs['instance']) + try: + CourseTeamIndexer.index(kwargs['instance']) + except ElasticSearchConnectionError: + pass @receiver(post_delete, sender=CourseTeam, dispatch_uid='teams.signals.course_team_post_delete_callback') @@ -127,4 +137,7 @@ def course_team_post_delete_callback(**kwargs): # pylint: disable=invalid-name """ Reindex object after delete. """ - CourseTeamIndexer.remove(kwargs['instance']) + try: + CourseTeamIndexer.remove(kwargs['instance']) + except ElasticSearchConnectionError: + pass diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index cdca9ee676..f1610a6d99 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/lms/djangoapps/teams/tests/test_views.py @@ -5,6 +5,9 @@ import pytz from datetime import datetime from dateutil import parser import ddt +from elasticsearch.exceptions import ConnectionError +from mock import patch +from search.search_engine_base import SearchEngine from django.core.urlresolvers import reverse from django.conf import settings @@ -1397,3 +1400,49 @@ class TestDeleteMembershipAPI(EventTestMixin, TeamAPITestCase): def test_missing_membership(self): self.delete_membership(self.wind_team.team_id, self.users['student_enrolled'].username, 404) + + +class TestElasticSearchErrors(TeamAPITestCase): + """Test that the Team API is robust to Elasticsearch connection errors.""" + + ES_ERROR = ConnectionError('N/A', 'connection error', {}) + + @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR) + def test_list_teams(self, __): + """Test that text searches return a 503 when Elasticsearch is down. + + The endpoint should still return 200 when a search is not supplied.""" + self.get_teams_list( + expected_status=503, + data={'course_id': self.test_course_1.id, 'text_search': 'zoinks'}, + user='staff' + ) + self.get_teams_list( + expected_status=200, + data={'course_id': self.test_course_1.id}, + user='staff' + ) + + @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR) + def test_create_team(self, __): + """Test that team creation is robust to Elasticsearch errors.""" + self.post_create_team( + expected_status=200, + data=self.build_team_data(name='zoinks'), + user='staff' + ) + + @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR) + def test_delete_team(self, __): + """Test that team deletion is robust to Elasticsearch errors.""" + self.delete_team(self.wind_team.team_id, 204, user='staff') + + @patch.object(SearchEngine, 'get_search_engine', side_effect=ES_ERROR) + def test_patch_team(self, __): + """Test that team updates are robust to Elasticsearch errors.""" + self.patch_team_detail( + self.wind_team.team_id, + 200, + data={'description': 'new description'}, + user='staff' + ) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index eea9e58400..07f477fb6c 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -58,7 +58,7 @@ from .serializers import ( add_team_count ) from .search_indexes import CourseTeamIndexer -from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam +from .errors import AlreadyOnTeamInCourse, ElasticSearchConnectionError, NotEnrolledInCourseForTeam TEAM_MEMBERSHIPS_PER_PAGE = 2 TOPICS_PER_PAGE = 12 @@ -293,6 +293,9 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): example, the course_id may not reference a real course or the page number may be beyond the last page. + If the server is unable to connect to Elasticsearch, and + the text_search parameter is supplied, a 503 error is returned. + **Response Values for POST** Any logged in user who has verified their email address can create @@ -366,7 +369,14 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView): return Response(error, status=status.HTTP_400_BAD_REQUEST) result_filter.update({'topic_id': topic_id}) if text_search and CourseTeamIndexer.search_is_enabled(): - search_engine = CourseTeamIndexer.engine() + try: + search_engine = CourseTeamIndexer.engine() + except ElasticSearchConnectionError: + return Response( + build_api_error(ugettext_noop('Error connecting to elasticsearch')), + status=status.HTTP_503_SERVICE_UNAVAILABLE + ) + result_filter.update({'course_id': course_id_string}) search_results = search_engine.search(