Commit Graph

7633 Commits

Author SHA1 Message Date
pyup-bot
a3ba4b3e3d Update iso8601 from 0.1.12 to 0.1.13 2020-11-06 10:51:09 +00:00
pyup-bot
73ca396504 Update eventlet from 0.27.0 to 0.29.1 2020-11-06 10:51:09 +00:00
pyup-bot
8cb1797ec4 Update cffi from 1.14.2 to 1.14.3 2020-11-04 13:00:02 +00:00
Katie Smith
1806a6c8b3 Merge pull request #3025 from alphagov/revert-2992-validate-email-size
Revert "Tailor message-too-long error message depending on the notification type"
2020-11-03 11:09:47 +00:00
Katie Smith
c4075f1fc0 Revert "Tailor message-too-long error message depending on the notification type" 2020-11-03 10:55:15 +00:00
Rebecca Law
184db4fa5d Merge pull request #3024 from alphagov/revert-3020-revert-3000-add-save-sms-api-task
Add a task to save-api-sms for high volume services.
2020-11-02 12:36:19 +00:00
Pea M. Tyczynska
b503267062 Merge pull request #2992 from alphagov/validate-email-size
Tailor message-too-long error message depending on the notification type
2020-10-30 11:07:29 +00:00
Pea Tyczynska
41d1cf453d Update limit to 1MB and update tests
SES rejects email messages bigger than 10485760 bytes (just over 10 MB per message (after base64 encoding)):
https://docs.aws.amazon.com/ses/latest/DeveloperGuide/quotas.html#limits-message

Base64 is apparently wasteful because we use just 64 different values per byte, whereas a byte can represent
256 different characters. That is, we use bytes (which are 8-bit words) as 6-bit words. There is
a waste of 2 bits for each 8 bits of transmission data. To send three bytes of information
(3 times 8 is 24 bits), you need to use four bytes (4 times 6 is again 24 bits). Thus the base64 version
of a file is 4/3 larger than it might be. So we use 33% more storage than we could.
https://lemire.me/blog/2019/01/30/what-is-the-space-overhead-of-base64-encoding/

That brings down our max safe size to 7.5 MB == 7500000 bytes before base64 encoding

But this is not the end! The message we send to SES is structured as follows:
"Message": {
    'Subject': {
        'Data': subject,
    },
    'Body': {'Text': {'Data': body}, 'Html': {'Data': html_body}}
},
Which means that we are sending the contents of email message twice in one request: once in plain text
and once with html tags. That means our plain text content needs to be much shorter to make sure we
fit within the limit, especially since HTML body can be much byte-heavier than plain text body.

Hence, we decided to put the limit at 1MB, which is equivalent of between 250 and 500 pages of text.
That's still an extremely long email, and should be sufficient for all normal use, while at the same
time giving us safe margin while sending the emails through Amazon SES.
2020-10-29 14:07:49 +00:00
Pea Tyczynska
9708b09ba3 Tailor message-too-long error message
depending on the notification type.

Up until now, only sms messages could get message-too-long error,
but now we also need to validate the size of email messages, so
the message content needs to be tailored to the notification type.
2020-10-29 14:07:48 +00:00
Rebecca Law
4a9eca3dff Remove space between queue names. Doh 2020-10-29 11:14:11 +00:00
Rebecca Law
29b6f84f6c Revert "Revert "Add a task to save-api-sms for high volume services."" 2020-10-29 11:12:46 +00:00
Toby Lorne
d39379062d Merge pull request #3022 from alphagov/sent-finishes-broadcast
client: cbc_proxy passes through sent/expires
2020-10-28 11:51:31 +00:00
Toby Lorne
dd012d6831 client: cbc_proxy passes through sent/expires
A BroadcastEvent knows when an event was sent and should expire

We pass through these values directly to the CBC Proxy, because
BroadcastEvent knows how they should be formatted

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
2020-10-28 11:37:06 +00:00
Toby Lorne
2c29b45c9c Merge pull request #3018 from alphagov/link-tests
Link tests
2020-10-28 09:58:56 +00:00
Rebecca Law
ddee6e7dd5 Merge pull request #3021 from alphagov/fix-utc-bug
Quick UTC bug fix
2020-10-27 17:37:03 +00:00
Rebecca Law
df325c78b7 There seems to be a UTC time bug around the dao_get_uploads_by_service_id function.
To fix the build tonight I'm putting freezing the time for the test. But will investigate further tomorrow.
2020-10-27 17:25:58 +00:00
Rebecca Law
f47a1af305 Merge pull request #3020 from alphagov/revert-3000-add-save-sms-api-task
Revert "Add a task to save-api-sms for high volume services."
2020-10-27 16:28:16 +00:00
Rebecca Law
06ff1bf596 Revert "Add a task to save-api-sms for high volume services." 2020-10-27 16:18:57 +00:00
Toby Lorne
dda71bf685 celery: add task for triggering link tests
We want to periodically kick off some link tests, so that:

