Closed bboreham closed 4 years ago
Conceptually LGTM! I only added a few minor comments concerning the config robustness.
I just looked at the code though (didn't go as far as trying to test) - how well do you think the current unit tests cover the logic modified here?
The current unit tests don't cover any of the DynamoDB/S3 code. I think if I was going to add tests I'd do it as integration tests, running mock DynamoDB and S3 containers.
This means we store fewer, bigger, reports, which reduces cost of storage and time to render when data is viewed.