Merge pull request #1942 from openhealthcare/celery-upgrade

Drops support for python 3.5 and does the celery upgrade that is required to run on 3.6+

Author: davidmiller

Diff for: .travis.yml

@@ -1,7 +1,6 @@
 language: python
 dist: xenial
 python:
-  - "3.5"
   - "3.6"
   - "3.7"
   - "3.8"

Diff for: changelog.md

@@ -1,9 +1,18 @@
+### 0.21.0 (Major Release)
+
+#### Celery upgrade and Django Celery library change.
+`django-celery` only officially supports Python 3.5 which Opal no longer supports. It is also no longer the recommended solution for results by Celery. So we have removed this dependency and added the `django-celery-results` module that has a similar functionality.
+
+#### Drops support for Python 3.5
+Opal now supports Python 3.6 - Python 3.8.
+
+
 ### 0.20.0 (Major Release)
 
 #### Python 3 version changes.
 
-Opal now supports python 3.5 - python 3.8.
-Please note that the 0.21.0 release will drop support for python 3.5.
+Opal now supports Python 3.5 - Python 3.8.
+Please note that the 0.21.0 release will drop support for Python 3.5.
 
 ### Scaffold application episode category
 
@@ -19,7 +28,7 @@ per application basis is simple, and requires only settings and requirements cha
 
 ### Removes Letter
 
-The dependency on the letter python package has been removed from the Opal core application. If you wish to send emails, django ships with email wrappers to make this easy.
+The dependency on the letter python package has been removed from the Opal core application. If you wish to send emails, Django ships with email wrappers to make this easy.
 
 #### Updates to the Dependency Graph
 

Diff for: doc/docs/reference/upgrading.md

@@ -3,6 +3,51 @@
 This document provides instructions for specific steps required to upgrading your Opal
 application to a later version where there are extra steps required.
 
+#### v0.20.0 -> v0.21.0
+
+Opal no longer supports Python 3.5. 
+You will need to use 3.6, 3.7 or 3.8 and therefore must make sure they are installed in your environment.
+
+#### Celery changes
+
+Opal does not require you to run Celery but we do pre-configure Opal applications for use with 
+Celery.
+
+If you don't have `celery` or `django-celery` in your requirements.txt this section can be ignored.
+
+`django-celery` has been removed as a dependency. Please remove it from your requirements.
+
+__Note__ This means that old results from `django-celery` will no longer be visible from the admin.
+
+`django-celery-results==2.0.0` replaces `django-celery`, please add it to your requirements. 
+This will show Celery task results in the admin and requires `python manage.py migrate` to be run.
+
+Celery has been upgraded to 5.0.2.
+
+So if you're using a requirements.txt for example it should now include.
+    # requirements.txt
+    opal==0.21.0
+    celery==5.0.2
+    django-celery-results==2.0.0
+
+
+The Django Celery management command has changed from
+`python manage.py celery worker -l info` to `celery -A opal.core worker -l INFO`
+
+Add the below to your settings.py.
+```
+CELERY_RESULT_BACKEND = 'django-db'
+CELERY_CACHE_BACKEND = 'django-cache'
+```
+
+Remove `djcelery` from your `INSTALLED APPS` in settings and add `django_celery_results`
+```
+INSTALLED_APPS = (
+    ...,
+    'django_celery_results',
+)
+```
+
 #### v0.18.3 -> v0.20.0
 
 ##### Dependency upgrades

Diff for: opal/core/celery.py

@@ -1,27 +1,24 @@
-from __future__ import absolute_import
-
+import os
 import sys
-
+from opal.core import commandline
 from celery import Celery
 
-# set the default Django settings module for the 'celery' program.
-# os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'proj.settings')
 
-from django.conf import settings  # noqa
+def set_up():
+    if 'runtests.py' not in sys.argv:
+        if 'DJANGO_SETTINGS_MODULE' not in os.environ:
+            app_name = commandline.find_application_name()
+            settings_location = f"{app_name}.settings"
+            os.environ.setdefault('DJANGO_SETTINGS_MODULE', settings_location)
+    app = Celery('opal')
+    app.config_from_object('django.conf:settings', namespace='CELERY')
+    app.autodiscover_tasks()
+    return app
 
-app = Celery('proj')
 
-# Using a string here means the worker will not have to
-# pickle the object when using Windows.
-app.config_from_object('django.conf:settings')
-app.autodiscover_tasks(lambda: settings.INSTALLED_APPS)
+app = set_up()
 
 
 @app.task(bind=True)
 def debug_task(self):
     sys.stdout.write('Request: {0!r}\n'.format(self.request))
-
-
-app.conf.update(
-    CELERY_RESULT_BACKEND='djcelery.backends.database:DatabaseBackend',
-)

Diff for: opal/core/commandline.py

