Merge branch 'master' into add-multiple-reply-to-email-endpoints

app/authentication/auth.py

@@ -51,7 +51,6 @@ def restrict_ip_sms():
         current_app.logger.info("X-Custom-forwarder {}".format(request.headers.get("X-Custom-forwarder")))
 
     # Check IP of SMS providers
-    ip = ''
     if request.headers.get("X-Forwarded-For"):
         # X-Forwarded-For looks like "203.0.113.195, 70.41.3.18, 150.172.238.178"
         # Counting backwards and look at the IP at the 3rd last hop - hence, hop(end-3)
@@ -80,7 +79,7 @@ def restrict_ip_sms():
                 'message': 'Inbound sms ip address',
                 'log_contents': {
                     'passed': allowed,
-                    'ip_address': ip
+                    'ip_address': inbound_ip
                 }
             })
             return

app/celery/celery.py

@@ -1,17 +1,28 @@
-from celery import Celery
+from flask import current_app
+from celery import Celery, Task
+
+
+class NotifyTask(Task):
+    abstract = True
+
+    def on_failure(self, exc, task_id, args, kwargs, einfo):
+        # ensure task will log exceptions to correct handlers
+        current_app.logger.exception('Celery task failed')
+        super().on_failure(exc, task_id, args, kwargs, einfo)
+
+    def __call__(self, *args, **kwargs):
+        # ensure task has flask context to access config, logger, etc
+        with current_app.app_context():
+            return super().__call__(*args, **kwargs)
 
 
 class NotifyCelery(Celery):
 
     def init_app(self, app):
-        super().__init__(app.import_name, broker=app.config['BROKER_URL'])
+        super().__init__(
+            app.import_name,
+            broker=app.config['BROKER_URL'],
+            task_cls=NotifyTask,
+        )
+
         self.conf.update(app.config)
-        TaskBase = self.Task
-
-        class ContextTask(TaskBase):
-            abstract = True
-
-            def __call__(self, *args, **kwargs):
-                with app.app_context():
-                    return TaskBase.__call__(self, *args, **kwargs)
-        self.Task = ContextTask

app/celery/scheduled_tasks.py

@@ -314,7 +314,7 @@ def populate_monthly_billing():
 def run_letter_jobs():
     job_ids = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND)
     notify_celery.send_task(
-        name=TaskNames.DVLA_FILES,
+        name=TaskNames.DVLA_JOBS,
         args=(job_ids,),
         queue=QueueNames.PROCESS_FTP
     )

app/config.py

@@ -48,7 +48,8 @@ class QueueNames(object):
 
 
 class TaskNames(object):
-    DVLA_FILES = 'send-files-to-dvla'
+    DVLA_JOBS = 'send-jobs-to-dvla'
+    DVLA_NOTIFICATIONS = 'send-notifications-to-dvla'
 
 
 class Config(object):
@@ -226,7 +227,7 @@ class Config(object):
         },
         'run-letter-jobs': {
             'task': 'run-letter-jobs',
-            'schedule': crontab(minute=30, hour=17),
+            'schedule': crontab(hour=17, minute=30),
             'options': {'queue': QueueNames.PERIODIC}
         }
     }

app/letters/rest.py

@@ -16,7 +16,7 @@ register_errors(letter_job)
 @letter_job.route('/send-letter-jobs', methods=['POST'])
 def send_letter_jobs():
     job_ids = validate(request.get_json(), letter_job_ids)
-    notify_celery.send_task(name=TaskNames.DVLA_FILES, args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP)
+    notify_celery.send_task(name=TaskNames.DVLA_JOBS, args=(job_ids['job_ids'],), queue=QueueNames.PROCESS_FTP)
 
     return jsonify(data={"response": "Task created to send files to DVLA"}), 201
 

app/v2/notifications/post_notifications.py

@@ -151,7 +151,7 @@ def process_letter_notification(*, letter_data, api_key, template):
         raise BadRequestError(message='Cannot send letters with a team api key', status_code=403)
 
     if api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST:
-            raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403)
+        raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403)
 
     job = create_letter_api_job(template)
     notification = create_letter_notification(letter_data, job, api_key)

requirements.txt

@@ -26,6 +26,6 @@ notifications-python-client==4.4.0
 awscli>=1.11,<1.12
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@21.0.0#egg=notifications-utils==21.0.0
+git+https://github.com/alphagov/notifications-utils.git@21.2.0#egg=notifications-utils==21.2.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

tests/app/celery/test_scheduled_tasks.py

@@ -690,6 +690,6 @@ def test_run_letter_jobs(client, mocker, sample_letter_template):
 
     run_letter_jobs()
 
-    mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES,
+    mock_celery.assert_called_once_with(name=TaskNames.DVLA_JOBS,
                                         args=(job_ids,),
                                         queue=QueueNames.PROCESS_FTP)

tests/app/letters/test_send_letter_jobs.py

@@ -25,7 +25,7 @@ def test_send_letter_jobs(client, mocker, sample_letter_template):
     assert response.status_code == 201
     assert json.loads(response.get_data())['data'] == {'response': "Task created to send files to DVLA"}
 
-    mock_celery.assert_called_once_with(name="send-files-to-dvla",
+    mock_celery.assert_called_once_with(name="send-jobs-to-dvla",
                                         args=(job_ids['job_ids'],),
                                         queue="process-ftp-tasks")