- we are periodically communicating with the CBC Proxies
- each CBC Proxy is periodically communicating with its CBC

This simulation of traffic to the CBC will give us advance warning if
something is going to break, or is broken, before someone tries to send
a real live message

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Richard <richard.baker@digital.cabinet-office.gov.uk>
Co-authored-by: Pea <pea.tyczynska@digital.cabinet-office.gov.uk>
2020-10-27 15:39:28 +00:00
Toby Lorne
7542709455 clients: cbc_proxy sends message_type
When we ask the CBC Proxy to send a message, we should specify that we
want to send a real message, when we want a real message

We will do this by specifying the message_type which can have 4 types, 3
of which represent a real message:

| Name   | Effect                   |
| ------ | ------------------------ |
| alert  | Create an alert          |
| update | Update an existing alert |
| cancel | Cancel an existing alert |
| test   | Send a link test         |

We will use message_type to represent the table above

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Richard <richard.baker@digital.cabinet-office.gov.uk>
Co-authored-by: Pea <pea.tyczynska@digital.cabinet-office.gov.uk>
2020-10-27 15:24:02 +00:00
Rebecca Law
ef416a64ed Merge pull request #3000 from alphagov/add-save-sms-api-task
Add a task to save-api-sms for high volume services.
2020-10-27 14:41:51 +00:00
Chris Hill-Scott
5459d96c58 Merge pull request #3016 from alphagov/use-utils-to-format-broadcast-content
Use utils template to format broadcast content
2020-10-27 12:36:26 +00:00
Rebecca Law
a32e418bab Make sure that letters can not use the high volume service code path.
This code is already restricted to SMS and email, so this is only to make it obvious if there is a code refactor down the line. Perhaps this is overkill and we back out this commit.
2020-10-27 12:05:59 +00:00
Toby Lorne
e7e0b06669 Merge pull request #3017 from alphagov/cbc-proxy-canary
Add task to send canary to cbc proxy
2020-10-27 10:57:58 +00:00
Chris Hill-Scott
28f01bd776 Use utils template to format broadcast content
Brings in:
- [x] https://github.com/alphagov/notifications-utils/pull/801

Formats the content of the template at the time of creating the event.
This means that any downstream code (eg Celery tasks) can assume the
content is already formatted correctly.

Also, these downstream tasks don’t  know which template the broadcast
was created from, so if we support personalisation in the future this is
the most sensible place to bring together the template and the
personalisation.

---

I had to re-create some of the deleted code from utils for stuff like
formatting the timestamp to the CAP standard.
2020-10-27 10:55:26 +00:00
Pea Tyczynska
5cff30aa47 Send a log message informing that we are sending canary to CBC proxy 2020-10-27 10:45:32 +00:00
Toby Lorne
be90455944 Add task to send canary to cbc proxy
Create and schedule a Celery task that tests if we
can send a canary message to cbc proxy.

This will help us know if something happens to our
connection to cbc proxy.

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Pea <pea.tyczynska@digital.cabinet-office.gov.uk>
Co-authored-by: Richard <richard.baker@digital.cabinet-office.gov.uk>
2020-10-27 10:38:09 +00:00
Toby Lorne
052de84c9e clients: cbc_proxy client has canary method
The CBC Proxy is essentially a lambda function which we invoke with
various arguments.

A way in which this can fail is that the notifications-api app invoking
the function may not be able, any longer, to invoke the function.

This could be caused by, for example:
* an egress restriction preventing access to eu-west-2.lambda.amazonaws.com
* a network partition preventing access to eu-west-2.lambda.amazonaws.com
* the app's credentials have been rotated or revoked

If we invoke a simple "canary" lambda function for which the app should
have access to invoke, and check it for failures, we will know quickly
if something is likely to be broken.

