#2205 - Comp and Pen Batches #2206
Conversation
| template, | ||
| sms_sender_id, | ||
| reply_to_text, | ||
| [DynamoRecord(**item) for item in records], |
There was a problem hiding this comment.
nitty and no fix required.
Double iteration - We're iterating over all the records to convert them in a DynamoRecord object. Then in _send_comp_and_pen_sms we are iterating over thru all the records again. Could we do both these things in the same iteration?
There was a problem hiding this comment.
That's a good callout. I had gone back and forth on creating the dataclass at each iteration vs in a single call. I opted to go this route for simplicity in the downstream method since the other methods acts as a "pre-processor".
There was a problem hiding this comment.
Approved: Reviewed new code and compared to old logic. Reviewed new testing..
Reviewed Glue script . @k-macmillan says he working with Corey to add script to vanotify-commons.
Left one comment about a double iteration which is not a big deal because our batch size is relatively small.
We may need follow up work to return the is_processed attribute from DynamoDB as we're no longer using that attribute to determine batches.
|
|
||
| @notify_celery.task(name='comp-and-pen-batch-process') | ||
| @statsd(namespace='tasks') | ||
| def comp_and_pen_batch_process(records: list[dict[str, str]]) -> None: |
There was a problem hiding this comment.
Did we want to leave a comment about this being triggered by a Glue Script?
There was a problem hiding this comment.
We don't do that with other code. I don't believe that's necessary here. I do need to update some of our other docs though.
Description
Test turned full implementation. Rather than process all records from the table with Celery Beat we instead run AWS Glue. Glue has been used as an ETL. It pulls all records from dynamodb, changes them into the required format, and then sends them to SQS in "batches" (homebrew batches, not sqs batches). This completed in under a minute on prod with 65,580 records, so DelaySeconds was added to slow it down.
Removed Comp and Pen feature flag. Removed
notification_typefrom the bypass route because it was redundant.issue #2205
How Has This Been Tested?
Deployed the notification-api and then ran the Glue script. It ingested all the records as expected and processed them correctly. The celery broker picked up the work and sent it to Celery tasks for processing.
The
prod-bip-consumer-dead-letter-queuewas purged and all prod records were sent there so they can be seen, though they contain an extra field that I have since deleted.65,580 / 25 = 2623.2 (which would round to one more
send_messagecall, or 2624):It managed to process and enqueue all of those in under a minute:
Unit tests cover 100% of the new code:
All table entries being processed:
Checklist