Conversation
cduhn17
left a comment
There was a problem hiding this comment.
The move to bulk checking is a great feature, but the current implementation introduces a significant performance risk for our database.
Performance (Blocker): The code performs Blocklist.objects.get(ip=ip) inside the for loop. This creates an N+1 query problem. Please refactor this to use Blocklist.objects.filter(ip__in=ip_addresses) to retrieve all data in a single database query.
Security: Please add a check to limit the length of ip_addresses (e.g., max 100) to prevent resource exhaustion attacks on the Lambda.
Typos: Please fix "Determing" -> "Determining" in the docstring.
|
Don't see any issues other than the suggestions brought up by Craig. |
cduhn17
left a comment
There was a problem hiding this comment.
Thanks for the updates.
🗣 Description
💭 Motivation and context
CRASM-3651
🧪 Testing
✅ Pre-approval checklist
bump_versionscript if this repository is versioned and the changes in this PR warrant a version bump.✅ Pre-merge checklist
✅ Post-merge checklist