Merge branch 'v0.21.0'

Author: fredkingham

.travis.yml

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

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
 

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

doc/mkdocs.yml

@@ -121,7 +121,7 @@ dev_addr: 0.0.0.0:8965
 include_next_prev: false
 
 extra:
-  version: v0.20.0
+  version: v0.21.0
 
 markdown_extensions:
     - fenced_code

opal/_version.py

@@ -1,4 +1,4 @@
 """
 Declare our current version string
 """
-__version__ = '0.20.0'
+__version__ = '0.21.0'

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',
-)

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:])

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

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)

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)

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})

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

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
 
 

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}}

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"}
+        )

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.

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)

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