Merge pull request #846 from uktrade/hotfix-remove-timeout-alert

Hotfix: Remove signal alert

core/cache.py

@@ -118,7 +118,8 @@ def populate(instance_pk):
 
     @classmethod
     def populate_async(cls, instance):
-        cls.populate.delay(instance.pk)
+        if instance.__class__ in MODELS_SERIALIZERS_MAPPING and instance.__class__ is not Page:
+            cls.populate.delay(instance.pk)
 
     @classmethod
     def populate_sync(cls, instance):
@@ -372,6 +373,5 @@ def delete(cls, sender, instance, *args, **kwargs):
 @app.task
 def rebuild_all_cache():
     for page in Page.objects.live().specific():
-        if page.__class__ in MODELS_SERIALIZERS_MAPPING and page.__class__ is not Page:
-            CachePopulator.populate_async(page)
+        CachePopulator.populate_async(page)
     MarketPagesCachePopulator.populate()

core/helpers.py

@@ -1,7 +1,6 @@
 from num2words import num2words
 import copy
 import os
-import signal
 from urllib.parse import urljoin
 
 import bleach
@@ -246,19 +245,3 @@ def get_page_full_url(domain, full_path):
 
 def num2words_list(length):
     return list(map(num2words, list(range(1, length + 1))))
-
-
-class timeout:
-    def __init__(self, seconds=2, error_class=TimeoutError):
-        self.seconds = seconds
-        self.error_class = error_class
-
-    def handle_timeout(self, signum, frame):
-        raise self.error_class
-
-    def __enter__(self):
-        signal.signal(signal.SIGALRM, self.handle_timeout)
-        signal.alarm(self.seconds)
-
-    def __exit__(self, type, value, traceback):
-        signal.alarm(0)

core/views.py

@@ -27,14 +27,6 @@
 logger = getLogger(__name__)
 
 