@@ -8,8 +8,6 @@
 import os
 import subprocess
 import sys
-import logging
-
 from django.core import management
 import ffs
 
@@ -243,14 +241,5 @@ def parse_args(args):
     sys.exit(0)
 
 
-def warn_for_python_35():
-    vi = sys.version_info
-    if vi.major == 3 and vi.minor == 5:
-        logging.warn(
-            "Python 3.5 will be deprecated in Opal 0.21.0",
-        )
-
-
 def main():
-    warn_for_python_35()
     parse_args(sys.argv[1:])

Diff for: opal/core/search/extract.py

@@ -323,9 +323,9 @@ def zip_archive(episodes, description, user):
     return target
 
 
-def async_extract(user, criteria):
+def async_extract(user_id, criteria):
     """
     Given the user and the criteria, let's run an async extract.
     """
     from opal.core.search import tasks
-    return tasks.extract.delay(user, criteria).id
+    return tasks.extract.delay(user_id, criteria).id

Diff for: opal/core/search/tasks.py

@@ -1,11 +1,12 @@
 from __future__ import absolute_import
-
+from django.contrib.auth.models import User
 from celery import shared_task
 
 
 @shared_task
-def extract(user, criteria):
+def extract(user_id, criteria):
     from opal.core.search import queries, extract
+    user = User.objects.get(id=user_id)
     query = queries.create_query(user, criteria)
     episodes = query.get_episodes()
     fname = extract.zip_archive(episodes, query.description(), user)

Diff for: opal/core/search/tests/test_tasks.py

@@ -21,5 +21,5 @@ def test_extract(self, zip_archive):
                 u'queryType': u'Equals'
             }
         ]
-        fname = tasks.extract(self.user, criteria)
+        fname = tasks.extract(self.user.id, criteria)
         self.assertEqual('Help', fname)

Diff for: opal/core/search/views.py

@@ -150,7 +150,7 @@ def post(self, *args, **kwargs):
         if getattr(settings, 'EXTRACT_ASYNC', None):
             criteria = _get_request_data(self.request)['criteria']
             extract_id = async_extract(
-                self.request.user,
+                self.request.user.id,
                 json.loads(criteria)
             )
             return json_response({'extract_id': extract_id})

Diff for: opal/core/signals/tasks.py

@@ -4,6 +4,7 @@
 from celery import shared_task
 
 from opal.models import Patient, Episode
+from opal.core.subrecords import get_subrecord_from_api_name
 
 
 @shared_task
@@ -23,8 +24,9 @@ def episode_post_save(created, instance_id):
 
 
 @shared_task
-def subrecord_post_save(sender, created, instance_id):
+def subrecord_post_save(sender_api_name, created, instance_id):
     from opal.core.signals.worker import subrecord_post_save
+    sender = get_subrecord_from_api_name(sender_api_name)
     instance = sender.objects.get(id=instance_id)
     subrecord_post_save.send(sender, created=created, instance=instance)
     return

Diff for: opal/core/signals/worker.py

@@ -14,14 +14,16 @@
 
 def post_save_worker_forwarder(sender, created=None, instance=None, **kwargs):
     from django.conf import settings
-    if 'djcelery' in settings.INSTALLED_APPS:
+    if 'django_celery_results' in settings.INSTALLED_APPS:
         from opal.core.signals import tasks
         if sender == models.Patient:
             tasks.patient_post_save.delay(created, instance.id)
         if sender == models.Episode:
             tasks.episode_post_save.delay(created, instance.id)
         if issubclass(sender, models.Subrecord):
-            tasks.subrecord_post_save.delay(sender, created, instance.id)
+            tasks.subrecord_post_save.delay(
+                sender.get_api_name(), created, instance.id
+            )
     return
 
 

Diff for: opal/scaffolding/scaffold/requirements.txt.jinja2

@@ -12,6 +12,6 @@ requests==2.25.0
 djangorestframework==3.12.2
 django-compressor==2.4
 python-dateutil==2.8.1
-django-celery==3.2.2
-celery==3.1.25
+django-celery-results==2.0.0
+celery==5.0.2
 opal=={{version}}

Diff for: opal/tests/test_celery.py

@@ -1,14 +1,47 @@
 """
 Unittests for opal.core.celery
 """
-from unittest.mock import patch, MagicMock
-
+from unittest.mock import patch
 from opal.core.test import OpalTestCase
 
 from opal.core import celery
 
+
 class DebugTaskTestCase(OpalTestCase):
-    def test_debug_task(self):
+    def test_with_runtests(self):
         with patch.object(celery.sys, 'stdout') as mock_stdout:
             celery.debug_task()
