This has been moved to the letters utils file since it will be used in
more than one place. The notification parameter has been removed so that
the function can be used when we don't have a notification id.
previously, we didn't create templated letters, and just marked them as
delivered straight away. However, we may need to return PDFs for these
letters, so we should create them the same as live letters. Then update
the functions so that they know where to look for these letters.
The `process_virus_scan_passed` task now catches S3 errors - if these
occur, it logs an exception and puts the letter in a `technical-failure`
state. We don't retry the task, because the most common reason for
failure would be the letter not being in the expected S3 bucket, in
which case retrying would make no difference.
the celery decorator should always be on the outside so that all other
decorators will be captured within the celery task. We had problems
with cronitor not reporting, and only for this task.
This celery task was not decorated with the cronitor decorator so never
checked in with cronitor.
Adding the decorator will ensure this task is monitored.
The requisite cronitor key is in the credentials repository already.
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
if we partially retry a day, we would create new zip files, containing
different letters (if some were processed succesfully). We need these
files to have different filenames to earlier zip files so that we can
avoid overwriting log data in zips_sent.
Hashing the filename means that we'll only overwrite if it was the same
file containing the same content.
DVLA don't care about the naming conventions of zip files, other than
it must start with `NOTIFY.` and end with `.ZIP`. So lets format the
date in a more readable way, and separate it from the batch number
previously ftp would name the files itself by giving them a timestamp
when uploading. we ran into issues with tasks being picked up multiple
times and as such, uploading duplicate files. By naming the file before
creating the task, we can avoid this issue.
Files are now named `NOTIFY.YYYYMMDD######.ZIP` where the number is a
counter that increments with each task we've issued in that run of
collate-letter-pdfs-for-day
antivirus is sometimes tough to get running locally - now in dev
antivirus is skipped unless `ANTIVIRUS_ENABLED=1` is set on the command
line. on all other environments it is always enabled.
The template preview app now accepts a null value for the `filename`
parameter. If a service doesn't have a letter branding option set,
previously we defaulted to their dvla_organisation (probably HM
Government). Now, we pass through None, so that we generate letters
without any logo or branding.
If a precompiled letter can't be opened (e.g. because it isn't a valid
PDF) we were setting its billable units to 0, but not moving it to the
invalid PDF bucket. If a precompiled letter failed sanitisation, we were
moving it to the invalid PDF bucket but not setting its billable units
to 0.
This commit makes sure that we always set the billable units to 0
and move the PDF to the right bucket if it fails sanitisation or can't be
opened.
We want to send two new headers, ServiceId and NotificationId to the
template preview /precompiled/sanitise endpoint. This is to allow us to log
errors from this endpoint in template preview with all the information needed,
instead of needing to pass the information back to notifications-api and
to log it there.
We were passing both dvla_org_id and filename to template-preview
temporarily while we switch to only using filename. Now that
template-preview is set up to use the filename, we can stop sending the
dvla_org_id too.
We now pass `filename`, the filename of the letter logo to use, through
to Template Preview in addition to the `dvla_org_id`. Once Template
Preview has been updated to only use the `filename` we will stop
sending the `dvla_org_id`.
it doesn't match a string 😩
I couldn't think of a good way to test this in a unit test, since
it involves changing the service id on all of the components of a
service.
the move from virus scan to validation failed function was called with
the wrong variables, and had some internal logic that was slightly
wrong.
Also, Don't use `update_notification_by_id` for notifications if they
are not in `created`, `sending`, `pending`, or `sent`. It silently
doesn't update them. I didn't want to do a deeper investigation into
the reasons behind this terrifying state machine as part of this commit
so I just changed the functions to call `dao_update_notification`
manually
- pass new, sanitised pdf for sending
- move invalid pdfs to a newly created bucket
- set status fro notifications that failed pdf validation to a new status validation-failed
- adjust existing tests
There was a datetime bug in the query which resulted in files not being sent to the postal provider.
The trigger-letter-pdfs-for-day task is no longer needed, so rather than fix the query just call collate_letter_pdfs_for_day directly.
Less code is always better.
Deployment considerations: I realized this is strictly not backwards compatible if the scheduled job is in progress and a task is on the queue that no longer exists. This is ok since we will deploy this well before 17:50.
For returned letter updates most notifications won't exist in the
notifications table, so in order to find out whether the reference
matches any known letters we need to check the count of updated
history records.
Notify antivirus, on success, calls the process_virus_scan_passed taks.
Previously, this task would:
* update status to created (or delivered for test keys)
* copy the file from the scan bucket to either the live or test bucket
based on the results
* delete the old file in the scan bucket
We want it to:
* download file from scan bucket
* sanitise PDF using new template-preview functionality
* if sanitise failed, set to new status "validation-failed" and save
the pdf somewhere.
* send new pdf to live/test bucket
* update status to created (or delivered for test keys)
* delete the original file in the scan bucket
This PR does some of that:
* download file from scan bucket
* sanitise PDF using new template-preview functionality
* if sanitise failed, just log.
* send OLD pdf to live/test bucket
* update status to created (or delivered for test keys)
* delete the original file in the scan bucket
So if sanitising fails, we won't fall over and not deliver the letter,
we'll just log a message for now. If sanitise throws an unexpected
error (as opposed to a 400), we'll retry up to fifteen times (the same
as when creating a new letter). I've added the code for using the
sanitised pdf, but it's commented out for now
be known. Added the notification id to the logging message so that
the notification can be traced through the logging system by knowing
the notification id, making it easier to debug. Also changed to raise an
exception so that alerts are generated. This way we should get an email
to say that there has been an error.
application. If the Anti-virus app fails due to s3 errors or ClamAV
so does not scan (even after retries) the file at all an error needs
to be raised and the notification set to technical-failure.
Files should be moved to a 'folder' a separate one for ERROR and FAILURE.
* Added new letter task to process the error
* Added a new method to letter utils.py to move a file into an error or
failure folder based on the input
* Added tests to test the task and the utils.py method
- precompiled PDFs sent by test key uploaded to scan bucket
- set status to VIRUS-SCAN-FAILED for pdfs failing virus scan rather than PERMANENT-FAILURE
- Make call to AV app for precompiled letters sent via a test key, and set notification status to PENDING-VIRUS-SCAN
