Polish repo for attractiveness and add cost-optimization improvements#6
Conversation
tukue
commented
May 26, 2026
tukue
commented
- Add LICENSE (MIT), CONTRIBUTING.md, PR template, .editorconfig
- Overhaul README with badges, config tables, policy matrix, project tree
- Improve package.json with description, keywords, repo metadata
- Conditional NAT Gateway (~2/mo saved) — deployed only when enrichment API CIDRs are configured
- DynamoDB TTL on job records for auto-cleanup
- Configurable Fargate CPU/memory and log retention periods
- Add SQS VPC endpoint to reduce NAT dependency
- Cover all new options in tests (20/20 passing)
- Add LICENSE (MIT), CONTRIBUTING.md, PR template, .editorconfig - Overhaul README with badges, config tables, policy matrix, project tree - Improve package.json with description, keywords, repo metadata - Conditional NAT Gateway (~2/mo saved) — deployed only when enrichment API CIDRs are configured - DynamoDB TTL on job records for auto-cleanup - Configurable Fargate CPU/memory and log retention periods - Add SQS VPC endpoint to reduce NAT dependency - Cover all new options in tests (20/20 passing)
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds valuable cost optimization features (conditional NAT Gateway, DynamoDB TTL) and improved configurability (Fargate CPU/memory, log retention). The repository polish improvements are also excellent.
Critical Issue Found:
- DynamoDB TTL is enabled but the Ttl attribute is never populated in job records, preventing auto-cleanup from working. The removed RetentionDays field must be replaced with proper Unix epoch timestamp calculation.
All other changes reviewed:
- Conditional NAT Gateway logic is correct and will save ~$32/month when enrichment APIs aren't needed
- SQS VPC endpoint addition is appropriate
- Configurable Fargate resources and log retention work correctly
- Helper functions in deployment-config.ts improve code quality
- All security configurations remain properly enforced
- Tests comprehensively cover new features (20/20 passing)
Once the TTL population issue is fixed, this PR will be ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| pointInTimeRecoverySpecification: { | ||
| pointInTimeRecoveryEnabled: true, | ||
| }, | ||
| timeToLiveAttribute: 'Ttl', |
There was a problem hiding this comment.
🛑 Logic Error: TTL attribute 'Ttl' enabled but never populated in DynamoDB items. The removed RetentionDays field from markJobStarted (deleted at old line 420) must be replaced with a Ttl attribute containing Unix epoch timestamp (current time + jobRetentionDays * 86400). Without this, job records will never auto-expire despite TTL being enabled.
The TTL attribute was enabled on the DynamoDB table but never written into job records, preventing auto-cleanup. Add a small inline Lambda that calculates current time + (jobRetentionDays * 86400) and invoke it from the Step Functions workflow before the MarkJobStarted PutItem task.
1 participant
