From 7a412873d3802370976cc0508f10bece4f0faec3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 3 Apr 2017 11:51:47 +0100 Subject: [PATCH] Refactor for better string concatenation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using `+` to concatenate strings isn’t very memory efficient. Not sure if there are real-world implications for how it’s being used here, but can’t hurt to use `.join` instead. Rewriting it as a generator also lets us remove some unneeded variable assignment. --- app/celery/tasks.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index a868a76dc..96fbc1d53 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -267,18 +267,22 @@ def persist_letter( def build_dvla_file(self, job_id): try: if all_notifications_are_created_for_job(job_id): - notifications = dao_get_all_notifications_for_job(job_id) - file = "" - for n in notifications: - t = {"content": n.template.content, "subject": n.template.subject} - # This unique id is a 7 digits requested by DVLA, not known if this number needs to be sequential. - unique_id = int(''.join(map(str, random.sample(range(9), 7)))) - template = LetterDVLATemplate(t, n.personalisation, unique_id) - file = file + str(template) + "\n" - s3upload(filedata=file, - region=current_app.config['AWS_REGION'], - bucket_name=current_app.config['DVLA_UPLOAD_BUCKET_NAME'], - file_location="{}-dvla-job.text".format(job_id)) + file_contents = '\n'.join( + str(LetterDVLATemplate( + notification.template.__dict__, + notification.personalisation, + # This unique id is a 7 digits requested by DVLA, not known + # if this number needs to be sequential. + numeric_id=int(''.join(map(str, random.sample(range(9), 7)))), + )) + for notification in dao_get_all_notifications_for_job(job_id) + ) + s3upload( + filedata=file_contents + '\n', + region=current_app.config['AWS_REGION'], + bucket_name=current_app.config['DVLA_UPLOAD_BUCKET_NAME'], + file_location="{}-dvla-job.text".format(job_id) + ) else: current_app.logger.info("All notifications for job {} are not persisted".format(job_id)) self.retry(queue="retry", exc="All notifications for job {} are not persisted".format(job_id))