From 451c5a9f1a2fda35c2e15ab27ab236178265a691 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 23 Aug 2017 18:05:48 +0100 Subject: [PATCH 1/2] Made celery task arg a tuple --- app/celery/scheduled_tasks.py | 2 +- tests/app/celery/test_scheduled_tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 5cc35f257..16fcf0d4f 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -311,7 +311,7 @@ def run_letter_jobs(): job_ids = [job.id for job in dao_get_letter_jobs_by_status(JOB_STATUS_READY_TO_SEND)] notify_celery.send_task( name=TaskNames.DVLA_FILES, - args=(job_ids), + args=(job_ids,), queue=QueueNames.PROCESS_FTP ) current_app.logger.info("Queued {} ready letter job ids onto {}".format(len(job_ids), QueueNames.PROCESS_FTP)) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 78d46a54b..cc638c01b 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -696,5 +696,5 @@ def test_run_letter_jobs(client, mocker, sample_letter_template): run_letter_jobs() mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES, - args=([job.id for job in jobs]), + args=([job.id for job in jobs],), queue=QueueNames.PROCESS_FTP) From 1b35731fb2bd6715680137c0fc4d57024f599ae1 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 24 Aug 2017 11:57:46 +0100 Subject: [PATCH 2/2] Refactor code - updated dao_get_letter_jobs_by_status to dao_get_letter_job_ids_by_status to return array of strings --- app/celery/scheduled_tasks.py | 4 +-- app/dao/jobs_dao.py | 6 ++-- tests/app/celery/test_scheduled_tasks.py | 7 +++-- tests/app/dao/test_jobs_dao.py | 37 +++++++++--------------- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 16fcf0d4f..a1f727ecd 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -13,7 +13,7 @@ from app.dao.date_util import get_month_start_and_end_date_in_utc from app.dao.inbound_sms_dao import delete_inbound_sms_created_more_than_a_week_ago from app.dao.invited_user_dao import delete_invitations_created_more_than_two_days_ago from app.dao.jobs_dao import ( - dao_get_letter_jobs_by_status, + dao_get_letter_job_ids_by_status, dao_set_scheduled_jobs_to_pending, dao_get_jobs_older_than_limited_by ) @@ -308,7 +308,7 @@ def populate_monthly_billing(): @notify_celery.task(name="run-letter-jobs") @statsd(namespace="tasks") def run_letter_jobs(): - job_ids = [job.id for job in dao_get_letter_jobs_by_status(JOB_STATUS_READY_TO_SEND)] + job_ids = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND) notify_celery.send_task( name=TaskNames.DVLA_FILES, args=(job_ids,), diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index ee90ffbe8..4b2e417b1 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -157,8 +157,8 @@ def dao_get_all_letter_jobs(): ).all() -def dao_get_letter_jobs_by_status(status): - return db.session.query( +def dao_get_letter_job_ids_by_status(status): + jobs = db.session.query( Job ).join( Job.template @@ -172,6 +172,8 @@ def dao_get_letter_jobs_by_status(status): desc(Job.created_at) ).all() + return [str(job.id) for job in jobs] + @statsd(namespace="dao") def dao_get_job_statistics_for_job(service_id, job_id): diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index cc638c01b..f795584b4 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -687,14 +687,15 @@ def test_populate_monthly_billing_updates_correct_month_in_bst(sample_template): def test_run_letter_jobs(client, mocker, sample_letter_template): jobs = [create_job(template=sample_letter_template, job_status=JOB_STATUS_READY_TO_SEND), create_job(template=sample_letter_template, job_status=JOB_STATUS_READY_TO_SEND)] + job_ids = [str(job.id) for job in jobs] mocker.patch( - "app.celery.scheduled_tasks.dao_get_letter_jobs_by_status", - return_value=jobs + "app.celery.scheduled_tasks.dao_get_letter_job_ids_by_status", + return_value=job_ids ) mock_celery = mocker.patch("app.celery.tasks.notify_celery.send_task") run_letter_jobs() mock_celery.assert_called_once_with(name=TaskNames.DVLA_FILES, - args=([job.id for job in jobs],), + args=(job_ids,), queue=QueueNames.PROCESS_FTP) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index ea09d5c18..487760ff4 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -19,7 +19,7 @@ from app.dao.jobs_dao import ( dao_get_jobs_older_than_limited_by, dao_get_job_statistics_for_job, dao_get_job_stats_for_service, - dao_get_letter_jobs_by_status) + dao_get_letter_job_ids_by_status) from app.dao.statistics_dao import create_or_update_job_sending_statistics, update_job_stats_outcome_count from app.models import ( Job, JobStatistics, @@ -551,35 +551,24 @@ def stats_set_up(notify_db, notify_db_session, service): return job_1, job_2 -def test_dao_get_letter_jobs_by_status(sample_service): +def test_dao_get_letter_job_ids_by_status(sample_service): another_service = create_db_service(service_name="another service") sms_template = create_db_template(service=sample_service, template_type=SMS_TYPE) email_template = create_db_template(service=sample_service, template_type=EMAIL_TYPE) - letter_template = create_db_template(service=sample_service, template_type=LETTER_TYPE) - another_letter_template = create_db_template(service=another_service, template_type=LETTER_TYPE) - ready_letter_jobs = [] - ready_letter_jobs.append( - create_db_job( - letter_template, - job_status=JOB_STATUS_READY_TO_SEND, - original_file_name='1.csv' - ) - ) - ready_letter_jobs.append( - create_db_job( - another_letter_template, - job_status=JOB_STATUS_READY_TO_SEND, - original_file_name='2.csv' - ) - ) + letter_template_1 = create_db_template(service=sample_service, template_type=LETTER_TYPE) + letter_template_2 = create_db_template(service=another_service, template_type=LETTER_TYPE) + letter_job_1 = create_db_job(letter_template_1, job_status=JOB_STATUS_READY_TO_SEND, original_file_name='1.csv') + letter_job_2 = create_db_job(letter_template_2, job_status=JOB_STATUS_READY_TO_SEND, original_file_name='2.csv') + ready_letter_job_ids = [str(letter_job_1.id), str(letter_job_2.id)] + create_db_job(sms_template, job_status=JOB_STATUS_FINISHED) create_db_job(email_template, job_status=JOB_STATUS_FINISHED) - create_db_job(letter_template, job_status=JOB_STATUS_SENT_TO_DVLA) - create_db_job(letter_template, job_status=JOB_STATUS_FINISHED) - create_db_job(letter_template, job_status=JOB_STATUS_PENDING) + create_db_job(letter_template_1, job_status=JOB_STATUS_SENT_TO_DVLA) + create_db_job(letter_template_1, job_status=JOB_STATUS_FINISHED) + create_db_job(letter_template_2, job_status=JOB_STATUS_PENDING) - result = dao_get_letter_jobs_by_status(JOB_STATUS_READY_TO_SEND) + result = dao_get_letter_job_ids_by_status(JOB_STATUS_READY_TO_SEND) assert len(result) == 2 - assert set(result) == set(ready_letter_jobs) + assert set(result) == set(ready_letter_job_ids)