Merge pull request #132 from edx/jsa/tweak-health

Don’t let LMS take us down during health checks.

ecommerce/health/tests/test_views.py

 """Tests of the service health endpoint."""
 import json
-import logging
 
+from django.core.urlresolvers import reverse
+from django.db import DatabaseError
+from django.test import TestCase
 import mock
 from requests import Response
 from requests.exceptions import RequestException
 from rest_framework import status
-from django.test import TestCase
-from django.db import DatabaseError
-from django.core.urlresolvers import reverse
+from testfixtures import LogCapture
+
+from ecommerce.health.constants import Status, UnavailabilityMessage
 
-from ecommerce.health.constants import Status
+LOGGER_NAME = 'ecommerce.health.views'
 
 
 @mock.patch('requests.get')
@@ -19,12 +21,6 @@ class HealthTests(TestCase):
     def setUp(self):
         self.fake_lms_response = Response()
 
-        # Override all loggers, suppressing logging calls of severity CRITICAL and below
-        logging.disable(logging.CRITICAL)
-
-        # Remove logger override
-        self.addCleanup(logging.disable, logging.NOTSET)
-
     def test_all_services_available(self, mock_lms_request):
         """Test that the endpoint reports when all services are healthy."""
         self.fake_lms_response.status_code = status.HTTP_200_OK
@@ -46,27 +42,31 @@ class HealthTests(TestCase):
         )
 
     def test_lms_outage(self, mock_lms_request):
-        """Test that the endpoint reports when the LMS is experiencing difficulties."""
+        """Test that the endpoint reports when the LMS is unhealthy."""
         self.fake_lms_response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE
         mock_lms_request.return_value = self.fake_lms_response
 
-        self._assert_health(
-            status.HTTP_503_SERVICE_UNAVAILABLE,
-            Status.UNAVAILABLE,
-            Status.OK,
-            Status.UNAVAILABLE
-        )
+        with LogCapture(LOGGER_NAME) as l:
+            self._assert_health(
+                status.HTTP_200_OK,
+                Status.OK,
+                Status.OK,
+                Status.UNAVAILABLE
+            )
+            l.check((LOGGER_NAME, 'CRITICAL', UnavailabilityMessage.LMS))
 
     def test_lms_connection_failure(self, mock_lms_request):
         """Test that the endpoint reports when it cannot contact the LMS."""
         mock_lms_request.side_effect = RequestException
 
-        self._assert_health(
-            status.HTTP_503_SERVICE_UNAVAILABLE,
-            Status.UNAVAILABLE,
-            Status.OK,
-            Status.UNAVAILABLE
-        )
+        with LogCapture(LOGGER_NAME) as l:
+            self._assert_health(
+                status.HTTP_200_OK,
+                Status.OK,
+                Status.OK,
+                Status.UNAVAILABLE
+            )
+            l.check((LOGGER_NAME, 'CRITICAL', UnavailabilityMessage.LMS))
 
     def _assert_health(self, status_code, overall_status, database_status, lms_status):
         """Verify that the response matches expectations."""

ecommerce/health/views.py

@@ -48,7 +48,7 @@ def health(_):
         database_status = Status.UNAVAILABLE
 
     try:
-        response = requests.get(LMS_HEALTH_PAGE)
+        response = requests.get(LMS_HEALTH_PAGE, timeout=1)
 
         if response.status_code == status.HTTP_200_OK:
             lms_status = Status.OK
@@ -59,7 +59,7 @@ def health(_):
         logger.critical(UnavailabilityMessage.LMS)
         lms_status = Status.UNAVAILABLE
 
-    overall_status = Status.OK if (database_status == lms_status == Status.OK) else Status.UNAVAILABLE
+    overall_status = Status.OK if (database_status == Status.OK) else Status.UNAVAILABLE
 
     data = {
         'overall_status': overall_status,