Merge pull request #850 from uktrade/develop

Staging release

CHANGELOG.md

@@ -1,17 +1,30 @@
 # Changelog
 
 ## Pre-release
+### Implemented enhancements
+### Fixed bugs
+
+
+## [2020.06.15](https://github.com/uktrade/directory-cms/releases/tag/2020.06.15)
+[Full Changelog](https://github.com/uktrade/directory-cms/compare/2020.06.12...2020.06.15)
+
+### Hotfix
+- No ticket - Stop using signal.alarm for bailing out of expensive code
+
+
+## [2020.06.12](https://github.com/uktrade/directory-cms/releases/tag/2020.06.12)
+[Full Changelog](https://github.com/uktrade/directory-cms/compare/2020.05.28...2020.06.12)
 
 ### Implemented enhancements
 - No ticket - Improve caching of markets endpoint
 
-## Fixed bugs
+### Fixed bugs
 - No ticket - Upgrade django to fix security vulnerability
 
 ## [2020.05.28](https://github.com/uktrade/directory-cms/releases/tag/2020.05.28)
 [Full Changelog](https://github.com/uktrade/directory-cms/compare/2020.05.26...2020.05.28)
 
-## Fixed bugs
+### Fixed bugs
 - No ticket - Upgraded dependencies to fix security vulnerability
 - No ticket - Expedite non-success response during cache miss
 

Procfile

@@ -1,3 +1,3 @@
-web: python manage.py distributed_migrate --noinput && gunicorn conf.wsgi --bind 0.0.0.0:$PORT
+web: python manage.py distributed_migrate --noinput && gunicorn conf.wsgi --worker-class gevent --worker-connections 1000 --bind 0.0.0.0:$PORT
 celery_worker: FEATURE_ENFORCE_STAFF_SSO_ENABLED=False celery -A conf worker -l info
 celery_beat: FEATURE_ENFORCE_STAFF_SSO_ENABLED=False celery -A conf beat -l info --scheduler django_celery_beat.schedulers:DatabaseScheduler

conf/wsgi.py

@@ -11,6 +11,10 @@
 
 from django.core.wsgi import get_wsgi_application
 
+import psycogreen.gevent
+psycogreen.gevent.patch_psycopg()
+
+
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "conf.settings")
 
 application = get_wsgi_application()

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']

requirements.in

@@ -2,7 +2,6 @@ django==2.2.*
 djangorestframework==3.9.*
 django-environ==0.4.5
 gunicorn==19.5.0
-gevent==1.2.2
 sentry-sdk==0.13.4
 django_storages==1.7.1
 whitenoise==4.1.2
@@ -36,4 +35,6 @@ notifications-python-client==5.3.*
 pillow>=6.*  # for security fix. check compatibility on next wagtail upgrade
 num2words==0.5.10
 pycountry==19.8.18
-elastic-apm>=5.5.2,<6.0.0
+elastic-apm>=5.5.2,<6.0.0
+gevent>=20.6.1,<21.0.0
+psycogreen>=1.0.2,<2.0.0

requirements.txt

@@ -39,9 +39,9 @@ djangorestframework==3.9.4  # via -r requirements.in, sigauth, wagtail
 docopt==0.6.2             # via notifications-python-client, num2words
 docutils==0.16            # via botocore
 draftjs-exporter==2.1.7   # via wagtail
-elastic-apm==5.6.0        # via -r requirements.in
+elastic-apm==5.7.0        # via -r requirements.in
 future==0.18.2            # via celery, notifications-python-client
-gevent==1.2.2             # via -r requirements.in
+gevent==20.6.1            # via -r requirements.in
 greenlet==0.4.16          # via gevent
 gunicorn==19.5.0          # via -r requirements.in
 html2text==2018.1.9       # via -r requirements.in
@@ -59,6 +59,7 @@ num2words==0.5.10         # via -r requirements.in
 oauthlib==3.1.0           # via requests-oauthlib
 packaging==20.4           # via bleach
 pillow==6.2.2             # via -r requirements.in, wagtail
+psycogreen==1.0.2         # via -r requirements.in
 psycopg2==2.7.3.2         # via -r requirements.in
 pycountry==19.8.18        # via -r requirements.in
 pyjwt==1.7.1              # via notifications-python-client
@@ -87,6 +88,8 @@ webencodings==0.5.1       # via bleach, html5lib
 whitenoise==4.1.2         # via -r requirements.in
 willow==1.3               # via wagtail
 zipp==3.1.0               # via importlib-metadata
+zope.event==4.4           # via gevent
+zope.interface==5.1.0     # via gevent
 
 # The following packages are considered to be unsafe in a requirements file:
 # setuptools

requirements_test.txt

@@ -43,13 +43,13 @@ djangorestframework==3.9.4  # via -r requirements.in, sigauth, wagtail
 docopt==0.6.2             # via coveralls, notifications-python-client, num2words
 docutils==0.16            # via botocore
 draftjs-exporter==2.1.7   # via wagtail
-elastic-apm==5.6.0        # via -r requirements.in
+elastic-apm==5.7.0        # via -r requirements.in
 factory-boy==2.12.0       # via -r requirements_test.in, wagtail-factories
 faker==4.1.0              # via factory-boy
-flake8==3.8.2             # via -r requirements_test.in
+flake8==3.8.3             # via -r requirements_test.in
 freezegun==0.3.14         # via -r requirements_test.in
 future==0.18.2            # via celery, notifications-python-client
-gevent==1.2.2             # via -r requirements.in
+gevent==20.6.1            # via -r requirements.in
 greenlet==0.4.16          # via gevent
 gunicorn==19.5.0          # via -r requirements.in
 html2text==2018.1.9       # via -r requirements.in
@@ -69,8 +69,9 @@ num2words==0.5.10         # via -r requirements.in
 oauthlib==3.1.0           # via requests-oauthlib
 packaging==20.4           # via bleach, pytest, pytest-sugar
 pillow==6.2.2             # via -r requirements.in, wagtail
-pip-tools==5.2.0          # via -r requirements_test.in
+pip-tools==5.2.1          # via -r requirements_test.in
 pluggy==0.13.1            # via pytest
+psycogreen==1.0.2         # via -r requirements.in
 psycopg2==2.7.3.2         # via -r requirements.in
 py==1.8.1                 # via pytest
 pycodestyle==2.6.0        # via flake8
@@ -111,6 +112,8 @@ webencodings==0.5.1       # via bleach, html5lib
 whitenoise==4.1.2         # via -r requirements.in
 willow==1.3               # via wagtail
 zipp==3.1.0               # via importlib-metadata
+zope.event==4.4           # via gevent
+zope.interface==5.1.0     # via gevent
 
 # The following packages are considered to be unsafe in a requirements file:
 # pip

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={