-            mock_stdout.write.assert_called_with('Request: <Context: {}>\n')
+            mock_stdout.write.assert_called_with("Request: <Context: {'args': (), 'kwargs': {}}>\n")
+
+    @patch("opal.core.celery.sys")
+    @patch("opal.core.celery.Celery")
+    @patch("opal.core.celery.os")
+    def test_with_settings_module(self, os, Celery, sys):
+        sys.argv = []
+        os.environ = {'DJANGO_SETTINGS_MODULE': 'already_set'}
+        celery.set_up()
+        Celery.assert_called_once_with('opal')
+        app = Celery.return_value
+        app.config_from_object.assert_called_once_with(
+            'django.conf:settings', namespace='CELERY'
+        )
+        app.autodiscover_tasks.assert_called_once_with()
+
+    @patch("opal.core.celery.sys")
+    @patch("opal.core.celery.Celery")
+    @patch("opal.core.celery.os")
+    @patch("opal.core.celery.commandline")
+    def test_without_settings_module(self, commandline, os, Celery, sys):
+        sys.argv = []
+        os.environ = {}
+        commandline.find_application_name.return_value = "my_fake_app"
+        celery.set_up()
+        Celery.assert_called_once_with('opal')
+        app = Celery.return_value
+        app.config_from_object.assert_called_once_with(
+            'django.conf:settings', namespace='CELERY'
+        )
+        app.autodiscover_tasks.assert_called_once_with()
+        self.assertEqual(
+            os.environ, {"DJANGO_SETTINGS_MODULE": "my_fake_app.settings"}
+        )

Diff for: opal/tests/test_command_detect_duplicates.py

@@ -10,10 +10,10 @@
 
 from opal.management.commands import detect_duplicates as detector
 
-class CommandTestCase(OpalTestCase):
 
+class CommandTestCase(OpalTestCase):
     def test_handle(self):
-        patient = models.Patient.objects.create()
+        models.Patient.objects.create()
         c = detector.Command()
         with patch.object(c.stdout, 'write') as writer:
             c.handle()
@@ -35,11 +35,9 @@ def test_handle_duplicate_names(self):
         c = detector.Command()
         with patch.object(c.stdout, 'write') as writer:
             c.handle()
-
             writer.assert_any_call('Jenny Smith {0}'.format(p1.id))
             writer.assert_any_call('Jenny Smith {0}'.format(p2.id))
 
-
     def test_handle_duplicate_with_three(self):
         """
         Print the duplicate triple once, but not the third instance.

Diff for: opal/tests/test_core_commandline.py

@@ -225,21 +225,3 @@ def test_main(self):
         with patch.object(commandline, 'parse_args') as pa:
             commandline.main()
             pa.assert_called_with(sys.argv[1:])
-
-
-@patch("opal.core.commandline.sys")
-@patch("opal.core.commandline.logging")
-class WarnForPython35TestCase(OpalTestCase):
-    def test_with_python_35(self, logging, sys):
-        sys.version_info.major = 3
-        sys.version_info.minor = 5
-        commandline.warn_for_python_35()
-        logging.warn.assert_called_with(
-            "Python 3.5 will be deprecated in Opal 0.21.0"
-        )
-
-    def test_not_with_python_35(self, logging, sys):
-        sys.version_info.major = 3
-        sys.version_info.minor = 6
-        commandline.warn_for_python_35()
-        self.assertFalse(logging.called)

Diff for: runtests.py

@@ -51,12 +51,13 @@
         'django.contrib.admin',
         'reversion',
         'compressor',
-        'djcelery',
+        'django_celery_results',
         'opal',
         'opal.tests',
         'opal.core.search',
         'opal.core.pathway.tests.pathway_test',
         'opal.core.pathway',
+        'opal.core.signals'
     ),
     TEMPLATES = [
         {
@@ -80,7 +81,7 @@
             },
         },
     ],
-    CELERY_ALWAYS_EAGER=True,
+    CELERY_TASK_ALWAYS_EAGER=True,
     LOGGING = {
         'version': 1,
         'disable_existing_loggers': False,
@@ -131,7 +132,6 @@
 import django
 django.setup()
 from opal.core import celery
-celery.app.config_from_object('django.conf:settings')
 
 try:
     sys.argv.remove('--failfast')

Diff for: setup.py

@@ -53,15 +53,15 @@
         'django-reversion==3.0.8',
         'django-compressor==2.4',
         'python-dateutil==2.8.1',
-        'django-celery==3.2.2',
-        'celery==3.1.25',
+        'django-celery-results==2.0.0',
+        'celery==5.0.2',
         'MarkupSafe==1.1.1', # required for python 3.5
         'six==1.15.0'
         ],
     classifiers = [
         "Programming Language :: Python",
-        "Programming Language :: Python :: 3.5",
         "Programming Language :: Python :: 3.6",
         "Programming Language :: Python :: 3.7",
+        "Programming Language :: Python :: 3.8",
     ],
 )