When we're bulk updating, make sure we call `synchronize_session=False`,
and make sure that we then commit before we try and access any ORM
objects in the session that might be deleted.
Tutorial time:
when you delete, sqlalchemy needs to work out what to do with objects in
the session. "evaluate", "fetch", or False (do nothing). It wants to
remove items from the session if you're about to delete them, so that
you don't get confused about the state of objects
* evaluate compares the delete query to each item in the session in
turn. this is the default. if you have lots in the session this could
be super slow I guess but that's rarely the case for us. This can lead
to errors if the column names are different to the model names, like
on our notification and history models.
* fetch runs the delete query as if it's a select, and then checks the
results of that vs the session. This could be slow.
* False doesn't do anything. This means the session will be stale and
potentially will contain now-deleted items, until we call `commit` or
`expire_all` on the session.
https://docs.sqlalchemy.org/en/13/orm/query.html?highlight=query.update#sqlalchemy.orm.query.Query.delete
So since removing the subquery in the previous commit, we now are doing
all of the inserts using offsets and limits to group by 10,000 and deleting
all records in a single query (which could be up to as many as 10
million rows).
We want to avoid doing this, because both of these ways we think are
going to result in expensive queries. Therefore we have introduced our
own chunking of the notifications by hour periods meaning we do not need
to use offset and limits.
We estimate that GOV.UK email will at most send 600,000 notifications
per hour (175 per second * 60 * 60).
This runs on the new `sms-callbacks` queue. The function
`process_sms_client_response` has been replaced with a task called
`process_sms_client_response`. This involved some reorganisation of the
existing code and tests.
This should add an extra 400 connections maximum, which will not tip us over the allowable 5000 db connections. And it may help with the queue pool connection errors.
We are formatting the postcode here, because if we did it in template
preview, that could break flows like API and admin one-off, since
we are not validating postcode there yet, and format_postcode
needs a nice validated postcode.
We are not doing it in admin, as then we would have to either
rewrite the CSV file or pass data differently to API. First would
be nasty, second is a lot of overhead.
In the long run we might want to move postcode formatting to
template preview so that the postcode in letter preview looks the same
before and after user sends it, but now to get it out quickly it's better
to format the postcode here in the task.
- Table to store meta data for the emergency contact list for a service.
- Endpoint for fetching contact lists for service
- Endpoint for saving contact list for service.
The list will be stored in S3. The service will then be able to send emergency announcements to staff.
https://github.com/alphagov/notifications-api/pull/2742/files#diff-d9a6761afaff4a491e60b64f6063654fL265
introduced a change in behaviour which causes template preview to 500
for a precompiled letter that is invalid but not because the content is
outside the printable area.
The changes it back to the previous behaviour, where we b64encode and
utf-8 decode for a page that is invalid for reasons that are not content
outside the printable area, for example a non portrait a4 letter.
I didn't end up writing a unit test for this because I really can't
figure out the existing behaviour we expect for this use case. This
method is too big and complex but I'm confident by looking at the change
in the commit mentioned above that this puts behaviour back to how it
was. Manual testing also confirms the fix.
I've also taken this opportunity to improve two test names whilst I was
looking at things.
- update check_sms_content_char_count to use the SMSTemplate.is_message_too_long function, and updated the error message to align with the message returned by the admin app.
- Update the the code used by version 1 of the api to use the validate_template method.
- I did find a couple of services still using the old api, however, this change should not affect them as I checked the messages being sent and they are not too long.
- We will be sending a message to them to see if they can upgrade.
- Update the log message for authenication to include the URL - makes it easier to track if a service is using version 1 of the api.
If the png page is invalid then show the overlay for that page
If the png page is not invalid, even if other pages are invalid, then
don't shown the overlay for that page
These keeps the behaviour were if you get the pdf for a pdf which has
invalid pages then the whole PDF is requested (not just a single page)
and all of the pages include the overlay
Tests have been improved to be more explicit about what we are testing
If you have some returned letters, but none in the last 7 days, we don’t
count how many you have in the last 7 days. But we should test to make
sure we’re not going to the database again.
We want to start granting access to the org page. But it will be a bit
weird if the invites come from us personally, since the people we’re
inviting don’t know us.
It makes more sense, and sounds more official if the invites appear to
come from the ‘GOV.UK Notify team’ instead.