-class PageNotSerializableError(NotImplementedError):
-    pass
-
-
-class PageSerializationTooExpensiveError(TimeoutError):
-    pass
-
-
 class APIEndpointBase(PagesAdminAPIEndpoint):
     """At the very deep core this is a DRF GenericViewSet, with a few wagtail
     layers on top.
@@ -50,15 +42,6 @@ class APIEndpointBase(PagesAdminAPIEndpoint):
         )
     )
 
-    def handle_exception(self, exc):
-        if isinstance(exc, PageNotSerializableError):
-            # page that exists has been requested, but it's not serializable. E.g, it's a folder page
-            return Response(status=204)
-        if isinstance(exc, PageSerializationTooExpensiveError):
-            # the request took too long so the code manually bailed out by raising a timeout
-            return Response(status=501)
-        return super().handle_exception(exc)
-
     def get_model_class(self):
         if self.action == 'listing_view':
             model = self.get_queryset().model
@@ -107,30 +90,38 @@ def detail_view(self, request, **kwargs):
         # Exit early if there are any issues
         self.check_parameter_validity()
 
-        variation_kwargs = {'lang': translation.get_language()}
+        draft_version = helpers.is_draft_requested(request)
 
-        if helpers.is_draft_requested(request):
-            variation_kwargs['draft_version'] = True
+        langs = []
+        if translation.get_language():
+            langs.append(translation.get_language())
+        langs.append('en-gb')
 
         # Return a cached response if one is available
-        cached_data = cache.PageCache.get(page_id=self.object_id, **variation_kwargs)
-        if cached_data:
-            cached_response = helpers.CachedResponse(cached_data)
-            cached_response['etag'] = cached_data.get('etag', None)
-            return get_conditional_response(
-                request=request,
-                etag=cached_response['etag'],
-                response=cached_response,
-            )
+        for lang in langs:
+            cached_data = cache.PageCache.get(page_id=self.object_id, lang=lang, draft_version=draft_version)
+            if cached_data:
+                cached_response = helpers.CachedResponse(cached_data)
+                cached_response['etag'] = cached_data.get('etag', None)
+                return get_conditional_response(
+                    request=request,
+                    etag=cached_response['etag'],
+                    response=cached_response,
+                )
 
         logger.warn('Page cache miss')
 
-        cache.CachePopulator.populate_async(self.get_object())
+        page = self.get_object()
+
+        if type(page) not in MODELS_SERIALIZERS_MAPPING:
+            # page that exists has been requested, but it's not serializable. E.g, it's a folder page
+            return Response(status=204)
+        else:
+            cache.CachePopulator.populate_async(self.get_object())
 
         # super().detail_view can take several seconds due to unoptimized database reads - so lots of
-        # resources get used but ultimately will timeout. So here we attempt, but bail out after 2 seconds
-        with helpers.timeout(seconds=2, error_class=PageSerializationTooExpensiveError):
-            return super().detail_view(request, pk=None)
+        # resources get used but ultimately will timeout
+        return Response(status=501)
 
     def get_serializer_context(self):
         context = super().get_serializer_context()
@@ -148,12 +139,6 @@ def listing_view(self, request):
     def object_id(self):
         return self.kwargs['pk']
 
-    def get_serializer_class(self):
-        model_class = self.get_model_class()
-        if model_class not in MODELS_SERIALIZERS_MAPPING:
-            raise PageNotSerializableError(model_class.__name__)
-        return super().get_serializer_class()
-
 
 class DetailViewEndpointBase(APIEndpointBase):
     detail_only_fields = ['id']

tests/core/test_cache.py

@@ -242,6 +242,6 @@ def test_rebuild_all_cache_task(mock_cache_populator):
     article2 = InternationalArticlePageFactory(live=True)
     InternationalArticlePageFactory(live=False)
     cache.rebuild_all_cache()
-    assert mock_cache_populator.populate_async.call_count == 3  # contains home page
+    assert mock_cache_populator.populate_async.call_count == 5  # contains home page
     assert mock_cache_populator.populate_async.call_args_list[-2] == call(article1)
     assert mock_cache_populator.populate_async.call_args_list[-1] == call(article2)

tests/core/test_views.py

@@ -65,6 +65,8 @@ def test_permissions_published(rf):
 def test_api_translations_are_loaded_when_available(
     client, translated_page, site_with_translated_page_as_root, language_code, expected_title
 ):
+    cache.rebuild_all_cache()
+
     # to be added as a query params to all requests
     languge_query_params = {'lang': language_code}
 
@@ -99,6 +101,7 @@ def test_api_translations_are_loaded_when_available(
 def test_api_falls_back_to_english_when_translations_unavailable(
     client, untranslated_page, site_with_untranslated_page_as_root, language_code
 ):
+    cache.rebuild_all_cache()
     # to be added as a query params to all requests
     languge_query_params = {'lang': language_code}
 
@@ -134,9 +137,8 @@ def test_api_falls_back_to_english_when_translations_unavailable(
 
 
 @pytest.mark.django_db
-def test_api_serves_drafts(
-    client, page_with_reversion, site_with_revised_page_as_root
-):
+def test_api_serves_drafts(client, page_with_reversion, site_with_revised_page_as_root):
+    cache.rebuild_all_cache()
     # For applying the draft token as a query param for each request
     param_name = permissions.DraftTokenPermisison.TOKEN_PARAM
     draft_query_params = {
@@ -232,6 +234,7 @@ def test_upstream_anon(client, translated_page, image, url_name):
 def test_add_page_prepopulate(
     is_edit, expected_template, international_root_page, translated_page, admin_client, image, cluster_data
 ):
+    cache.rebuild_all_cache()
     cache.PageIDCache.populate()
     url = reverse('preload-add-page')
     model_as_dict = model_to_dict(translated_page, exclude=[
@@ -324,6 +327,8 @@ def test_page_listing(translated_page, admin_client):
 
 @pytest.mark.django_db
 def test_translations_exposed(translated_page, settings, client):
+    cache.rebuild_all_cache()
+
     url = reverse('api:api:pages:detail', kwargs={'pk': translated_page.pk})
 
     response = client.get(url)
@@ -340,6 +345,7 @@ def test_unserializable_page_requested(settings, client):
         depth=2,
         path='/thing',
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': page.pk})
 
@@ -354,6 +360,8 @@ def test_lookup_by_path(international_root_page, page, admin_client):
     parent_page = InternationalSectorPageFactory(parent=international_root_page)
     page.move(target=parent_page, pos='last-child')
 
+    cache.rebuild_all_cache()
+
     # to lookup page, the path should include the parent's slug and
     # the page's slug, but NOT that of app_root_page
     path = '/'.join([parent_page.slug, page.slug])
@@ -389,6 +397,8 @@ def test_lookup_by_path_for_non_existent_page(client):
 
 @pytest.mark.django_db
 def test_lookup_by_slug(translated_page, admin_client):
+    cache.rebuild_all_cache()
+
     url = reverse(
         'api:lookup-by-slug',
         kwargs={

tests/export_readiness/test_views.py

@@ -5,7 +5,7 @@
 
 from tests.export_readiness import factories
 from directory_constants import urls
-from core.cache import MarketPagesCachePopulator
+from core import cache
 
 
 @pytest.mark.django_db
@@ -15,6 +15,7 @@ def test_performance_dashboard(admin_client, root_page):
         parent=root_page,
         product_link=urls.SERVICES_GREAT_DOMESTIC
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': page.pk})
 
@@ -29,6 +30,7 @@ def test_performance_dashboard_notes(admin_client, root_page):
         live=True,
         parent=root_page
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': page.pk})
 
@@ -54,6 +56,7 @@ def test_topic_landing_page_view(admin_client, root_page):
         parent=article_listing_page,
         live=True
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': topic_landing_page.pk})
     response = admin_client.get(url)
@@ -75,6 +78,7 @@ def test_article_listing_page_view(admin_client, root_page):
         parent=article_listing_page,
         live=True
     )
+    cache.rebuild_all_cache()
 
     url = reverse(
         'api:api:pages:detail', kwargs={'pk': article_listing_page.pk}
@@ -104,6 +108,8 @@ def test_article_page_view(admin_client, root_page):
     article.tags = [tag, tag2]
     article.save()
 
+    cache.rebuild_all_cache()
+
     url = reverse('api:api:pages:detail', kwargs={'pk': article.pk})
     response = admin_client.get(url)
     assert response.status_code == 200
@@ -117,6 +123,7 @@ def test_domestic_homepage(admin_client, root_page):
         parent=root_page,
         hero_cta_url='/foo/bar'
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': home_page.pk})
     response = admin_client.get(url)
@@ -133,6 +140,8 @@ def test_country_page_view(admin_client, root_page):
     tag = factories.IndustryTagFactory(name='test')
     page.tags = [tag]
     page.save()
+    cache.rebuild_all_cache()
+
     url = reverse('api:api:pages:detail', kwargs={'pk': page.pk})
     response = admin_client.get(url)
     assert response.status_code == 200
@@ -166,6 +175,7 @@ def test_topic_landing_page_view_country_guides_alph_order(
         live=True,
         heading='foo'
     )
+    cache.rebuild_all_cache()
 
     url = reverse('api:api:pages:detail', kwargs={'pk': topic_landing_page.pk})
     response = admin_client.get(url)
@@ -232,7 +242,7 @@ def test_lookup_market_guides_missing_filters(client):
     market2 = factories.CountryGuidePageFactory()
     market2.tags = [tag]
     market2.save_revision().publish()
-    MarketPagesCachePopulator.populate()
+    cache.MarketPagesCachePopulator.populate()
 
     url = reverse('api:lookup-country-guides-list-view')
 
@@ -251,7 +261,7 @@ def test_lookup_market_guides_missing_region(client):
     market2 = factories.CountryGuidePageFactory()
     market2.tags = [tag]
     market2.save_revision().publish()
-    MarketPagesCachePopulator.populate()
+    cache.MarketPagesCachePopulator.populate()
 
     url = reverse('api:lookup-country-guides-list-view')
 
@@ -274,7 +284,7 @@ def test_lookup_market_guides_missing_region_markets_have_region(client):
     market2.save_revision().publish()
     factories.CountryGuidePageFactory()
 
-    MarketPagesCachePopulator.populate()
+    cache.MarketPagesCachePopulator.populate()
 
     url = reverse('api:lookup-country-guides-list-view')
     response = client.get(f'{url}?industry={tag.name},{tag2.name}')
@@ -298,7 +308,7 @@ def test_lookup_market_guides_all_filters_no_cache(client):
     market2.save_revision()
     url = reverse('api:lookup-country-guides-list-view')
 
-    MarketPagesCachePopulator.populate()
+    cache.MarketPagesCachePopulator.populate()
     response = client.get(f'{url}?industry={tag.name}&region={region.name}')
     assert response.status_code == 200
     assert len(response.json()) == 1
@@ -316,7 +326,7 @@ def test_lookup_market_guides_all_filters_cache(client):
 
     factories.CountryGuidePageFactory(country=country)
 
-    MarketPagesCachePopulator.populate()
+    cache.MarketPagesCachePopulator.populate()
     url = reverse('api:lookup-country-guides-list-view')
 
     response = client.get(f'{url}?industry={tag.name}&region={region.name}')