This is especially important for cell broadcasts compared to email/SMS
because we always have a baseline of traffic for email/SMS, and so any
failure is observed almost immediately. This is not true for CB where we
may expect to only see one CB message every week/month/quarter/year, as
opposed to every minute or second for email/SMS.

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Pea <pea.tyczynska@digital.cabinet-office.gov.uk>
2020-10-26 17:14:08 +00:00
Toby Lorne
fbf3b96113 Merge pull request #3015 from alphagov/cbc-proxy-no-descriptions
CBC Proxy does not need area descriptions
2020-10-26 15:27:59 +00:00
Toby Lorne
d22adb7813 broadcasts: do not send description to cbc_proxy
"areas" and "simple_polygons" in "transmitted_areas" do not have the
same length

as an example, choosing the area "england" results in a single item in
"areas" but many polygons in "simple_polygons"

therefore zipping these two together gives a list of areas:
* of length 1
* containing only new grimsby

which is not what we want

as the CBC does not care about the areaDesc field within CAP, we should
omit it from the function invocation and delegate the contents of
areaDesc to the CBC Proxy implementation

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Richard <richard.baker@digital.cabinet-office.gov.uk>
Co-authored-by: David <david.mcdonald@digital.cabinet-office.gov.uk>
2020-10-26 15:16:33 +00:00
Toby Lorne
ca2bea8ae6 broadcasts: test helper uses areas from message
instead of looking at "transmitted_areas" argument in the
create_broadcast_event, we should use the areas from the broadcast
message

we should be explicit in our tests about which areas we are sending, the
tests were implicitly using the default, rather than the areas from the
broadcast message which was confusing

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Co-authored-by: Richard <richard.baker@digital.cabinet-office.gov.uk>
Co-authored-by: David <david.mcdonald@digital.cabinet-office.gov.uk>
2020-10-26 15:14:26 +00:00
Leo Hemsted
902a2dde94 Merge pull request #3014 from alphagov/get-query-in-50k-batches
Optimise dao_get_letters_to_be_printed query
2020-10-26 13:23:17 +00:00
Rebecca Law
3dee4ad310 Add a task to save-api-sms for high volume services.
When we initially added a new task to persist the notifications for a high volume service we wanted to implement it as quickly as possible, so ignored SMS.
This will allow a high volume service to send SMS, the SMS will be sent to a queue to then persist and send the SMS, similar to emails.

At this point I haven't added a new application to consume the new save-api-sms-tasks. But we can add a separate application or be happy with how the app scales for both email and sms.
2020-10-26 13:09:37 +00:00
Leo Hemsted
3bc3ed88b3 use yield_per instead of limit
limit means we only return 50k letters, if there are more than that for
a service we'll skip them and they won't be picked up until the next
day.

If you remove the limit, sqlalchemy prefetches query results so it can
build up ORM results, for example collapsing joined rows into single
objects with chidren. SQLAlchemy streams the data into a buffer, and
normally will still prefetch the entire resultset so it can ensure
integrity of the session, (so that if you modify one result that is
duplicated further down in the results, both rows are updated in the
session for example). However, we don't care about that, but we do care
about preventing the result set taking up too much memory. We can use
`yield_per` to yield from sqlalchemy to the iterator (in this case the
`for letter in letters_awaiting_sending` loop in letters_pdf_tasks.py) -
this means every time we hit 10000 rows, we go back to the database to
get the next 10k. This way, we only ever need 10k rows in memory at a
time.

This has some caveats, mostly around how we handle the data the query
returns. They're a bit hard to parse but I'm pretty sure the notable
limitations are:

* It's dangerous to modify ORM objects returned by yield_per queries
* It's dangerous to join in a yield_per query if you think there will be
  more than one row per item (for example, if you join from notification
  to service, there'll be multiple result rows containing the same
  service, and if these are split over different yield chunks, then we
  may experience undefined behaviour.

These two limitations are focused around there being no guarantee of
having one unique row per item.

For more reading:
https://docs.sqlalchemy.org/en/13/orm/query.html?highlight=yield_per#sqlalchemy.orm.query.Query.yield_per
https://www.mail-archive.com/sqlalchemy@googlegroups.com/msg12443.html
2020-10-26 13:01:34 +00:00
Toby Lorne
59bce8e52f Merge pull request #3012 from alphagov/no-more-stub-cbc
Remove references to stub CBC
2020-10-26 12:35:19 +00:00
Toby Lorne
fdacb2e0d7 broadcasts: remove references to cbc stub
We are phasing out our cbc-proxy stub which displayed CAP XML messages

We are in the process of testing with real CBCs, so maintaining our own
stub is not useful

This commit
* removes the HTTP POST requests to the CBC proxy
* writes up the update/cancel methods of the cbc_client (not impl)

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
2020-10-26 10:23:49 +00:00
Toby Lorne
1aba4925e5 Merge pull request #3011 from alphagov/send-polygons-to-cbc-proxy
clients: cbc_proxy actions accepts areas param
2020-10-26 10:22:48 +00:00
Leo Hemsted
ed182c2a22 return just the columns we need for collating letters
previously we were returning the entire ORM object. Returning columns
has a couple of benefits:

* Means we can join on to services there and then, avoiding second
  queries to get the crown status of the service later in the collate
  flow.
* Massively reduces the amount of data we return - particularly free
  text fields like personalisation that could be potentially quite big.
  5 columns rather than 26 columns.
* Minor thing, but will skip some CPU cycles as sqlalchemy will no
  longer construct an ORM object and try and keep track of changes. We
  know this function doesn't change any of the values to persist them
  back, so this is an unnecessary step from sqlalchemy.

Disadvantages are:

* The dao_get_letters_to_be_printed return interface is now much more
  tightly coupled to the get_key_and_size_of_letters_to_be_sent_to_print
  function that calls it.
2020-10-23 20:01:18 +01:00
Toby Lorne
aa002afd31 clients: cbc_proxy actions accepts areas param
related:
https://github.com/alphagov/notifications-broadcasts-infra/pull/23

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
2020-10-23 17:09:00 +01:00
Leo Hemsted
d9ceed55b5 Merge pull request #3009 from alphagov/limit-collate-task-to-50k-letters
limit collate task to 50,000 messages
2020-10-23 15:41:45 +01:00
Leo Hemsted
f8fbaba77d limit collate task to 50,000 messages
we've seen issues where tasks mysteriously hang and do not process
large volumes of letters - in this case >150k letters in created state.

to try and get at least some letters out of the door, limit the query to
only return 50k letters per postage type. We may need to run the task
multiple times, or letters may get delayed until the next day when
they'd be picked up (provided there's enough capacity then). The task
should only be re-run AFTER the ftp tasks have all finished, and updated
the letters to sending, or we run the risk of sending the same letters
twice.

For context, the largest ever letter day we've sent is ~65k in march of
this year.
2020-10-23 15:28:34 +01:00
Chris Hill-Scott
55efbb0bb1 Merge pull request #3010 from alphagov/revert-3001-remove-upload-letters-permission
Revert "Remove the upload letters permission"
2020-10-23 15:26:19 +01:00
Chris Hill-Scott
88cd92b946 Revert "Remove the upload letters permission" 2020-10-23 15:14:37 +01:00
Chris Hill-Scott
e7c1f7c60e Merge pull request #3001 from alphagov/remove-upload-letters-permission
Remove the upload letters permission
2020-10-23 14:27:48 +01:00
Leo Hemsted
96f59a51e3 Merge pull request #3008 from alphagov/stream-dvla-files
stream notifications when collating zip files
2020-10-23 12:31:04 +01:00
Leo Hemsted
4b61060d32 stream notifications when collating zip files
we had issues where we had 150k 2nd class notifications, and the collate
task never ran properly, presumably because the volume of data being
returned was too big.

to try and help with this, we can switch to streaming rather than using
`.all` and building up lists of data. This should help, though the
initial query may be a problem still.
2020-10-23 12:20:26 +01:00
David McDonald
6fbf12afeb Merge pull request #3007 from alphagov/simplify-filename
Remove 'INSOLVENCY' from zip files for insolvency letters
2020-10-23 10:21:10 +01:00
David McDonald
3dcb97c45a Remove 'INSOLVENCY' from zip files for insolvency letters
This is at request of DVLA. They would prefer to have zip files with the
same number of arguments in the name. After being offered a few
different options, such as including an org and service id for all zips,
they chose to just remove the 'INSOLVENCY' tag.

For more context see PR that added the tag
https://github.com/alphagov/notifications-api/pull/3006
2020-10-23 09:58:28 +01:00
David McDonald
890a66de46 Merge pull request #3003 from alphagov/staging-preview-cbc
Non-production environments invoke CBC Proxy during broadcast event creation
2020-10-22 16:21:11 +01:00
Toby Lorne
ff1ffc7fba clients: cbc_proxy lambda client is unabbreviated
for code clarity

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
2020-10-22 12:22:11 +01:00